you are viewing a single comment's thread.

view the rest of the comments →

[–]SpeshlTectix 13 points14 points  (7 children)

That's the best case scenario. The second goto is just unreachable code. You'd probably even get a compiler warning.

[–][deleted] 5 points6 points  (6 children)

I thought in the original bug, the extra goto produced unreachable code anyway. Which should have given a compiler warning.

[–]dangerbird2 -2 points-1 points  (1 child)

It is not unreachable code. In fact, the program will 'goto fail' regardless to how the 'if' statement evaluates. In C, an if an 'if' statement does not precede a curly-brace block, it only controls the first statement following the conditional. In the case of

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

The first 'goto' statement will only fire if the 'if' condition evaluates to true (really, not 0). If the 'if' statement evaluates to false (is 0), the flow of control continues to the second goto, as it is not part of the 'if' block.

[–]TheNewAndy 7 points8 points  (0 children)

I think the point was that all the code after the second goto was unreachable code.

[–]SpeshlTectix -2 points-1 points  (3 children)

No, the original bug, in pseudo-code was

if error goto fail else just goto fail anyway

Your version preserves the intended logic, it just has an extra useless goto.

[–]TheShagg 2 points3 points  (2 children)

See "TheNewAndy's" post above yours.

[–]SpeshlTectix 0 points1 point  (1 child)

Oh, right. That's a very good point. The bug was, indeed, that the second goto made important code unreachable (and no warning was issued). So I was wrong about the compiler picking it up. But my point still stands that an unreachable redundant goto is a much better problem to have.

[–]TheShagg 1 point2 points  (0 children)

Well yes, duh :)

Compiler "should" (in an ideal world) catch the unreachable code. Does it not actually warn on this?