On Sat, Feb 22, 2014 at 3:22 PM, Justin Dolske <dol...@mozilla.com> wrote: > On 2/22/14 7:18 AM, Kyle Huey wrote: >> >> If you needed another reason to follow the style guide: >> https://www.imperialviolet.org/2014/02/22/applebug.html > > > I don't really disagree with bracing being a good idea, but I'll be > contrarian here. Mandatory bracing probably wouldn't have helped; since you > could still easily end up with: > > if ((err = SSLHashSHA1.update(&hashCtx, &foo)) != 0) { > goto error; > } > if ((err = SSLHashSHA1.update(&hashCtx, &bar)) != 0) { > goto error; > } > goto error; > if ((err = SSLHashSHA1.update(&hashCtx, &baz)) != 0) { > goto error; > } > > There may even be an argument against bracing, in this specific case, since > it makes the repeated line slightly less visually obvious. I think a remark > about this was made in the recent style guide discussion: when reviewing > code, one often starts off by scanning for abnormal patterns (hence of value > of having consistent style, so "abnormal" isn't conditional on which file > you're in). But I don't really care. I think the correctness-impact is > usually exceedingly rare/minor, and the ensuing style flamewars distract > focus from better solutions. > > For example, _both_ the Apple bug and my example above would have been > flagged by tools that warn about incorrect/misleading indentation. > Unsurprisingly, gps proposed doing this in the recent style guide thread > around automatic brace-insertion (and the risks thereto). Dead/unreachable > code analysis should have caught both as well, even with "correct" > indentation. > > But really, the best way to fix this would be to use a macro: > > err = SSLHashSHA1.update(&hashCtx, &foo); > SSL_ENSURE_SUCCESS(err, err); > err = SSLHashSHA1.update(&hashCtx, &bar); > SSL_ENSURE_SUCCESS(err, err); > err = SSLHashSHA1.update(&hashCtx, &baz); > SSL_ENSURE_SUCCESS(err, err); > > Ok, maybe I'm not being entirely serious. :P > > Justin > > _______________________________________________ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform
Too bad everyone wants to get rid of our error code handling macros! - Kyle _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
