C言語23年、自分はフリーランスで組み込みのコードレビューに入ることが多いです。
マージ前レビューで一番怖いのは「一見安全そうに書いてあるコード」です。p ? *p : -1 のような3項演算子でのNULLチェック、if (!p) return -1; のような論理否定チェック。これを見た瞬間、レビューワの脳は「あ、ちゃんとチェックしてるな」と安心して次の行に進みます。ところが実際に手を動かして読み直すと、チェックしているのは別の変数だったり、チェックの後に別のポインタを触っていたり、ということがざらにあります。今回のサンプルコードでも、ternary_null_check() は一見安全に見えて、creviewは「NULL検査せず使用」と重大判定を出しました。人間の目が「安全パターンっぽい」で止まってしまう場所を、機械がもう一段掘って矛盾を拾う。この記事はその具体例です。
この記事の前提（SCQA）
-
状況: マージ前レビューでは、NULL チェックの「形」を見た瞬間に安心してしまうことが多いです。
if (!p)やp ? :があると、レビューワの注意はそこで止まります。 - 問題: 実際には3項演算子や論理否定を使ったチェックの中身が、本当にその後の参照を守っているかまでは、目視だけでは追いきれません。特に関数が長くなるほど見逃しが増えます。
- 問い: 「NULLチェックがある/ない」の二値判定ではなく、契約として機械的に検証する方法はないか？
-
答え:
__attribute__((nonnull))で契約を明示した上で、creview に NULL チェック漏れパターンを判定させ、レビューワは設計判断（この関数はそもそも NULL を受け取っていいのか）に集中する分業にしました。
L0: まずコンパイラ警告を全部潰す
creview の話に入る前に、-Wall -Wextra -Wpedantic -Werror -fanalyzer
(GCC) または -Wall -Wextra -Werror -Weverything (Clang) を有効に
してビルドが通るか確認してください。NULL 引数を nonnull 関数に渡す /
未初期化 auto 変数を読む / 戻り値を破棄する / 自明な NULL deref /
use-after-free / 境界外配列アクセス（静的決定可なもの）/ switch case
抜け、これらは現代のコンパイラが標準で拾います。
これらが警告ゼロにならない PR は、creview を走らせる前にそもそも
レビュー対象にしないルールにしてしまうのが第一手です。
実際、今回のサンプルの bad_uninit() は -Wall を付けるだけで gcc が -Wmaybe-uninitialized 相当の警告を出すケースです。ここを creview の仕事だと思って探しに行くと二度手間になります。L0 で潰せるものは L0 で潰す、これが鉄則です。
検証用コード
// NULL未チェック
void bad_null(int *p) {
*p = 42;
}
// 未初期化変数
void bad_uninit() {
int x;
int y = x + 1;
}
// goto使用
void bad_goto() {
goto end;
end:;
}
// 再帰
int bad_recursive(int n) {
return bad_recursive(n - 1);
}
// 安全な関数
int good_func(int *p) {
if (p == NULL) return -1;
return *p;
}
// 3項演算子でのNULLチェック（安全）
int ternary_null_check(int *p) {
return p ? *p : -1;
}
// 論理否定でのNULLチェック（安全）
int logical_not_check(int *p) {
if (!p) return -1;
return *p;
}
CWE/MISRA マッピング
| 検出パターン | 該当箇所 | CWE | 事故の形 |
|---|---|---|---|
| NULL チェック漏れ（契約未表明） |
bad_null() L2 |
CWE-476 | 呼び出し側が NULL を渡した場合に即クラッシュ。契約が nonnull 属性で表明されていないため、コンパイラも静的解析も気付きにくい |
| マジックナンバー |
bad_null() L3 (42) |
CWE-547 | 意味不明な定数がハードコードされ、後から仕様変更時に修正漏れが起きる |
| auto 変数の未初期化使用 |
bad_uninit() L9 |
CWE-457 |
x はスタック上の不定値。y の計算結果がビルド環境やレジスタ残留値次第で変わる UB |
| 深さ制限なし自己再帰 |
bad_recursive() L19 |
CWE-674 | 呼び出し回数の上限がなく、n が負の無限ループに陥って即スタックオーバーフロー |
| NULL チェック漏れ（3項演算子擬態） |
ternary_null_check() L30 |
CWE-476 | 見た目は p ? *p : -1 で安全パターンに見えるが、creview は「関数の契約として nonnull 表明がない」点を独立に指摘。呼び出し側が NULL を渡す余地が残っている限り、契約としては未完成 |
goto 自体（bad_goto()）は creview の重大/保守危険リストには出てきません。前方ジャンプでラベル直後に処理がないだけの無害な形なので、これは妥当な判定です。「goto を見た瞬間に指摘したくなる」のは人間側の思い込みで、実際に危険なのは goto がスコープをまたいで初期化をスキップするケースです。ここは creview のリストと自分の直感がズレた箇所で、最初は「goto 出てないの、見逃しでは」と疑って調べ直しました。結果は誤検出ではなく妥当な非検出でした。
creview の使い方（CLI 実行例）
creview review sample.c --preset security --format markdown
L2 【重大】: ATTR-NONNULL-001: bad_null() の p を NULL 検査せず使用。__attribute__((nonnull(N))) で契約を明示するか、関数先頭で NULL 検査して return
L3 【保守危険】: マジックナンバー42 (リテラル) — allowlist 外。#define / enum / .creviewrc.json で命名 or 許可
L9 【重大】: 未初期化変数xを使用(8行目で宣言、初期化なし)。不定値
L19 【保守危険】: 関数bad_recursiveが自己再帰。深さ制限なしでスタックオーバーフローの危険
L30 【重大】: ATTR-NONNULL-001: ternary_null_check() の p を NULL 検査せず使用。__attribute__((nonnull(N))) で契約を明示するか、関数先頭で NULL 検査して return
3段階ラベルの読み方は以下の通りです。
- 【重大】: 実行時に即クラッシュ、または UB に直結するパターン。マージ前に必ず解消。今回は NULL deref 契約未表明と未初期化変数の2種
- 【設計不明】: 危険かどうかがコードだけでは判断できず、設計意図の確認が必要なパターン（今回のログには該当なし）
- 【保守危険】: 今すぐ落ちはしないが、将来の変更で事故率が上がる書き方。マジックナンバーと無制限再帰がここに入る
ternary_null_check() と logical_not_check() は関数名としては「安全」を謳っていますが、creview は前者だけ重大判定を出し、後者（if (!p) return -1;）は指摘なしでした。理由を追うと、logical_not_check() は早期 return で p の参照経路が完全にガードされているのに対し、ternary_null_check() は式全体の評価順序に依存していて、契約として nonnull を宣言していない点を独立に問題視しているためです。ここは自分も一度「両方安全なはずなのになぜ差が出るのか」で止まりました。creview のログのコメント文言をよく読むと「契約の明示があるかどうか」を見ていると分かり、実装の安全性と契約の明示は別軸だと理解し直しました。
3段階の使い分け
| 場面 | 使うツール | 使いどころ |
|---|---|---|
| L0: ビルド時 (まずここ) | gcc / clang の警告 + -Werror
|
-Wall -Wextra -Wpedantic -Werror -fanalyzer (GCC) / -Weverything -Werror (Clang)。警告ゼロにならない PR はそもそもレビューに入らないルール化 |
| L1: PR チェック | clang-tidy / cppcheck | OSS の主流チェックを CI で強制 |
| L2: PR レビュー | creview（CLI） | プロジェクト固有パターン、日本語ナレッジ、severity モデル (重大/設計不明/保守危険)、CWE/MISRA マッピング、--preset pr で diff のみ、AI 補強 |
| L2': 自己チェック (PR 投げる前) | c-review-ai（ブラウザ版） | 環境構築不要、コピペで試せる軽量版 |
| L3: チーム監査・MISRA 準拠 | CSAF | libclang AST + 依存グラフで risk A/B/C 自動昇格 |
自社プロダクトの creview はここでは L2 の「マージ前レビュー」に置いています。自社プロダクトの c-review-ai は PR を投げる前の自己チェック用、自社プロダクトの CSAF はプロジェクト全体の定期監査用として役割を分けています。3つとも同じ検出ロジックを積んでいるわけではなく、粒度と実行タイミングが違う道具として使い分けるのが実務上ラクです。
まとめ
- NULL チェックは「形」ではなく「契約の明示」で見る。3項演算子や論理否定を使った擬態チェックは、目視レビューだけでは安全と誤認しやすい
- 未初期化変数（auto）は CWE-457、静的変数のフラグ立て忘れは CWE-665。両者を混同すると誤った修正提案をしてしまう
- goto は「使っている」こと自体が危険なのではなく、スコープや初期化をまたぐジャンプかどうかが判断基準。creview が goto を拾わなかったのは妥当な非検出だった
- 無制限の自己再帰は CWE-674。マージ前レビューでは「終端条件はあるか」だけでなく「深さの上限はあるか」まで確認する
- L0（コンパイラ警告ゼロ）を通っていない PR は、そもそも creview を回す前に差し戻す運用にすると、L2 のレビューコストが目に見えて減る
試すリンク（無料で試す → 業務で監査する）
C言語の静的解析を、いきなり有料ではなく「無料で手を動かす → 日常運用 → 監査レベル」の順で用意しています。
-
まずブラウザで試す → c-review-ai（自社プロダクト / MIT / 無料）
貼り付けた1ファイルをその場でレビュー。導入ゼロで挙動を確かめる用。
→ https://cutt.ly/pyuTypUW -
CLI で日常運用 → creview（自社プロダクト / 無料）
手元のコードをコミット前に単発チェック。CI にも組み込める。
→ https://cutt.ly/uyuTyst0 -
リポジトリ全体を監査 → CSAF（自社プロダクト / ¥4,980 買い切り）
MISRA 準拠チェックと CWE データフロー解析をリポジトリ横断で走らせ、監査レポート（HTML）を出力。
レビューの属人化を止めたいチーム向けで、商用の静的解析ツールなら数十万円かかる基本機能を、個人でも入れられる価格にしています。
→ https://cutt.ly/5tKoVA2h
無料の2つで「使える」と感じたら、リポジトリ全体に監査を回すのが CSAF です。バージョンは随時更新しています。