

Finding More Than One Worm in the Apple - dkav
http://queue.acm.org/detail.cfm?id=2620662

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

[https://archive.eiffel.com/doc/manuals/technology/contract/a...](https://archive.eiffel.com/doc/manuals/technology/contract/ariane/)

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.

------
sanxiyn
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.

~~~
josteink
The C# compiler has this sort of warning on by default.

Unfortunately, it warns about lots of things which the "lazier" developers
really can't be arsed to do (like documenting the public methods and classes
in their APIs).

The result is that in a team where not everyone is vigilant, a bigger project
is doomed to have a compiler output with 100s and 100s warnings about things
left and right.

And the result is that almost everyone ignores the compiler warnings, even
when it tells you about things which are legitimate errors.

In short: The noise is a feature but is treated like a bug. The result of that
is buggy code. I'm not sure how or if such a thing can be fixed by technology
alone. A major shift in culture is probably also needed.

~~~
duncans
> "it warns about lots of things ... like documenting the public methods and
> classes in their APIs"

Only if you're switched on the Generate XML Documentation switch `/doc` (in
which case I think it makes sense).

Most places I've worked have /warnaserror as a standard part of the build so
that the build fails if there are any warnings (and they can be `#pragma
nowarn`-ed out)

------
tempodox
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.

------
stuart_v
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

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

