From a different point of view, part of the problem is the undefined state of the code at the "fail" label.
Execution will arrive there somehow, but the 'how' is unclear. The word "fail" implies you should reach that point only if there was an error, but that is a bad assumption in this case.
If the real answer to 'how did we get here?' was checked, then the bug could not hide in the undefined behavior. This would not allow a dangling goto to result in a false positive. A false negative will get someone's attention when their web page doesn't load.
Something like this could remove the undefined state:
goto pass;
fail:
if ( err == 0 ) {
assert( err != 0 ); // BUG! variable must contain an error number
err = kSomeAppropriateErrorNumber;
}
pass:
SSLFreeBuffer(&signedHashes);
SSLFreeBuffer(&hashCtx);
return err;
Haha love the whitespace comment. This also makes you think of the static source-code analysis done at Apple. Surely static tools would have picked this up, no...?
Clang currently does not warn about this, but I'd wager that Xcode will boast a feature in the next version that detects dead code due to early returns/gotos.
Clang may not warn about white space issues, but it certainly does warn about unreachable code if -Wunreachable is on. This code would not compile in my project because the unconditional goto leaves the following code unreachable.
