「iOS7.0.6で修正された「最悪のセキュリティバグ」はありがちなコーディングミスで発生していた」記事へのコメント

unreachable codeの警告は無視されたのか

[Anonymous Coward]

警告を無視しちゃいかんという教訓を得たと。

こんな感じに、if分を一行にまとめてしまえば、こんな不具合出なかった気がします。

if ((err = SSLHashSHA1.update(&hashCtx, &serverRandom)) != 0) goto fail;
if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0) goto fail;
if ((err = SSLHashSHA1.final(&hashCtx, &hashOut)) != 0) goto fail;

一行が長くなるのを嫌ったのかな？

Re:

[Anonymous Coward]

#define ErrorCheck(f) do { if ((err = f) != 0) goto fail; } while (0)

ErrorCheck(SSLHashSHA1.update(&hashCtx, &serverRandom));
ErrorCheck(SSLHashSHA1.update(&hashCtx, &signedParams));
ErrorCheck(SSLHashSHA1.final(&hashCtx, &hashOut));

ってやりたくなるんだけど、ダメ？

Re:unreachable codeの警告は無視されたのか

[Anonymous Coward]

do while いるの？

Re:unreachable codeの警告は無視されたのか

[ymasa (31598)]

マクロをdo-while(0)で囲むワケ
http://d.hatena.ne.jp/ryochack/20111022/1319252481 [hatena.ne.jp]

Re:

[Anonymous Coward]

do whileがなかったら、
if ( foo == true )
    ErrorCheck()
else
    ErrorCheck()
これが
if ( foo == true )
    if(～) goto fail;
else
    if(～) goto fail;
になってしまう。凡ミスを防ぐための処理。
fooがtrueでない場合にelseのほうを実行してほしいのに、マクロ展開すると、fooがfalseだとなにも実行されないという。
まぁ、これもカッコつけろよって話なんだけど。

Re:

[Anonymous Coward]

doとwhile(0)が必要な理由になってないんだけど・・・

Re:unreachable codeの警告は無視されたのか

[Anonymous Coward]

マクロ関数を普通の関数のように最後に ; を付けて適切に展開されるようにするため。

do while (0) なしだと

if ()
    ErrorCheck(...);
else
    ErrorCheck(...);

が

if ()
    { ... };
else
    { ... };

となり、 if の終わりの {} の後の ; が空文となるので、 else の行で文法エラーとなる。

do while (0) ありだと

if ()
    ErrorCheck(...);
else
    ErrorCheck(...);

が

if ()
    do { ... } while (0);
else
    do { ... } while (0);

と、 if と else が適切に対応する形になるよう展開される。

Re:

[Anonymous Coward]

do~while(0)を使う理由はわかったが、
if、elseには必ず{~}をつける、というルールを徹底した方が良い。
レビューの度にこういう議論をしたくない。

Re:

[Anonymous Coward]

if()
  ErrorCheck()
else
  ErrorCheck()

でおｋ

Re:

[Anonymous Coward]

要は、マクロを思うように展開してくれないけど、それでもマクロを使いたいから余分な処理を付け足したということ？
余分な処理をつけることに罪悪感無いのかなあ。

Re:

[Anonymous Coward]

#2550910だけど、私もそう思う。
if文はカッコがないとコンパイルエラーにしちゃえばいいのにねぇ。

考えてみたら、defineマクロにgoto入れられてたら、do while関係なしに投げ捨てたくなるわ。

スマホで書き込んでるから、説明簡素化しすぎて伝えられなくてごめん。
詳細に書いてくれた人ありがとう。

Re:

[Anonymous Coward]

Cでのイディオムのひとつです。
いわゆるバッドノウハウの最たるものですけど。
余分な処理（）ってwwwww
オプティマイザが消しちゃいますよ。
なので「余分な処理（）」に関しては全然抵抗ありません。

Re:

[Anonymous Coward]

> if文はカッコがないとコンパイルエラーにしちゃえばいいのにねぇ。
#2553818参照

Re:

[Anonymous Coward]

今回のケースでは必要ないけど、#2550910がいうような凡ミスを防ぐために習慣的に付けるよね。多少のタイプ量以上のオーバーヘッドもないし。

Re:

[Anonymous Coward]

後から読むときもループなのか、終了条件は・・・ループじゃねえのか！なんでdo while付いてんだ！ってオーバーヘッドがあるじゃん。
#2550910がいうミスを防ぐなら括弧だけでいいのに

Re:

[Anonymous Coward]

do while (0)は使い古されたテクニックだから、そういうもんだと一度覚えればいいだけだが。
こういうBad Know Howを嫌うのも仕方ないけど、Cだからね。
で、括弧だけではダメな理由は、

if (foo == true)
    ErrorCheck();
else
    ...

がコンパイルエラーになること。やってみそ。

Re:

[Anonymous Coward]

複文からなるマクロはdo while(0)で囲むというのはまあ普通のイディオムではあるのだけど、元コメみたいなアホが理解しないでコピペで使ってどこか別の場所で惨事を引き起すだろうから、やっぱり技巧的なマクロはコーディング規約で使用禁止にすべきかもしれないな。

Re:

[Anonymous Coward]

オーバーヘッドがあるかないかはコンパイラ依存。
do～while(0)の場合に{～}と同じアセンブラを吐け、という規約はないだろ。