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.
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.
> "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)
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.
8 comments
[ 2.9 ms ] story [ 29.7 ms ] threadIn 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
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.
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.
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)
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.