超低レベルC言語プログラミング・三流エンジニア（私）のくだらないこだわり

はじめに

私事ですが、過去に機器メーカーのソフトウェアエンジニア部隊に所属しておりまして、長いことC言語での組み込みソフトウェア開発に携わっていた人間です。

仕事でソフトウェア開発に関わったことがある方なら大なり小なり経験はあると思いますが、この世界には、視点や考え方、時代の変遷で変わったり、あるいは純粋に宗教的とも取れる側面があり、何が正しくて何が悪いのか分からなくなることが多々あります。

そんな中では、自分の考え方が職場での開発スタイルとは相容れなかったり、なかなか周囲からは賛同してもらえなかったりすると、余計なストレスを抱え込むことになります。

幸いなことに組み込みソフトウェア開発とは距離を置くようになり、抑圧からは解放されたのですが、あの頃の自分を忘れないよう、感じたことや不思議だったことなどを思いつくまままとめてみました。
こだわり強めでうだつの上がらないソフトウェアエンジニアの自分流を書き連ねた、くだらない記事ということで、斜めにでも読んでいただければ幸いです。

関数の仮引数

const修飾子を付けるようにしています。こんな感じ。

void hoge(const int count)
{
    int i;
    for (i=0; i<count; i++)
    {
        //　count回繰り返しする処理
    }
}

仮引数を関数内のワーキング変数（作業用変数）として使いませんよ、という意志表示です。

仮引数を作業用変数として使うということは、パフォーマンスを落としたり脆弱性を引き起こしたりといった弊害は全く無いのですが（ポインタの場合はまた別問題ですが）、その変数の持つ意味合い（振る舞いであったり目的）が複数になってしまい、その結果、作成者の意図がぼやけたり、可読性が悪くなるケースがあり、コードのメンテナンス性を悪化させる原因となります。

仮引数への代入を良く無いとするコーディングルールとして一般的で、職場での静的解析ツールのチェック項目に入っていました。このため、この原則そのものは皆さん守っているようでした。しかし「const修飾子を付けたらいいんじゃない？」という提案に対しては最後まで賛同は得られませんでした。

その真意についてきちんと話は出来なかったのですが、こんな感じなのかなと憶測しています。

• ルールチェッカに指摘されない（スコアが良い）コードが正義で、そのルールが何を意図しているかには全く興味が無い（そんなことを考える時間が無駄）⇒constを付けても付けなくても結果は同じなのだから、無駄な労力
• 余計な記述が増えることで可読性が悪くなる（規約で1行あたり80文字の制限があるので文字数が増える行為はNGだそうです）
• 若手・初心者には「ROMに配置するためconstを付ける」と教えているので、彼らを混乱させる

我が職場の優秀な方々は、ずっと高みにいたようです。

変数の意味

上の項目ともつながっているのですが、変数にも役割や意味合いを持たせたいというこだわりです。
ADコンバータの換算処理での例で示しましょうか（良い例が思いつかなかったので説得力はイマイチなのですが）。

#define ADC_RESOLUTION (12) // ADC分解能ビット数
static const double vref = 3.3F; // ADC基準電圧[V]
static const double amp_gain = (120.0F / 5.1F); // 入力アンプ回路ゲイン
static const double amp_offset = (0.4F); // 入力アンプ回路オフセット電圧[V]
/* @brief 信号入力12ビットADC変換データからアナログ回路ゲインを逆換算し信号源電圧値を取得
   @param a_in:ADC変換データ
   @ret 信号源電圧換算値[V]
   @detail 換算式は（省略）
   @note このサンプルではa_in異常値（offset以下もしくは4096以上）のエラー処理は省略
 */
double GetSourceVoltage(const uint16_t a_in)
{
    const int adc_conv = ((2 ^ ADC_RESOLUTION) - 1); // ADC正規化係数
    double ain_shift; // 入力値に量子化誤差0.5LSB分下駄を履かせて浮動小数点化（途中経過）
    double ain_scale; // 0.0-1.0スケールに正規化した入力値（途中経過）
    double v_ain; // 電圧換算した入力値
    double v_source; // アンプのゲイン／オフセットを逆算した信号源電圧換算値
    
    ain_shift = (double)a_in + 0.5F;
    ain_scale = a_round / (double)adc_conv;
    v_ain = (ain_scale * vref);
    v_source = (v_ain - amp_offset) / amp_gain
    return v_source;
}

