14. A good compiler and coding style aren't always enough

We have already spoken about good styles of coding, but this time we'll have a look at an anti-example. It's not enough to write good code: there can be various errors and a good programming style isn't always a cure-all.

The fragment is taken from PostgreSQL. The error is detected by the following PVS-Studio diagnostic: V575 The 'memcmp' function processes '0' elements. Inspect the third argument.

Cppcheck analyzer can also detect such errors. It issues a warning: Invalid memcmp() argument nr 3. A non-boolean value is required.

Datum pg_stat_get_activity(PG_FUNCTION_ARGS)
{
  ....
  if (memcmp(&(beentry->st_clientaddr), &zero_clientaddr,
             sizeof(zero_clientaddr) == 0))
  ....
}

Explanation

A closing parenthesis is put in a wrong place. It's just a typo, but unfortunately it completely alters the meaning of the code.

The sizeof(zero_clientaddr) == 0 expression always evaluates to 'false' as the size of any object is always larger than 0. The false value turns to 0, which results in the memcmp() function comparing 0 bytes. Having done so, the function assumes that the arrays are equal and returns 0. It means that the condition in this code sample can be reduced to if (false).

Correct code

if (memcmp(&(beentry->st_clientaddr), &zero_clientaddr,
           sizeof(zero_clientaddr)) == 0)

Recommendation

It's just the case when I can't suggest any safe coding technique to avoid typos. The only thing I can think of is "Yoda conditions", when constants are written to the left of the comparison operator:

if (0 == memcmp(&(beentry->st_clientaddr), &zero_clientaddr,
                sizeof(zero_clientaddr)))

But I won't recommend this style. I don't like and don't use it for two reasons:

First, it makes conditions less readable. I don't know how to put it exactly, but it's not without reason that this style is called after Yoda.

Second, they don't help anyway if we deal with parentheses put in a wrong place. There are lots of ways you can make a mistake. Here's an example of code where using the Yoda conditions didn't prevent the incorrect arrangement of parentheses:

if (0 == LoadStringW(hDllInstance, IDS_UNKNOWN_ERROR,
        UnknownError,
        sizeof(UnknownError) / sizeof(UnknownError[0] -
        20)))

This fragment is taken from the ReactOS project. The error is difficult to notice, so let me point it out for you: sizeof(UnknownError[0] - 20).

So Yoda conditions are useless here.

We could invent some artificial style to ensure that every closing parenthesis stands under the opening one. But it will make the code too bulky and ugly, and no one will be willing to write it that way.

So, again, there is no coding style I could recommend to avoid writing closing parentheses in wrong places.

And here's where the compiler should come in handy and warn us about such a strange construct, shouldn't it? Well, it should but it doesn't. I run Visual Studio 2015, specify the /Wall switch... and don't get any warning. But we can't blame the compiler for that, it has enough work to do as it is.

The most important conclusion for us to draw from today's post is that good coding style and compiler (and I do like the compiler in VS2015) do not always make it. I sometimes hear statements like, "You only need to set the compiler warnings at the highest level and use good style, and everything's going to be OK" No, it's not like that. I don't mean to say some programmers are bad at coding; it's just that every programmer makes mistakes. Everyone, no exceptions. Many of your typos are going to sneak past the compiler and good coding style.

So the combo of good style + compiler warnings is important but not sufficient. That's why we need to use a variety of bug search methods. There's no silver bullet; the high quality of code can be only achieved through a combination of several techniques.

The error we are discussing here can be found by means of the following methods:

• code review;
• unit-tests;
• manual testing;
• static code analysis;
• etc.

I suppose you have already guessed that I am personally interested in the static code analysis methodology most of all. By the way, it is most appropriate for solving this particular issue because it can detect errors at the earliest stage, i.e. right after the code has been written.

Indeed, this error can be easily found by such tools as Cppcheck or PVS-Studio.

Conclusion. Some people don't get it that having skill isn't enough to avoid mistakes. Everyone makes them - it's inevitable. Even super-guru make silly typos every now and then. And since it's inevitable, it doesn't make sense blaming programmers, bad compilers, or bad style. It's just not going to help. Instead, we should use a combination of various software quality improving techniques.