Saturday, October 15, 2011

The Trouble With Lint

You all know what lint is; those little bits of white fuzz that stick to your clothes. Lint isn't exactly dirt. It mostly just makes your clothes look untidy. Mostly.

Software has lint too; those compiler warnings you see every time you do a clean build. You've seen them a thousand times before. You know they're harmless. If they're in your code, maybe you think, "I need to fix that someday. But not now."

This posting is about why it is important to remove lint from your software sooner rather than later.

You see, lint has a dark side. Enough lint in the same place is flammable; sometimes even explosive. Houses burn down each year from fires that start in the clothes dryer.

Software lint has the same nature. Too much lint; too many warning messages; and you stop paying attention. Then when an important warning appears, you don't notice. At a previous employer, a team of half a dozen engineers spent four 16-hour days (including Saturday and Sunday) hunting a bug that held up an urgently needed release, when the problem they were hunting was printing a visible compiler warning. It was one of 200 such warnings, so nobody paid any attention.

I was going through some code today, removing lint. I came across this warning.
Foo_UnitTest.h:178: warning: right-hand operand of comma has no effect
Anyone know what this means? Here's a hint; the code at line 178 read
delete [] buf1, buf2;
Still confused? Here's an explanation. C and C++ share a little-known feature called the comma operator. The expression <expr1> , <expr2> evaluates <expr1>, then throws the result away (except for side effects), then evaluates <expr2>.

I imagine the person who wrote this line of code intended to delete the storage pointed to by both buf1 and buf2, but that's not what happened. Instead, the expression delete [] buf1 was evaluated, then discarded (except for the side effect of deleting buf1), and then the expression buf2 was evaluated. So buf2 didn't get deleted.

If you say, "Well, it doesn't really matter because it's just a unit test." then you are entirely missing the point. Removing lint is a discipline that results in clean code that runs the way you expect. If buf2 has a non-trivial destructor, there's no telling how much code isn't getting run (and therefore isn't getting tested) because buf2 isn't getting deleted. You wouldn't know that without investigating what the type of buf2 was.

Even lint that doesn't affect the generated code, like unused variables, can be a problem if these unused variables confuse a future reader of the code. It's easy to comment out this code if you want it to stay in the listing. Easier yet to delete it.

C++ provides syntax for defining functions that don't use all their arguments.
void classname::func ( int parm1, int /*parm2*/ ) {...}
defines a method that takes two int arguments, but only uses the first one. Sometimes you need this for defining base-class methods meant to be overridden in derived classes. Commenting out the argument name turns off the compiler warning, and clearly expresses your intent.

Sometimes your compiler insists on issuing a warning for code that you know is ok. If you can't fix the code with an explicit cast or something, the language implementation probably provides a #pragma for temporarily turning off the warning. You can go even further, running a static checker like lint(1) or cppcheck and fixing the code to remove the warnings it produces. There's just no reason not to have a clean build, and every reason to have one.