低レベルなシステムのパッチで、細かい書き方より本当のバグを見つける

## バグらしき物は確信が弱くても出し、書き方の指摘はかなり確かな時だけ出す 正しさに関する欠陥を最も価値の高い指摘として扱います。正しさの不具合である可能性が50パーセント以上なら報告し、証拠が不完全な場合は不確かさを明示します。解放後使用、リソース漏れ、競合、境界違反の可能性は、追加確認が必要でも取り上げる価値があります。 スタイル、書式、手順に関する指摘には別の基準を適用します。 - 確信度が80パーセントを超える場合に限って報告します。 - 最終的に正しい、許容できる、または無害だと判断したものは報告しません。 - 一つの指摘の中で懸念を示した後に、それを自ら否定してはいけません。 - 可能な限り、各コメントを実行可能な一文にまとめます。 各パッチは、そのパッチ自体の動作に基づいてレビューします。単独の差分から、パッチ系列の順序、パッチ間のコンパイル依存関係、貢献者の事情を推測してはいけません。系列を扱う場合、問題のないパッチを「問題なし」と要約せず、報告対象から外します。 ## メモリとエラー時の流れを追い、漏れや解放後の使用を探す メモリ割り当て、開いたハンドル、取得したロック、初期化途中のオブジェクトについて、その後に続くすべての分岐を追跡します。早期リターンとクリーンアップラベルを確認し、必要な解放が漏れなく対応していること、正しい順序、エラーコードの保持を検証します。 次の項目を優先します。 - 解放後のアクセス、二重解放、ヌル参照、到達可能な未初期化読み取り。 - メモリ、記述子、ソケット、ロック、構築途中のメンバーの漏れ。 - 実際に失敗し得る処理に対する失敗確認の欠落。 - 誤った戻り値、破棄されたエラー状態、反転した条件、境界の一つずれ。 - 最初に読み取られる前に無条件で上書きされる値。 - 入れ子のループで同じループカウンターを再利用すること。 - サンプルを加算せず代入してしまうカウンター累積。 累積カウンター、ゲージ、初期代入を区別します。戻り値が整数型という理由だけで、未確認の戻り値を問題にしてはいけません。対応プラットフォームが、有効な引数ではその呼び出しが失敗しないと文書化している場合は特に該当します。 ## 整数の幅とパケットのサイズを確かめる 演算は格納先のビット幅だけでなく、オペランドのビット幅で確認します。 - 結果により広い型が必要なら、乗算前に少なくとも一方のオペランドを拡張します。 - 広いビットマスクへシフトする前に、リテラルまたはオペランドを格納先と同じ幅にします。 - ポインター演算や広い結果に使うシフトの前に、幅の狭い符号なし値を拡張します。 - 結果の全範囲が収まり、より広い文脈が関与しない場合は、狭い結果を得るシフトをそのままにします。 - ゲスト、パケット、デバイス、APIからの入力で制御される連鎖または添字の走査には、必ず上限を設けます。 パケットまたは記述子オブジェクトを一括解放する際は、所有権を尊重します。一つのアロケーターまたはプールを前提とする一括処理は、要素の出所が異なる可能性のある配列には安全ではありません。高速経路の契約で所有権の統一が証明されない限り、プールを認識する解放経路を使います。 プロトコルのペイロードサイズとフレーム全体のサイズを区別します。フレーム化のオーバーヘッドは一律の定数ではなくデバイスの能力から導出し、セットアップ後は古い要求フィールドではなく、正本となる現在の構成を読み取ります。 一つの受信バッファーにフレームが収まらない場合、実装は対応する複数セグメント経路を有効にするか、その構成を拒否しなければなりません。初期構成、キュー設定、実行時のサイズ変更の各段階で再確認し、構成済みモードと実際のサイズ要件の両方に基づいて受信関数を選択します。 ## 共有データに、volatileでなくアトミックやロックを使っているか確かめる `volatile`をスレッド間同期として認めてはいけません。共有される可変状態にはアトミック操作または適切なロックが必要です。`volatile`が制約するのはコンパイラーによるアクセスだけで、原子性もメモリ順序も提供しません。 正しさを保てる最も弱い順序を選びます。 - 関連データを公開しない独立した統計値やフラグには、緩和順序を使います。 - 公開と消費の関係には、リリース書き込みとアクワイア読み取りを使います。 - 状態を消費し、同時に公開する読み取り・変更・書き込み操作には、アクワイア・リリース順序を使います。 - 独立したアトミック値に単一の大域順序が必要な場合に限り、逐次一貫性を使います。 独立したフェンスより、アトミック操作自体に順序を指定する方を優先します。フェンスが適切なのは、より大きな非アトミックアクセス群の順序付けが必要な場合だけです。 ハードウェアレジスターや、実際にデバイスとやり取りするその他のアクセスには`volatile`を許容します。プロセス間共有メモリに配置するミューテックス、条件変数、バリア、読み書きロックにはプロセス共有の初期化を必須とし、プロセス専用メモリには要求しません。待機動作が用途に合う場合は、共有メモリ向けに設計された同期プリミティブを優先します。 ## 鍵や秘密の値を扱うパッチの時だけ、暗号の確認をする パッチが鍵、ノンス、認証情報、セキュリティプロトコルのコードを扱う場合に限り、暗号に関する確認を適用します。 - 攻撃者の影響を受ける認証タグ、ダイジェスト、MAC、完全性値は、定数時間の等価比較プリミティブで比較します。 - 通常のバイト比較を置き換える場合は、等価性の意味を維持します。 - 鍵、展開済みスケジュール、セッションシークレットなどは、解放前に省略不能な処理で消去します。 - 鍵、初期化ベクトル、ノンスは、暗号学的に安全な乱数源で生成します。 アルゴリズム識別子、長さ、能力、検索キーの通常の比較を問題として扱ってはいけません。再利用が実行時の性質であり、静的なパッチ差分では確定できない場合に、ノンスの再利用を断定してはいけません。 ## ライブラリのAPIを単純に保ち、互換性のないABIの変更にはバージョンを付ける 再利用可能なライブラリはコンパイラーのようなサービスとして設計します。ハンドルを作成し、入力を渡し、構造化された出力を受け取り、その後の処理は呼び出し側が決定します。呼び出し側をライブラリのバックエンドにしてしまう、単一インスタンス状態やコールバックテーブルは避けます。 公開コールバックとデータモデルについては、次を守ります。 - 大きな操作テーブルではなく、一つのコールバック引数を優先します。 - コールバックが五つを超えたら再設計の警告、二十を超えたらAPI設計エラーとして扱います。 - コールバックの所有権、有効期間、戻り値、失敗時の動作を文書化します。 - 失敗し得る処理に戻り値のないコールバックを使ってはいけません。 - 増え続ける列挙型と共用体の組み合わせより、拡張可能なタグ付きレコードを優先します。 ヘッダーとしてインストールされるものは、ラベルにかかわらずすべて公開APIです。特に内部の静的記憶域について、バッファーの所有権と有効期間を明示します。 新しいコードに非推奨インターフェースを導入してはいけません。非推奨化には、ツリー内に利用者が残っていないこと、リリース文書、移行経路、コンパイラーから認識できる注釈が必要です。公開、実験的、内部用の注釈はツールチェーンが期待する正確な位置に置き、ソース定義からシンボルをエクスポートし、互換性のない安定シグネチャー変更にはABIバージョニングを使います。生成されたシンボルマップの手作業による編集を要求してはいけません。 ## 機械的な書き方はツールに任せ、ツールで分からない所をレビューする 機械的な置換はフォーマッター、セマンティックパッチツール、リポジトリ検証スクリプトに任せます。これらのツールでは理解できないケースに、人間またはAIによるレビューを使います。 インデント、改行コード、インクルードのグループ、波括弧、関数レイアウト、識別子、名前空間接頭辞について、プロジェクトで確立された規則を確認します。静的リンクでの衝突を避けるため、外部から見えるライブラリまたはドライバーのシンボルには一意の接頭辞を必須とします。包摂的な用語を使います。 真偽状態には実際の真偽型を優先しますが、エラーコードや複数状態の値には整数を維持し、確立済みコードで無関係な型変更を避けます。実行時登録や能力検出によって変更される場合を除き、コンパイル時の関数ディスパッチテーブルは定数に保ちます。 割り当てとコピーのプリミティブをデータ経路に合わせます。 - 一般的な制御データには通常のヒープ割り当てを使います。 - DMA、キュー、プロセス間データには、デバイスから見え、NUMAを考慮し、ゼロ初期化された割り当てを使います。 - 最適化された一括コピーは、高速経路に効果があり、その契約を満たす場合に限って使います。 コメントではコードを言い直すのではなく、制約と意図を説明します。 ## コードを変えたら、テストとドキュメントも一緒に直す すべてのコミットは単独でビルドできなければなりませんが、パッチだけを見るレビュー担当者は系列の順序を推測してはいけません。プロジェクトが対応するコンパイラー、ターゲット、オプション、ビルドスクリプトで検証します。 新しい動作と公開APIには、対象を絞ったテストを追加します。リポジトリの機能テスト用アサーションとテストスイート実行器を使い、環境要件を正確に記録し、デバイス向けの新しいAPIには少なくとも一つの実装を用意します。 コードと文書を一体として更新します。 - 公開APIと、利用者から見える重要な変更を文書化します。 - 機能一覧表と操作文書を実装と一致させます。 - 公開API、新しいドライバー、または重大な動作変更については、現在のリリースノートだけを更新します。 - テストのみの変更や内部実装の変更に、リリースノートを要求してはいけません。 - 構造化文書で各項目が用語と十分な説明から成る場合は定義リストを優先し、短い項目や手順には単純なリストを維持します。 リポジトリで採用されているビルド言語のフォーマッターと順序規則を適用します。Pythonは指定されたフォーマッターと行長ポリシーで検証します。 ## 対象が安定版の時だけ、安定版の決まりを当てはめる 対象ブランチまたはリリースが安定版だと明示されている場合に限り、厳格な安定版リリース規則を使います。その場合は不具合修正だけを認め、新機能、公開API、ABI変更、シンボル、構成オプション、無関係なリファクタリングは認めません。 バックポートには元の修正への参照と、プロジェクトの安定版通知規則への準拠を必須とします。小さな修正の周囲で大規模な整理を行うことは、バックポートのリスクを高めるため警告として扱います。 リリースが指定されていない場合は、新機能やAPIが有効になり得るメイン開発ブランチを前提とします。リポジトリが安定版ブランチも維持しているという理由だけで、安定版専用の禁止事項を適用してはいけません。 ## 具体的な直し方を書き、出す前に指摘を2回見直す 確認済みのメモリ安全性、同期、境界、クリーンアップ、暗号、ABI、コンパイルの欠陥はエラーとして分類します。テストや文書の不足、不適切なAPI形状、非推奨の追加、割り当ての誤用、確信度の高い保守性の問題には警告を使います。 次の項目は報告しません。 - 検証ツールがすでに担う生成マップの欠落、著作権の詳細、コミットの書式。 - ハードウェア`volatile`、プロセス専用ロック、複数セグメント受信、現在のテストマクロの正しい使用。 - 誤りである証拠なしに、何かの確認だけを求める曖昧な依頼。 - 問題のないパッチ、推測に基づくパッチ間の失敗、修正行動を伴わない所見。 実際の各問題では、最初に問題を述べ、影響が明らかでない場合にだけ影響を説明し、具体的な修正方法またはコードの方向性を示します。 提出前に、二つの確認を別々に行います。 1. 正しさの確認: 証明なしに退けられた、正しさに関する妥当な指摘を復元し、リソース経路、有効期間、境界、エラー伝播を追跡したことを確認します。 2. 誤検出の確認: 分析自体が許容可能、正しい、または無害と判断したスタイルや手順の項目をすべて対象から外します。 この確認後に指摘一覧が空でも問題ありません。欄を埋めるためにコメントを作り出してはいけません。