これに対し、皆様のコード（スタイル）は概ねこんな感じ。

#define ADC_RESOLUTION (12) // ADC分解能ビット数
#define ADC_VREF (3.3F) // ADC基準電圧[V]
#define ADC_GETA (0.5F) // 量子化誤差を最小化するための下駄（0.5LSB）
#define AMP_GAIN (120.0F / 5.1F) // アナログフロントエンド回路ゲイン
#define AMP_OFFSET (0.4F) // 入力アンプ回路オフセット電圧[V]
double GetSourceVoltage(uint16_t a_in)
{
    double tmp;
    
    tmp  = (double)a_in + ADC_GETA; // 入力値に量子化誤差0.5LSB分下駄を履かせて浮動小数点化
    tmp /= (double)((2 ^ ADC_RESOLUTION) - 1); // 入力値を0.0-1.0スケールに正規化
    tmp *= ADC_VREF; // ADC基準電圧を掛け算して電圧[V]に変換
    tmp  = (tmp - AMP_OFFSET) / AMP_GAIN // アンプのゲイン／オフセットを逆算して信号源電圧に換算
    return tmp;
}

つまり換算の途中経過において、意味合いであったり、単位系やスケーリングの違うものを別の変数に割り当てるか、一つのtmpという変数で使いまわすかの違いです。

確かに、ぱっと見は皆様のコードの方がすっきりしていて、理路整然と一気通貫しているようにも思われます。ですが、意味合いや特性が異なる値をtmpという一つの変数でやってしまっていることと、コメント文を解析しないと途中経過を把握できない（そしてその大前提として落書きかも知れないコメント文を信じるしかない）ことに気持ち悪さを感じてしまいます。

コメントではなく式や文、変数といったコードそのものに意味を持たせたい、というのが私のこだわりです。

後はデバッグ時にデータ処理の過程を変数ウォッチする時に分かり易くなるというメリットもあるか？

もっと言うなら、それぞれの特性に見合った変数の型を定義してより厳密にしたいとか、もうちょっとモデルっぽく記述したいとか、言語そのものに不満があるので余計なことを考えてしまうのかも知れません。
理想は追わず言語に合わせて臨機応変に考え方を切り替える対応力が足りないのは自覚していますが、これができないのが性格かも。

なお、職場では「お前のコードは分かり難い」と見事に撃沈しました。

具体例（？）を一つ。ADC変換データについては量子化誤差が付きまとうので、その配慮が必要です。皆様の作るコードはコード全体でその対処が「必要な時必要な分だけ」きちんと行われていて、本当に頭が良くて優秀なんだなと舌を巻いてみていました。コードレビューの時、説明されたコードを全体構造を含めリアルタイムに把握し「ここは対処が抜けている、要修正」などと適切に指摘する能力など私には有りませんでした。
自分ならADC変換データを取り扱うクラスもどきを作って、その中で丸め処理だったりの諸々をメソッドっぽく作り込んで、（量子化誤差を内包する）生データがあちこち好き勝手に出現しないよう歯止めをかけるに作るのですが、「そんな必要あるの？意味不明」と、まるっきりストイックではない私のこのスタイルは全く相手にされませんでした。

マジックナンバーについて

規約に「マジックナンバーだめ。マクロ定義すること」とあり、そのこと自体は理にかなっているのですが、どこからを駄目なパターンとするのか、その判断がメンバーによって解釈に幅があり、苦慮しました。

個人的には、文脈から明らかに意味の通る（分かる）数値は必ずしもマクロ定義する必要は無いと考えているのですが、プロジェクトリーダーが原理主義だと、式の中で数値が代入されている箇所があるだけで「規約に違反しているだろ！」と、彼とその支持者であるチーム全体から吊るし上げを食らうので辛い思いをしました。

それにしても「マクロ定義すべし」というのが奮っていてます。そもそもプリプロセッサ文は必要以上に乱用しないのが一種のお行儀かな、と信じている自分との価値観の違いは、何というか色々と考えさせられました。

一般的な規約だとマジックナンバーの類は、const修飾子付きの変数の値として持つか、enumで与えるのが上質なコードだというこだわりです。自分としても先のサンプルで挙げたような

