“Goto fail” would have been wrong in a non-goto language that use RAII.
The problem was not goto, it was that the else path was also “give up”. In a non goto/RAII/defer language it would have been something like
If (error)
Return
Return
My assumption has been this was some kind of merge error rather being wrong off the bay. Interestingly mandatory indenting or mandatory braces might have stopped this, but then i would have thought -Werror with the dead code warnings would have as well :-/
But again the error was not the goto, and believing it was is the exact problem the article is talking about: people are so opposed to goto they are unable to see real issues (I have seen C code that tries to avoid goto completely and error handling code becomes horrific, far more complex, and far more error prone that just using goto). The problem with goto is that it is very easy to use it unnecessarily, in ways that complicated control flow but don’t actually make things better.
For the curious this flag has been available since GCC 6.1 (2016) and Clang 10 (2020). It is enabled by -Wall for both compilers.
Interestingly (or perhaps coming full circle), the bug reference by the GGP comment is also called out in the GCC 6.1 release notes:
> -Wmisleading-indentation warns about places where the indentation of the code gives a misleading idea of the block structure of the code to a human reader. For example, given CVE-2014-1266
sslKeyExchange.c: In function 'SSLVerifySignedServerKeyExchange':
sslKeyExchange.c:629:3: warning: this 'if' clause does not guard... [-
Wmisleading-indentation]
if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
^~
sslKeyExchange.c:631:5: note: ...this statement, but the latter is misleadingly indented as if it is guarded by the 'if'
goto fail;
^~~~
Lack of code review and testing always seemed like the most pressing problems with that; both of which should have caught this error. Yes, it probably also should have had braces, but that seems like the lesser issue.
That error is not due to goto, it was just a goto which was errously executed because of badly formatted code. (It looked like the statement was inside an if-block due to the indent.)
Pyhon would have prevented this bug, but so would a formatter. Rust also requires braces for if-blocks to prevent this kind of error.
Using a language feature that is a known footgun (`if ...` instead of `if {...}`) without being cautious enough to avoid shooting yourself in the foot is not the fault of the footgun, it's the fault of the programmer.
Additionally, in the above linked case, the problem isn't a misused `goto`, it's a misused `if ...`. It would be just as problematic if they typed `cleanup_context();` instead of `goto fail;`, but nobody complains about cleaning up state, do they?
that one in particular was not really caused by goto, but rather by braceless if statements, it'd be a vulnerability all the same if the line was a "fail" function that was called instead of a goto.
