When should pointers be checked for NULL in C?

Question

Summary:

Should a function in C always check to make sure it is not dereferencing a NULL pointer? If not when is it appropriate to skip these checks?

Details:

I've been reading some books about programming interviews and I'm wondering what the appropriate degree of input validation for function arguments in C is? Obviously any function that takes input from a user needs to perform validation, including checking for a NULL pointer before dereferencing it. But what about in the case of a function within the same file that you don't expect to expose through your API?

For example the following appears in the source code of git:

static unsigned short graph_get_current_column_color(const struct git_graph *graph)
{
    if (!want_color(graph->revs->diffopt.use_color))
        return column_colors_max;
    return graph->default_column_color;
}

If *graph is NULL then a null pointer will be dereferenced, probably crashing the program, but possibly resulting in some other unpredictable behavior. On the other hand the function is static and so maybe the programmer already validated the input. I don't know, I just selected it at random because it was a short example in an application program written in C. I've seen many other places where pointers are used without checking for NULL. My question is general not specific to this code segment.

I saw a similar question asked within the context of exception handing. However, for a unsafe language such as C or C++ there is no automatic error propagation of unhandled exceptions.

On the other hand I have seen lots of code in open source projects (such as the example above) that does not do any checks of pointers before using them. I'm wondering if anyone has thoughts on guidelines for when to put checks in a function vs. assuming that the function was called with correct arguments.

I'm interested in this question in general for writing production code. But I'm also interested within the context of programming interviews. For instance many algorithm textbooks (such as CLR) tend to present the algorithms in pseudocode without any error checking. However, while this is good for understanding the core of an algorithm it's obviously not a good programming practice. So I would not want to tell an interviewer that I was skipping error checking to simplify my code examples (as a textbook might). But I also would not want to appear to produce inefficient code with excessive error checking. For instance the graph_get_current_column_color could have been modified to check *graph for null but its not clear what it would do if *graph was null, other than it should not dereference it.

Answer 1

Invalid null pointers can either be caused by programmer error or by runtime error. Runtime errors are something a programmer can't fix, like a malloc failing due to low memory or the network dropping a packet or the user entering something stupid. Programmer errors are caused by a programmer using the function incorrectly.

The general rule of thumb I've seen is that runtime errors should always be checked, but programmer errors don't have to be checked every time. Let's say some idiot programmer directly called graph_get_current_column_color(0). It will segfault the first time it's called, but once you fix it, the fix is compiled in permanently. No need to check every single time it's run.

Sometimes, especially in third party libraries, you'll see an assert to check for the programmer errors instead of an if statement. That allows you to compile in the checks during development, and leave them out in production code. I've also occasionally seen gratuitous checks where the source of the potential programmer error is far removed from the symptom.

Obviously, you can always find someone more pedantic, but most C programmers I know favor less cluttered code over code that is marginally safer. And "safer" is a subjective term. A blatant segfault during development is preferable to a subtle corruption error in the field.

Answer 2

Kernighan & Plauger, in "Software Tools", wrote that they would check everything, and, for conditions that they believed could in fact never happen, they would abort with an error message "Can't happen".

They report being rapidly humbled by the number of times they saw "Can't happen" come out on their terminals.

You should ALWAYS check the pointer for NULL before you (attempt to) dereference it. ALWAYS. The amount of code you duplicate checking for NULLs that don't happen, and the processor cycles you "waste", will be more than paid for by the number of crashes you don't have to debug from nothing more than a crash dump - if you're that lucky.

If the pointer is invariant inside a loop, it suffices to check it outside the loop, but you should then "copy" it into a scope-limited local variable, for use by the loop, that adds the appropriate const decorations. In this case, you MUST ensure that every function called from the loop body includes the necessary const decorations on the prototypes, ALL THE WAY DOWN. If you don't, or can't (because of e.g. a vendor package or an obstinate coworker), then you must check it for NULL EVERY TIME IT COULD BE MODIFIED, because sure as COL Murphy was an incurable optimist, someone IS going to zap it when you aren't looking.

If you are inside a function, and the pointer is supposed to be non-NULL coming in, you should verify it.