const double vref = 3.3F; // ADC基準電圧[V]
const double amp_gain = (120.0F / 5.1F); // 入力アンプ回路ゲイン
const double amp_offset = (0.4F); // 入力アンプ回路オフセット電圧[V]

でいいかなと。後は

const int timeout_num = 3; // タイムアウト回数
const int timeout_ms = 250; // タイムアウト時間[ms]

とか。これなら変数名で何を表現している数値なのか、ほぼほぼ推論できる（≒マジックナンバーではない）と自分は判断しています。

コードこそ最上のソフトドキュメントである！　などと私のこだわりを偉そうに語ったら失笑を買いました。

原理主義者のコードはヘッダファイルなりソースコードのマクロセクションに

#define TIMEOUT_X2 (2) // タイムアウト回数=2回
#define TIMEOUT_X3 (3) // タイムアウト回数=3回
#define TIMEOUT_X4 (4) // タイムアウト回数=4回
#define TIMEOUT_X5 (5) // タイムアウト回数=5回
#define TIMEOUT_250MS (250) // タイムアウト時間250[ms]
#define TIMEOUT_250MS (500) // タイムアウト時間500[ms]
#define TIMEOUT_250MS (1000) // タイムアウト時間1[sec]

のような感じでつらつらと記述されていて、くらくらしてきます。

これ、例えばソフト仕様でタイムアウト回数やタイムアウト時間の設定やモードが増えるたびに

#define TIMEOUT_X6 (6) // タイムアウト回数=6回（バージョンX.XXで追加）
#define TIMEOUT_750MS (750) // タイムアウト時間750[ms]（バージョンY.YYで追加）

のような感じでコード追加されていくんですぜ？

そもそも、他の項目も含めてですが、具体的な判断や運用について規約もガイドラインも無く、リーダーやレビュワーのその場その場の気分で良かったり駄目だったりという運用は望ましくないように感じていました。これについては恐らく意図的（あまり深くは追及しませんが）なもののようですが……。

コンパイラに任せたい

同僚とのちょっとした会話です。「C言語にも型推論があればいいのにね」と話題を振ったところ「不要。もし実装されたとしても、俺の目が黒いうちは使用を絶対に許さない」という反応が返ってきました。

型推論といってもオブジェクト指向言語で実装されている本物ではなく、例えばこんな感じのコードで、ちょっとしたことをコンパイラに判断させたい程度の内容です。

#define NUM_COUNT (200)
...
void func(void)
{
    uint8_t i;
    for (i=0; i<NUM_COUNT; i++)
    {
        //　NUM_COUNT回繰り返しする処理
    }
}

これ、仕様変更などで NUM_COUNT の値を例えば300に変更したらアウトな潜在的なバグを持っています。

私だったらこういった場合のループカウンタ i はint型で宣言しますが、規約によりint型の使用は禁止です（基本型は処理系依存だからという理由は分からないでもないですが、一方でsize_t型は使用が推奨されていたり整合性がありません……size_t型も処理系によってunsigned longだったりunsigned intだったりするのにね）。
なら安全サイドに振って32ビット型で宣言しちゃおう（どうせ今時は32ビットマイコンが標準で8ビットや16ビットの処理系に移植するなんてこと無いでしょ）とかやったら、必要以上に大きな型の変数は使うなと怒られました。1バイトは血の一滴（いや、1ビットは血の一滴かな）という文化が残っている職場でした。

要するに、人間が全てをコントロールしないと気が済まない、コンパイラは信用ならない存在（ましてやコンパイラが変数の型を勝手に弄るのことなど到底許せない）ということのようです。
ですが、ならアセンブリ言語で書けよ……などとは口が裂けても言い返せませんでした。

そう言えば、コンパイル後にリンカが出力したMAPファイルを見て「何だこの勝手にROMに配置されている関数は？ランタイム？こんなのは知らん消せ」と大騒ぎした人がいましたっけ。

職場ではコードは全て静的解析ツールによるチェックをかけ、結果をレビューしないといけないため、先のコード例の場合はチェッカで検出できるので実質的には「潜在的なバグ」に成り得ないのですが、コード改修にかかる目に見えないレベルのコストがかかるはずです。

• 次の仕様変更で NUM_COUNT を300から250に戻したらループカウンタの型も戻すの？
• それとも16ビット型ならそのままで良いという判断で通すの？その基準は？

