33. Never dereference null pointers

This bug was found in GIT's source code. The code contains an error that PVS-Studio 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 I have to give more reasons to prove my point. That's why this article topic is another attempt to change their mind.

I 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.

I 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. As I have already written, 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. The next section is to underline the fact that undefined behavior is really dangerous.