OpenBMC Anti-Patterns: 我的code你真的不要再搞笑

很多同學問我，我想從某某行業轉來做OpenBMC會很難嗎？最辛苦的地方可能在哪裡？第一件事情我會問的就是...「你擅長寫C++嗎？」，這個問題至關重要，我知道有些人的會跟我會開車一樣，就是我有駕照但我不上路的那種會！好！如果只是這種程度，那我會覺得還要再加油，不說別人我本人也是一樣，我還要加油！第二個我會問的問題就是「你對作業系統了解多少？」，我們會遇到很多coding很厲害的同學，但是在他的世界裡面所謂全部就只有他自己寫的那支程式，然而組成一個完整的BMC，需要有好多好多service在同時運作，彼此之間甚至有著很深的羈絆，在這愛恨糾葛中，有很多需要工程師去好好安排之處，才能讓整個系統運作的美妙且和諧。（這句形容有點噁，但就是這樣沒錯。）

前段時間有分享過OpenBMC gerrit link給大家，如果你仔細去研究upstream maintainer的留言，會發現國人（台灣工程師們）很常會被反覆提到某些不該搞笑的寫法。這些其實都被規範在一份叫做“openbmc/docs/anti-patterns.md”的文件中。如果上面都是英文，你看了跟沒有看一樣。那麼今天，我帶你一起把它完整的看過一遍！

(1) Custom ArgumentParser object

許多早期 OpenBMC 專案各自實作了自己的 ArgumentParser 類別，通常是為了包裝 argc/argv 或 getopt() 等函式。問題在於這些 parser 幾乎都在做相同的事，只是接受的參數不同。這導致：

各 repo 之間出現大量重複程式；
每個專案都必須維護自己的一份 parser；
若要修改行為（例如增加 --help 或預設值），需要在多處修改。

CLI11 是目前官方建議的現代替代方案，它支援子命令、預設值、型別轉換與驗證，也能與 C++17/20 的特性整合。

// ❌ 
class ArgumentParser {
public:
    ArgumentParser(int argc, char** argv) {
        for (int i = 1; i < argc; ++i)
            args.emplace(argv[i]);
    }
    bool has(const std::string& arg) { return args.count(arg); }
private:
    std::unordered_set<std::string> args;
};

// ✅ 正確方式：使用 CLI11
#include <CLI/CLI.hpp>

int main(int argc, char** argv)
{
    CLI::App app{"BMC Example Tool"};
    std::string configPath;

    app.add_option("-c,--config", configPath, "Config file path");
    CLI11_PARSE(app, argc, argv);

    std::cout << "Config: " << configPath << "\n";
}

(2) Explicit listing of shared library packages in RDEPENDS

Yocto 的 Bitbake 系統在建構階段會自動分析二進位檔所連結的共享函式庫（如 libsystemd.so、libstdc++.so），並自動產生相依性。因此若在 recipe 中手動列出 RDEPENDS_${PN} = "libsystemd"，不但多此一舉，還可能因版本更新導致相依清單失效。

這種「重複宣告依賴」的反模式會使映像檔變大、維護難度上升。官方建議依賴交由 Bitbake 自動偵測。

# ❌ 錯誤寫法
RDEPENDS_${PN} += "libsystemd libjson-c"

# ✅ 正確做法：刪除手動宣告
# Bitbake 會自動依照 ELF dependency 補上相依套件

(3) Use of /usr/bin/env in systemd service files

有些 OpenBMC 專案在 systemd 的 .service 檔中使用：

ExecStart=/usr/bin/env phosphor-hwmon

這樣做原本是為了在唯讀檔系統上支援動態覆寫 /usr/local/bin。但這種寫法在其他 Linux 專案幾乎不出現，因為它不直覺、難以維護。新的方法是使用 overlay mount 技術在 /usr 上建立可寫層，讓 live patch 不再需要 env。

因此，service 檔應直接指定完整路徑：

ExecStart=/usr/bin/phosphor-hwmon

(4) Incorrect placement of executables

根據 Filesystem Hierarchy Standard（FHS），系統管理工具與一般使用者工具應明確區分。許多早期專案將 daemon 安裝到 /usr/sbin 或 /bin，這導致：

daemon 無意間被加入 $PATH；
使用者誤執行不應直接啟動的程式；
不符合系統層級的標準。

# ❌ 錯誤路徑
install -m 755 my-daemon ${D}${sbindir}

# ✅ 正確路徑
install -m 755 my-daemon ${D}${libexecdir}/my-package/

系統由 systemd 控制 daemon 啟動，因此它不應放在 $PATH 之中。

(5) Handling unexpected error codes and exceptions

這是韌體開發中最常見、也最具危險性的反模式。許多程式碼為了「讓系統不要 crash」，在 catch 區塊中僅記錄 log，卻不真正處理錯誤。例如：

try {
    bus.method_call(...);
} catch (InternalFailure& e) {
    phosphor::logging::commit<InternalFailure>();
}

這樣寫的問題是：

錯誤被吞掉，服務仍繼續執行；
錯誤狀態未清除，可能導致後續邏輯異常；
只留下「InternalFailure」的 log，無法還原上下文。

正確做法是使用 std::system_error 或自定義 exception，並根據情況決定是否中止服務。例如：

try {
    bus.method_call(...);
} catch (const sdbusplus::exception::SdBusError& e) {
    lg2::error("DBus error: {ERR}", "ERR", e.what());
    throw std::system_error(EIO, std::generic_category(), "DBus failed");
}

