Always Use Braces (and BSD KNF Style)

23 Feb 2014

There’s been plenty of coverage of the serious SSL issue that was identified in Apple’s iOS/MacOS stack.

From ImperialViolet, here’s the bug:

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

That second goto fail; is the problem. Since those if clauses aren’t wrapped in braces, if you ever get pat the first two if statements, you’ll always hit the second goto fail;, leading to the error.

On our development team, and in my personal programming, I’m a pretty big believer in wrapping your control statements in braces, and in having the braces on the same line as the control statement. I’m a fan of something close to the BSD KNF Style, using the cuddled else, which looks something like this:

if (foo) {
    stuff;
} else {
    other stuff;
}

The reason I’m a fan of this is two-fold. First, you have braces around your control statements, so you’ll avoid that Apple bug above, and it’s clear what block your code belongs to. Second, and this is why I’m keen on this particular style: you won’t accidentally break up joined control statements.

For example, if you write your code without the cuddled else …

if (foo) {
    stuff;
}
else {
    other stuff;
}

that’s generally no big deal. Until someone isn’t paying attention and breaks up your if/else block, or drops a line of code in there. Now, at best, you’ve got a compilation error. In the worst case, you end up with random code executing (because maybe someone adds a different if statement in between).

Coding styles and guidelines aren’t a replacement for good code review or QA, but they can help to identify or flag issues like this before they slip through.

TNSFF

Always Use Braces (and BSD KNF Style)