Finding More Than One Worm in the Apple

http://queue.acm.org/detail.cfm?id=2620662

by dkav • 12 years ago

34 7 12 years ago

queue.acm.org

stuart_v• 12 years ago

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

tempodox• 12 years ago

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.

sanxiyn• 12 years ago

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• 12 years ago

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• 12 years ago

"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)

gissolved• 12 years ago

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

twic• 12 years ago

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.

Finding More Than One Worm in the Apple

Related Stories