(6) Non-standard debug application options and logging

早期不同 OpenBMC daemon 有自己的 debug 旗標，例如 --vv, 0xff, -d 等。這造成開發者在不同模組間調試時必須重新學習命令介面。為了解決這問題，建議統一採用：

CLI11 的 -v,--verbose；
journald 的標準 log level（info, debug, err 等）。

實例：

if (verbose) {
    lg2::debug("Entering function {FUNC}", "FUNC", __func__);
}

使用 journald 可以透過：

journalctl -p debug

直接檢視 debug 級別訊息，而不需自行控制輸出緩衝。

(7) DBus interface representing GPIOs

GPIO 屬於硬體層實作細節。若直接在 DBus 上暴露 xyz.openbmc_project.Gpio.Pin0 等接口，會使上層應用依賴具體硬體訊號，破壞軟體抽象層設計。

例如，某機種用 GPIO 控制「風扇啟動」，另一機種改成 I²C。若 API 被綁在 GPIO 上，程式就失去可移植性。

正確做法是抽象為「行為導向」的介面：

interface xyz.openbmc_project.Control.Fan
method Start()

讓具體實作在底層決定使用 GPIO 或其他機制。

(8) DBus interface representing GPIOs

Boost.Asio 與 sdbusplus 的非同步呼叫常用 lambda callback，但如果 lambda 內容太長（>10行），會導致：

可讀性下降；
無法單元測試；
無法設置斷點。

不良範例：

getSubTree("/", interfaces, [](auto ec, auto& res){
    if (ec) { ... }
    // 20 lines of processing
});

改進版本：

void handleSubTree(std::shared_ptr<bmcweb::AsyncResp>& resp,
                   boost::system::error_code& ec,
                   MapperGetSubTreeResult& res) { ... }

getSubTree("/", interfaces, std::bind_front(handleSubTree, asyncResp));

這樣一來，邏輯清晰、可重用，也能在單元測試中獨立驗證。

(9) Placing internal headers in a parallel subtree

C/C++ 中 include/ 資料夾的設計是為了放置公共 API。若專案本身是應用程式（非函式庫），其內部 header 不應安裝或暴露給外部。
將內部 header 放在 src/ 旁即可。例如：

src/
  main.cpp
  util.hpp  <-- internal header
include/    <-- 僅限公開 API

這樣可以減少路徑設定、降低維護成本，也避免他人誤用內部函式。

(10) Ill-defined data structuring in `lg2` message strings

lg2 是 OpenBMC 的標準 logging API，它基於 systemd-journald，可自動將 key-value pair 存入結構化欄位。但許多開發者誤以為要手動把所有資訊塞進字串，例如：

lg2::error("Error PATH={P} CODE={C}", "P", path, "C", code);

這樣做會讓訊息難以讀，也破壞 machine-readable 結構。

正確作法是只在 MESSAGE 放人類可讀的敘述，讓 journald 自動保存 metadata：

lg2::error("Failed to read sensor {SENSOR}", "SENSOR", name);

用 journalctl -o verbose 可以看到：

MESSAGE=Failed to read sensor TMP75
SENSOR=TMP75
LOG2_FMTMSG=Failed to read sensor {SENSOR}

補充說明：
在 OpenBMC 中，lg2 是 Phosphor-logging 專案提供的標準 C++ logging 介面。它的底層是 systemd-journald，而 journald 的設計與傳統 syslog 最大的不同在於：

syslog 是「純文字」；早期我們把所有資訊都串成一個字串，當我想要去BMC裡面找到有用的資訊的時候，必會用grep或者regex 去解析，更糟的是不同開發者拼字方式不一樣，有的寫 PATH=xxx，有的寫 BMC_PATH=xxx，導致查找困難。
journald 則是「結構化（structured）」。所謂結構化，就是除了訊息（MESSAGE）之外，每一筆 log 還可以帶有多個 key-value 屬性，

例如：

MESSAGE="Time mode changed to NTP"
MODE="xyz.openbmc_project.Time.Synchronization.Method.NTP"
CODE_FILE="manager.cpp"
CODE_LINE=98

這些欄位都能被 journald 自動儲存與索引。這代表——我們不需要在字串裡手動拼接這些資訊。

另外就是，早期我對 journald 不熟悉。以為只有 MESSAGE 會被顯示在 journalctl 裡，所以會想：「既然 MESSAGE 是唯一確定會被看到的，那我就把所有的變數或內容都塞進去吧！」於是就出現像這樣的寫法：

lg2::error("DBus call failed. PATH={PATH}, INTERFACE={INTF}, ERROR={E}", 
           "PATH", path, "INTF", intf, "E", e);

從表面上看，這樣的 log 很「完整」——看起來資料都在裡面。但問題是，它其實破壞了 journald 的整個資料結構理念。把關鍵訊息規範好，然後記錄下來，Message可以是人看得到最重要的資訊且一看就懂，同時細節可以在其他的欄位給機器查找時方便使用。（該顯示哪些資訊叫做重要資訊，我認為這就完全需要大家討論去達成共識）例如：

MESSAGE="Error getting time"
BMC_TIME_PATH="/xyz/openbmc_project/time/bmc"
TIME_INTF="xyz.openbmc_project.Time.Manager"
ERR_EXCEP="Timeout"

這是儲存在 binary log database 裡的結構化資料，每個 key 都是獨立欄位。因此當你用：

journalctl --output=json | jq -r '.TIME_INTF'

其實不是「字串搜尋」，而是查詢已結構化的欄位。