[Cryptography] The GOTO Squirrel! [was GOTO Considered Harmful]

[Patrick Chkoreff]

Larry Bisel (lbisel) wrote, On 02/28/2014 11:07 PM:

> 1)  The original poster took code that compiled, and replaced it with
> code that (as far as I can tell) would not even compile.  In C, "||"
> is a logical operation, and "|" is a bit-wise operation.  From my 25+
> years of writing C code, I have never encountered an "||=" operator,
> and none of my references mention such an operator.  So, that leads
> me to my favorite rant about refactoring:  DON'T REFACTOR CODE JUST
> TO REPLACE ONE SET OF PROBLEMS WITH ANOTHER SET OF PROBLEMS!

Yeah I still had my Perl hat on when I wrote it with ||=.  I've since
removed the ||= unicorn, as you can now see:
http://fexl.com/goto-considered-harmful.  I also updated the commentary
to reflect what I've heard here on this list, including a fourth section
at the bottom with Jerry Leichter's excellent suggestion about "cleanup"
versus "fail" labels.


> To summarize, refactoring code is a very complex issue, and it should
> not be taken lightly!

Heh, my only mistake was to use Perl's "||=" operator, which doesn't
exist in C.  Other than that, it was great!  ;) ;)

Of course I didn't actually bother trying to compile or test the code,
because there's too much context to drag in.  But there's no more
fictional unicorn in the code now.

I've also rewritten my post to emphasize that the lack of a unit test
was the real problem.  Also, to those on this list who think it might be
"too hard" to test that call to sslRawVerify, I would say that the
developers need to buck up and *do it anyway*.  In fact, if it's
actually really really hard to concoct a test for a signed server key
exchange, then I view that as a very very bad "code smell" right there
-- and some serious refactoring is in order so that it becomes *easy* to
test.  Mission critical functions should be *easy* to test, with high
observability, low probability of regression, and with strategically
placed internal redundancy.

As for "goto" itself, my biggest beef with it is that it makes code
harder to refactor.  If I have a piece of code that I want to make into
a separate function, that's usually pretty easy, but if there's a "goto"
buried in the middle of it, then that's it, I'm pretty much stuck until
I eliminate the goto.

Also, regarding refactoring, I have found that to be immensely
illuminating.  With systematic refactoring, I can expose defective or
missing logic in record time.  I do this mercilessly to my own code,
always becoming my own bitter antagonist the instant I add anything new.
 That's because the refactoring process *is* a proof process.

However, I do understand the "minimal diff" constraint in many shared
projects, and if I were on a team I wouldn't do anything massive, willy
nilly.  But also if I were on a team doing security code, the instant
anyone on the team whined about some critical code being "too hard to
test", I'd hand him a Kleenex and tell him to get to work.


-- Patrick