Never dereference null pointers

This bug was found in GIT‘s source code. The code contains an error that analyzer diagnoses in the following way: V595 The ‘tree’ pointer was utilized before it was verified against nullptr. Check lines: 134, 136.

void mark_tree_uninteresting(struct tree *tree)
{
  struct object *obj = &tree->object;
  if (!tree)
    return;
  ....
}

Explanation

There is no doubt that it’s bad practice to dereference a null pointer, because the result of such dereferencing is undefined behavior. We all agree about the theoretical basis behind this.

But when it comes to practice, programmers start debating. There are always people who claim that this particular code will work correctly. They even bet their life for it – it has always worked for them! And then we have to give more reasons to prove my point.

скачанные файлы (4)

We have deliberately chosen such an example that will provoke more discussion. After the tree pointer is dereferenced, the class member isn’t just using, but evaluating, the address of this member. Then if (tree == nullptr), the address of the member isn’t used in any way, and the function is exited. Many consider this code to be correct.

But it is not so. You shouldn’t code in such a way. Undefined behavior is not necessarily a program crash when the value is written at a null address, and things like that. Undefined behavior can be anything. As soon as you have dereferenced a pointer which is equal to null, you get an undefined behavior. There is no point in further discussion about the way the program will operate. It can do whatever it wants.

One of the signs of undefined behavior is that the compiler can totally remove the “if (!tree) return;” – the compiler sees that the pointer has already been dereferenced, so the pointer isn’t null and the compiler concludes that the check can be removed. This is just one of a great many scenarios, which can cause the program to crash.

We recommend having a look at the article where everything is explained in more details: http://www.viva64.com/en/b/0306/

Correct code

void mark_tree_uninteresting(struct tree *tree)
{
  if (!tree)
    return;
  struct object *obj = &tree->object;
  ....
}

Recommendation

Beware of undefined behavior, even if it seems as if everything is working fine. There is no need to risk that much. It’s hard to imagine how it may show its worth. Just try avoiding undefined behavior, even if it seems like everything works fine.

One may think that he knows exactly how undefined behavior works. And, he may think that this means that he is allowed to do something that others can’t, and everything will work. But it is not so.

Written by Andrey Karpov.
This error was found with PVS-Studio static analysis tool.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s