OpenSSL 1.0.0 released [LWN.net]

OpenSSL 1.0.0 released

Posted Mar 30, 2010 6:38 UTC (Tue) by MisterIO (guest, #36192)
In reply to: OpenSSL 1.0.0 released by patrick_g
Parent article: OpenSSL 1.0.0 released

IMO some of his claims seem right, but some others seem excessive. For example, the style is horrible, but isn't that the gnu style, which is (unfortunately) used by a bunch of other projects? Also, another example, is that if(0) used in the error path. It's used, from the code shown, only at the end of the functions, to clean in the error path and it's used so that they don't have to repeat code. It may not be elegant, but you could say the same for the use of goto, even if for cleaning the error path it's very useful.

OpenSSL 1.0.0 released

Posted Mar 30, 2010 8:19 UTC (Tue) by nix (subscriber, #2304) [Link] (1 responses)

The indentation style is most certainly *not* the GNU style. I tried to
provide an example reformatting of his code into the GNU style here, but I
can't stop the comment box eating all leading whitespace.

Among other differences: spaces before parentheses; no spaces in runs of
brackets (what were they *thinking*?); four-level indentation (tab-based
in the real thing) with braces at the two-character point (except for
top-level constructs, where the braces occur in column 0). Much much
easier to read than the OpenSSL abomination.

OpenSSL 1.0.0 released

Posted Mar 30, 2010 9:46 UTC (Tue) by MisterIO (guest, #36192) [Link]

I still see many similarities between the openssl and gnu style(indentation) and frankly I consider the gnu style an abomination in itself. The linux kernel style is so much more readable! That said, the rest of the openssl style is a mess, like the horrible pseudorandom use of capital letters.

OpenSSL 1.0.0 released

Posted Mar 30, 2010 16:48 UTC (Tue) by vonbrand (guest, #4458) [Link] (3 responses)

Where I'd see this kind of construct the err: label and what follows was at the end of the function, after all normal returns. If used consistently, there is no hunting around "where are errors handled" (and can use nesting, various labels which do part of cleanup and fall into the next), no head scratching about a if(0) or other strange constructs.

This is awful...

OpenSSL 1.0.0 released

Posted Mar 30, 2010 18:42 UTC (Tue) by MisterIO (guest, #36192) [Link] (2 responses)

Actually there's nothing strange about it(about that if(0), I mean). I've never used it that way, but I could understand its meaning immediately. Sure, the err label is usually at the end, after the return, but what happens if you need to do the normal cleanup, that you do in the normal path before the return, in the error path too? It happens that you need to repeat code, like this:

normal_cleanup;
return

:err
error_cleanup;
normal_cleanup;

Instead, with that if(0) construct, you can do this:

if (0) {
:err error_cleanup;
}

normal_cleanup;
return;

To me it seems clear and simple. The only problem could be if it screws up some static checker or some software to follow the flow of your code.

OpenSSL 1.0.0 released

Posted Apr 1, 2010 17:47 UTC (Thu) by vonbrand (guest, #4458) [Link] (1 responses)

What is wrong with e.g.

plain_normal_processing;

out:
normal_cleanup;
return;

err:
error_cleanup;
goto out;

BTW, if you worry an if(0) {...} might confuse a static checker or such, it will confuse the random programmer just as much ;-)

OpenSSL 1.0.0 released

Posted Apr 1, 2010 19:09 UTC (Thu) by MisterIO (guest, #36192) [Link]

I have no problems with any of those approaches.I sure never said that was the only one possible, in fact I had already seen the one you propose here. I just said I had never seen the if(0) approach(for this problem) before, yet I could understand its meaning immediately, which for me means that it's simple and clear. That's all.