「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:

[Anonymous Coward]

いまだにdoとwhile(0)を使う理由がわからない。
{~}だけでよいのでは。

Re:

[Anonymous Coward]

do while(0)は複文を単文にするためのテクニック。
括弧だけだとif文に後続する複文になってしまうから、do while(0)で囲んで単文にすることで、括弧なしのif文で使ったとき正しく展開されるようになる。
ただしif文そのものは単文だから、今回のif ((err = f) != 0) goto failはdo while(0)で囲む必要は全くない。
#define ErrorCheck(f) if ((err = f) != 0) goto fail
でおk
# 末尾に;つけないでおくのがポイント

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

[kr (10950)]

#define ErrorCheck(f) if ((err = f) != 0) goto fail
でおk

それは潜在的に危険なコードです。

ErrorCheck()を使う側で、以下ようなコードを書くと、

if (.../*条件A*/)
        ErrorCheck(...);
else
        .../*処理B*/;

この見た目と違って、実際のマクロ展開は(改行と字下げを付けると)、

if (.../*条件A*/)
        if ((err = ...) != 0)
                goto fail;
        else
                .../*処理B*/;

となり、「処理B」が「条件A」のelse部でなく、「(err = ...) != 0」のelse部にくっついてしまいます。

else節は最も近いif文にくっつくという構文規則になっているので、 マクロにelse節無しの剥き出しのif文を埋め込むのは危険なのです。

do〜while(0)で囲めば、このような副作用が無くなるので、より安全なマクロ定義となります。