More than the goto-issue, I believe implicit scoping is the real issue here. This is an excellent example of why it is a good idea to explicitly show the scope of if-else branching and similar constructs. In this case, adding braces after if(…) makes it very clear what scope that second goto belongs to.
if (ssl whatnot)
goto fail;
goto fail;
if (ssl someother)
goto fail;
That looks obviously wrong to me because it’s indented without braces. I’d look closer. The hypothetical braced version
if (ssl whatnot) {
goto fail;
}
goto fail;
if (ssl someother) {
goto fail;
}
That looks less wrong to me. All the code is in the right place. Yeah, there’s a goto in the middle of the function, but it requires actually reading the statement and recognizing it as a goto. The first example doesn’t require any further processing of the statement type. I think the claim that the goto is easy to spot out of place is only because you know it’s a goto and it’s out of place. The original authors and reviewers were not asked “find the goto that’s out of place”. You have the benefit of a lot of hindsight.
if (ssl whatnot) {
goto fail;
}
goto fail;
if (ssl someother) {
goto fail;
}
It’s tempting to believe this would have failed in just the right way for our preferred coding style to prevent it, but that’s not guaranteed. It would be more impressive to find an positive example of a duplicated goto in real code that was rendered harmless by braces.
More than the goto-issue, I believe implicit scoping is the real issue here. This is an excellent example of why it is a good idea to explicitly show the scope of if-else branching and similar constructs. In this case, adding braces after if(…) makes it very clear what scope that second goto belongs to.
I doubt this. At least to me.
That looks obviously wrong to me because it’s indented without braces. I’d look closer. The hypothetical braced version
That looks less wrong to me. All the code is in the right place. Yeah, there’s a goto in the middle of the function, but it requires actually reading the statement and recognizing it as a goto. The first example doesn’t require any further processing of the statement type. I think the claim that the goto is easy to spot out of place is only because you know it’s a goto and it’s out of place. The original authors and reviewers were not asked “find the goto that’s out of place”. You have the benefit of a lot of hindsight.
The point is that the code was probably originally
and someone added another goto to the first branch. If it had originally been
then the same line duplication would result in still-correct code:
Unless the added line went somewhere else:
It’s tempting to believe this would have failed in just the right way for our preferred coding style to prevent it, but that’s not guaranteed. It would be more impressive to find an positive example of a duplicated goto in real code that was rendered harmless by braces.
Yes, because in that case it would not surface as a bug.
This needs to be said more often, louder, and with more conviction. Brace-laziness has caused more errors than any other simple mistake I’ve seen.