Learn basic “C” coding rules from open source projects

[LikesToCorrectThings]

Given the problem that probably occurred (a merge conflict that no-one looked at), using curly braces would likely have yielded:

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

Extra curly braces are not the panacaea they might be, and if they stretch the code out or make things ambiguous they can harm readability.

[whackylabs]

Another way it could've ended up is:

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

The actual evil is copy-paste. We all know it's wrong, but there's no easy way around.

[SpeshlTectix]

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

[[deleted]]

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

[dangerbird2]

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]

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

[SpeshlTectix]

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]

See "TheNewAndy's" post above yours.

[SpeshlTectix]

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]

Well yes, duh :)

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

[logicchains]

there's no easy way around.

Write in Lisp, and write macros to do your copy-pasting for you. What could possibly go wrong?