これは何？
クソコードお焚き上げの会 Advent Calendar 2023 に参加するために書いた記事。
を見て、書こうと思った。
事例
ぼかすけど。
まあ下記のようなコード。本物は関数名こそちがうけど、まったく同じ趣旨。
C言語
/* a[i] と a[j] を入れ換える */
void FooSwapInt( int * a, int i, int j ){
int tmp = a[i];
a[j] = tmp;
a[i] = a[j];
}
経緯
私は、ある装置の機能追加のために参加した。
前バージョンは出荷済み。
私がやるべき仕事に上記の関数の周辺はあまり関係なかったんだけど、コード全体の量がそれほど多くなかったのと、なんとなく見ておいたほうが良いという予感があり。
頼まれてもいないのに全体を眺めてたら発見した。
たいへん驚いた。
何がまずいのか
関数名もコメントも値の入れ替えを意図しているように見えるし、中身も一時変数を使った入れ替えのコードによく似ている。
しかし、よく見ると分かるが、上記の計算では入れ替わらない。
酷い。
しかも、この関数は何箇所かで呼ばれていた。
なぜ生き残っていたのか
ここまで酷いと、なぜこのコードが生き残っていたのかが気になる。
入れ替えたくてこれを呼んでも入れ替わらないので、致命的な不具合として観測できそうなものだ。
調査した結果。
前述の通り、このコードを呼んでいる場所は何箇所かあったものの。
それぞれの箇所の流れを追うと、結局 FooSwapInt は一度も呼ばれておらず、装置の動作に影響を与えていなかった。
Swap 操作をする関数は他にも何個かあり (!)、本当に Swap が行われる場所は FooSwapInt 以外の関数で Swap 操作をしていた。
まとめ
このコードが発見されたことで、コードレビューがまともに機能していなかったことがわかった。
こんなに明瞭な間違いが出荷済みのコードの一部として発見されるという体験で、たいへん驚いた。