パスワードを忘れた? アカウント作成
この議論は賞味期限が切れたので、アーカイブ化されています。 新たにコメントを付けることはできません。

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

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

    こんな感じに、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;

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

    • by Anonymous Coward on 2014年02月24日 18時40分 (#2550889)

      #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));

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

      親コメント
      • by Anonymous Coward

        do while いるの?

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

          親コメント
        • by 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だとなにも実行されないという。
          まぁ、これもカッコつけろよって話なんだけど。

          • by Anonymous Coward

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

            • by Anonymous Coward on 2014年02月24日 19時34分 (#2550949)

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

              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 が適切に対応する形になるよう展開される。

              親コメント
              • by Anonymous Coward

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

              • by Anonymous Coward

                if()
                  ErrorCheck()
                else
                  ErrorCheck()

                でおk

              • by Anonymous Coward

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

              • by Anonymous Coward

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

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

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

              • by Anonymous Coward

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

              • by Anonymous Coward

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

        • by Anonymous Coward

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

          • by Anonymous Coward

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

            • by Anonymous Coward

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

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

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

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

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

      • by Anonymous Coward

        コードの見通しがすごく悪くない?

        • by Anonymous Coward

          個人的にgotoをマクロで隠さないでほしい、セットで使うようなdefineならわかるけど。
          ここだと短縮されてるわけでもないし、見やすくなってるとも思えない。
          オプションでマクロを切り替えるとかでもなさそうだし、マクロにするのはやりすぎに感じる。

      • by Anonymous Coward

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

        • by 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
          # 末尾に;つけないでおくのがポイント
          • by kr (10950) on 2014年02月24日 20時55分 (#2551020) 日記

            #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)で囲めば、このような副作用が無くなるので、より安全なマクロ定義となります。

            親コメント
          • by Anonymous Coward

            呼び出し元のスコープに影響与えるマクロの癖に関数と同じように使おうというのが間違いかなぁと思います。
            やっぱりC言語とそのマクロは滅びるべき

          • by Anonymous Coward

            こういう生半可を産んでしまうので、やはりマクロは鬼門ですな。
            constやインライン関数など、マクロを使わずにすむ仕掛けを使いたいところだけど、今回のようなものはそれらじゃ書けない。
            マクロが悪いのか、gotoが悪いのか...

      • by Anonymous Coward

        だめ、ぜったい。
        これは、エラーチェックだけしてるんじゃなくて、
        実行してからエラーだったらgoto failしてるんだから、
        もっとまともな名前にすべき。

        do-whileのテクニックで云々している上級者の方々もそこんとこよろしく。

        • by Anonymous Coward

          単文でも必ずブロック化しろってのがマイジャスティス。
          そもそもifの()内で関数呼び出しして、しかも真偽判定用の式内で代入してる時点で超気持ち悪いです。

          これ、条件式が追加される仕様変更が来たときに SSLHashSHA1.update関数とかが呼ばれなくなる可能性があるから書き直しになるんですが。

      • by Anonymous Coward

        このコードはCではなくてC++なのでは。SSLHashSHA1.updateはメンバ関数を呼び出している様に見えるけど
        同じディレクトリにcppファイルもあるみたいだし。
        C++11以降が使える処理系なら、こんなふうにgotoを使わずにラムダ式でreturnしたほうがいいのでは。
        int err
        const bool flag=[&](){
                        if((err=SSLHashSHA1.update(&hashCtx, &serverRandom))!=0){return false;}
                        if((err=SSLHashSHA1.update(&hashCtx, &signedPa

        • "."が使われている場合でも、C++のメンバ関数ではなく
          構造体のメンバに関数ポインタがあり、それを呼び出しているだけの可能性もあるので
          呼び出し側だけではC++noコードだとは判断が付きません。

          その構造体の定義は見つける前にGive upしましたが、関数ポインタをメンバに持つ似たような構造体定義は同じディレクトリの
          sslTypes.h [apple.com]の下の方にもあります。

          親コメント
        • by Anonymous Coward

          せっかく劣ったコンパイラを考慮する必要のない環境なのにねえ。

吾輩はリファレンスである。名前はまだ無い -- perlの中の人

処理中...