そういったことをいちいち考えたり、チーム全員で論議したりする労力、一つ一つはほんの僅かな時間ですが、その積み重ねをトータルすると馬鹿にならないコストになると思っているのです。そのことが気になってしまう怠け者の疑問でした。

モダンな言語の、コンパイラがイイ感じにやってくれるのを素直に良いな、と感じてしまう私とは断絶激しい環境でした。話は飛躍しますが皆さん、Rustがメモリ管理をどういう風にやっているか知ったら卒倒するんだろうな（当然のことながらRustも味見してみようか？といった機運もありませんでしたが、こんな危険な言語は絶対に使用禁止とか言いだす可能性もあるかな）。

「ウチはC言語をちょっと高級なアセンブリ言語として使ってるから」と事の本質をズバリ言い当てた同僚がいて、上手いこと言うなと感心した次第。やはり頭の良い人達が揃ってる職場でした。

ブール（ブーリアン）型とか

この記事のサンプルでも記述している bool や uint8_t といったCの標準型、実は職場では使えませんでした。社内で決められた共通ヘッダファイルを読み込んで、その中で記述されている、独自の型を再定義したものを使用するルールになっていました（古いコーディングスタイルでたまに見る BYTE や WORD に近い感じでしょうか）。

理由としては「標準ライブラリの使用禁止」という規約により、コンパイラから提供される標準のヘッダファイルを読み込むという行為自体にチェックが入ってしまい、stdint.h や stdbool.h の読み込みも（厳密には）禁止対象となっていたことがあります。

その割にはmath.hのインクルードは見て見ぬふりだったりバランスに欠けていたような気もする。

さらに根源的には、C99で追加された言語仕様（stdint.h や stdbool.h もこれに属する）は互換性や相互運用性に欠ける（ので使用しない方が良い）という古い認識が上の人の頭にあり、それがアップデートされないまま今に至っているのではないかと推測しています。

「独自型は再定義するべきではなく、C言語標準の型を使う」 という考え方がデファクトじゃないの？　というのが私の認識なのですが、「今のやり方で具体的にどんな弊害があるの？」という反論に対応できず、素直に折れ、郷に入っては郷に従えと会社の流儀に合わせていました。

あと新人や若手を混乱させるから、今までやって来た流儀や型名称を変えるな、という判断もあったかと思います。

型名称については所詮ポリシーの問題なので些細なこと（タイトル通り私のくだらないこだわり）ではあるのですが、ブール型の解釈には最後まで釈然としないものを感じていました。

typedef unsigned char BOOL
#define FALSE (0)
#define TRUE (1)

こんな感じ。で、こんな風に定義したBOOLやFALSEやTRUEを使えと。このやり方のヤバさを説明するのも面倒くさいですが、一言で言うと「言語仕様に無いことはするな」です。

そして、こんなコードがそこら中にあるわけです。

BOOL a;

a = IsEnabled(); // TRUEかFALSEを返す
if (a == TRUE)
{
    // Enableされたらここで処理をやる
}

これ、良く無いよ～とアドバイスしても、「aにはFALSE(0)かTRUE(1)しか代入されないんだから弊害は無いよね？　何が良く無いの？」という反応でした。
皆さん、違和感を覚えることなく、これが常識だと思っていたようです。世間一般のコーディングに触れていれば「ちょっと違う？」と自分たちのコードがどれだけ特殊（古いスタイル）か気付くこともあったでしょうが、OSSや社外ライブラリの使用は無い職場だったせいか、そういう認識は無いようでした。

私は、Cの言語仕様では「0が偽、0以外が真」なのに対し、上のコードだとaに2とか3が代入されていると、偽として扱われてしまい言語仕様と矛盾するので、trueとの比較式で判定するのは見えない危険性があるんじゃないの、という立場を取ります。

更に言うなら、私の中では、ブーリアン型に対して条件比較式は使わず、

#include <stdbool.h>
bool a;

a = IsEnabled(); // trueかfalseを返す
if (a)
{
    // Enableされたらここで処理をやる
}

とやりたいのがこだわりの部分ですが、実体が unsigned char な「なんちゃって」ブール型だと（比較演算子を通さない値だけの条件式は）静的解析ツールが駄目だよと言ってくるので、TRUEかFALSEとの比較演算子を使わざるを得ず、これも相まって、私のこだわりが説得力を持つことはありませんでした。