If you are receiving it from a function, and it is supposed to be non-NULL coming out, you should verify it. malloc() is particularly notorious for this. (Nortel Networks, now defunct, had a hard-and-fast written coding standard about this. I got to debug a crash at one point, that I traced back to malloc() returning a NULL pointer and the idiot coder not bothering to check it before he wrote to it, because he just KNEW he had plenty of memory... I said some very nasty things when I finally found it.)

Answer 3

Let me add one more voice to the fugue.

Like many of the other answers, I say--don't bother checking at this point; it's the caller's responsibility. But I have a foundation to build on rather than simple expediency (and C programming arrogance).

I try to follow Donald Knuth's principal of making programs as fragile as possible. If anything goes wrong, have it crash big, and referencing a null pointer is usually a good way to do that. The general idea is a crash or an infinite loop is far better than creating wrong data. And it gets programmers' attention!

But referencing null pointers (especially for large data structures) does not always cause a crash. Sigh. That's true. And that's where Asserts fall in. They are simple, can instantly crash your program (which answers the question, "What should the method do if it encounters a null?"), and can be turned on/off for various situations (I recommend NOT turning them off, as it's better for customers to have a crash and see a cryptic message than have bad data).

That's my two cents.

Answer 4

You can skip the check when you can convince yourself somehow that the pointer cannot possibly be null.

Usually, null pointer checks are implemented in code in which null is expected to appear as an indicator that an object is currently not available. Null is used as a sentinel value, for instance to terminate linked lists, or even arrays of pointers. The argv vector of strings passed into main is required to be null-terminated by a pointer, similarly to how a string is terminated by a null character: argv[argc] is a null pointer, and you can rely on this when parsing the command line.

while (*argv) {
   /* process argument string *argv */
   argv++; /* increment to next one */
}

So, the situations for checking for null are those in which a it is an expected value. The null checks implement the meaning of the null pointer, such as stopping the search of a linked list. They prevent the code from dereferencing the pointer.

In a situation in which a null pointer value is not expected by design, there is no point in checking for it. If an invalid pointer value arises, it will quite likely appear non-null, which cannot be distinguished from valid values in any portable way. For instance, a pointer value obtained from reading uninitialized storage interpreted as a pointer type, a pointer obtained via some shady conversion, or a pointer incremented out of bounds.

About a data type such as graph *: this could be designed so that a null value is a valid graph: something with no edges and no nodes. In this case, all the functions that take a graph * pointer will have to deal with that value, since it is a correct domain value in the representation of graphs. On the other hand, a graph * could be a pointer to a container-like object which is never null if we hold a graph; a null pointer then might tell us that "the graph object is not present; we did not allocate it yet, or we freed it; or this currently has no associated graph". This latter use of pointers is a combined boolean/satellite: the pointer being non-null indicates "I have this sister object", and it provides that object.

We might set a pointer to null even if we are not freeing a object, simply to dissociate one object from another:

tty_driver->tty = NULL; /* detach low level driver from the tty device */

Answer 5

In my opinion validating inputs (pre/post-conditions, i.e.) is a good thing to detect programming errors, but only if it results in a loud and obnoxious, show-stopping errors of a kind which cannot be ignored. assert typically has that effect.

Anything falling short of this can turn into a nightmare without very carefully-coordinated teams. And of course ideally all teams are very carefully-coordinated and unified under tight standards, but most environments I've worked in fell far short of that.

Just as an example, I worked with some colleagues that believed that one should religiously check for the presence of null pointers, so they sprinkled a lot of code like this:

void vertex_move(Vertex* v)
{
     if (!v)
          return;
     ...
}

... and sometimes just like that without even returning/setting an error code. And this was in a codebase which was several decades old with many acquired third party plugins. It was also a codebase plagued with many bugs, and often bugs which were very difficult to trace down to root causes since they had a tendency to crash in sites far removed from the immediate source of the problem.

And this practice was one of the reasons why. It's a violation of an established pre-condition of the above move_vertex function to pass a null vertex to it, yet such a function just silently accepted it and did nothing in response. So what tended to happen was that a plugin might have a programmer mistake which causes it to pass null to said function, only to not detect it, only to do many things afterwards, and eventually the system would start flaking out or crash.

But the real issue here was the inability to easily detect this problem. So I once tried to see what would happen if I turned the analogical code above to an assert, like so:

void vertex_move(Vertex* v)
{
     assert(v && "Vertex should never be null!");
     ...
}

... and to my horror, I found that assertion failing left and right even upon starting up the application. After I fixed the first few call sites, I did some more things and then got a boatload more assertion failures. I kept going until I had modified so much code that I ended up reverting my changes because they had become too intrusive and begrudgingly kept that null pointer check, instead documenting that the function allows accepting a null vertex.

But that's the danger, albeit a worst-case scenario, of failing to make violations of pre/post-conditions easily detectable. You can then, over the years, silently accumulate a boatload of code violating such pre/post-conditions while flying under the radar of testing. In my opinion such null pointer checks outside of a blatant and obnoxious assertion failure can actually do far, far more harm than good.

As to the essential question of when you should check for null pointers, I believe in asserting liberally if it's designed to detect a programmer error, and not letting that go silent and hard to detect. If it's not a programming error and something beyond the programmer's control, like an out of memory failure, then it makes sense to check for null and use error handling. Beyond that it's a design question and based on what your functions consider to be valid pre/post conditions.

Answer 6

I generally only check when a pointer is assigned, which is generally the only time I can actually do something about it and possibly recover if it is invalid.

If I get a handle to a window for example, I'll check for it being null right and then and there, and do something about the null condition, but I'm not going to check for it being null each and every single time I use the pointer, in each and every function the pointer is passed to, otherwise I'd have mountains of duplicate error handling code.

Functions like graph_get_current_column_color is probably completely unable to do anything useful to your situation if it does encounter a bad pointer, so I'd leave checking for NULL to its callers.

Answer 7

I would say that it depends on the following:

1. Is CPU utilization critical? Every check for NULL takes some amount of time.
2. What are the odds that the pointer is NULL? Was it just used in a previous function. Could the value of the pointer have been changed.
3. Is the system preemptive? Meaning could a task change happen and change the value? Could an ISR come in and change the value?
4. How tightly coupled is the code?
5. Is there some sort of automatic mechanism that will check for NULL pointers automatically?

CPU Utilization/Odds Pointer is NULL Every time you check for NULL it takes time. For this reason I try to limit my checks to where the pointer could have had its value changed.

Preemptive System If your code is running and another task could interrupt it and potentially change the value a check would be good to have.

Tightly Coupled Modules If the system is tightly coupled then it would make sense that you have more checks. What I mean by this is if there are data structures that are shared between multiple modules one module might change something out from under another module. In these situations it makes sense to check more often.

Automatic Checks/Hardware Assist The last thing to take into account is if the hardware that you are running on has some sort of mechanism that can check for NULL. Specifically I am referring to Page Fault detection. If you system has page fault detection the CPU itself can check for NULL accesses. Personally I find this to be the best mechanism since it always runs and does not rely on the programmer to put in explicit checks. It also has the benefit of practically zero overhead. If this is available I recommend it, debugging is a little harder but no overly so.

To test if it is available create a program with a pointer. Set the pointer to 0 and then try to read/write it.

Answer 8

The Null pointer check is good for un-allocated data as the allocation methods return zero if they fail. It's not much use for random garbage of unset pointers or data overwrites.

If the system supports it, find the actual memory range and check for the pointer being in that range, or ranges. If the target is an embedded device (microcontroller) the RAM and Flash address ranges are usually fixed so all code in running with known program and data address ranges. Perhaps there's a memory management unit (or similar) that breaks this, or supports it.

There's no "fail big" in an embedded system. It'll just sit there in an infinite loop off an error interrupt. There's no stack-dump data in "it's dead again."

I always check the pointers, and array indexes, for being in-range. With, for example, a 2MB block of memory in a 32-bit system, there's only 0.05% of addresses that are valid. It's a quick, and comparatively small, check on contemporary microcontrollers to add this.

Answer 9

One practice is to always perform the null check unless you have already checked it; so if input is being passed from function A() to B(), and A() has already validated the pointer and you are certain B() isn't called anywhere else, then B() can trust A() to have sanitized the data.