Finding More Than One Worm in the Apple

This reminds me of Bertrand Meyer's article on the Ariane 501 failure:

https://archive.eiffel.com/doc/manuals/technology/contract/a...

In that it is not an honest attempt at an inquiry into a problem, it's the use of a problem as a public whetstone on which to grind one's favourite axe.

Mr Bland's axe is unit testing:

This duplication is the critical factor leading to the manifestation of the vulnerability, and it can be traced directly to a lack of unit testing discipline — because of the absence of the craftsmanship and design sense that testable code requires. Someone writing code with unit testing in mind would have ensured only one copy of the algorithm existed—not only because it's theoretically more proper, but because it would have been easier to test.

Which is double nonsense. Firstly, because writing unit tests simply don't force you not to duplicate code like this. I spend day after day staring at a thoroughly unit-tested codebase which contains duplication like this, along with manifest other crimes. Secondly, because you don't need a unit test to force you not to duplicate code like this, you just need to be a competent, conscientious programmer.

Don't get me wrong, i am a huge fan of testing. This code should have had tests, without doubt. But please don't delude yourself into thinking that unit testing is going to lead to high-quality code. If you have sub-par programmers, or decent programmers in a sub-par culture, they will write low-quality code, whether they write unit tests for it or not.

One interesting point is that "goto fail" in this case causes dead code. Shouldn't compilers warn about dead code?

There is -Wunreachable-code. GCC used to have it, but removed it. Clang has it, but does not enable it (you specifically need to request it). Apparentlly it causes too much noises to enable by default.

Aside from the problems discussed in the article, don't define a C macro like this:

  #define FOOBAR  { foo(); bar(); }

but do it like that:

  #define FOOBAR  do { foo(); bar(); } while (0)

The latter is “C syntax safe”, the former is not.

Might be off topic here. But one thing I like about language like Go over C is that it enforces a strict syntax.

In Go, {} are not optional in a if statement so you will never have issue like this:

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

They will be easily caught

I would guess that the acm could do better then showing me a 500 error + a full stack trace of a ServletException