Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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.

share|improve this question
2  
If you're writing a function for an API where callers aren't supposed to understand the innards, this is one of those places where documentation is important. If you document that an argument must be a valid, non-NULL pointer, checking it becomes the caller's responsibility. –  Blrfl Feb 5 '13 at 23:24
1  
See also: stackoverflow.com/questions/4390007/… –  Billy ONeal Feb 6 '13 at 1:10
add comment

7 Answers

up vote 4 down vote accepted

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.

share|improve this answer
    
The question is somewhat subjective but this seemed like the best answer for now. Thanks to everyone who gave their thoughts on this question. –  Gabriel Feb 7 '13 at 4:05
add comment

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

share|improve this answer
1  
If you're in a function that requires a non-NULL pointer, but you check anyway and it's NULL... what next? –  detly Feb 6 '13 at 1:34
    
Checking malloc for NULL isn't always enough- for example Linux will happily overallocate memory, returning a non-NULL address, and leave it for the out of memory killer to deal with it. –  Scott Wales Feb 6 '13 at 2:05
1  
@detly either stop what you're doing and return an error code, or trip an assert –  James Feb 6 '13 at 2:54
    
@James - didn't think of assert, sure. I don't like the error code idea if you're talking about changing existing code to include NULL checks though. –  detly Feb 6 '13 at 2:55
3  
@detly you're not going to get very far as a C dev if you don't like error codes –  James Feb 6 '13 at 2:58
show 4 more comments

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 */
share|improve this answer
add comment

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.

share|improve this answer
add comment

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.

share|improve this answer
add comment

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.

share|improve this answer
    
I don't know if I would classify a segfault as performing an automatic NULL check. I agree that having a CPU memory protection does help so that one process can't do as much damage to the rest of the system but I would not call it automatic protection. –  Gabriel Feb 6 '13 at 6:53
add comment

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.

share|improve this answer
1  
...until in 6 months time someone comes along and adds some more code that calls B() (possibly assuming that whoever wrote B() surely checked for NULLs properly). Then you're screwed, aren't you? Basic rule - if an invalid condition exists for the input to a function, then check for it, because the input is outside of the function's control. –  Jimmy Shelter Feb 6 '13 at 1:17
    
@mh01 If you're just smashing out random code (ie. making assumptions and not reading documentation), then I don't think extra NULL checks are going to do much. Think about it: now B() checks for NULL and... does what? Return -1? If the caller doesn't check for NULL, what confidence can you have that they going to deal with the -1 return value case anyway? –  detly Feb 6 '13 at 1:38
1  
That's the callers responsibility. You deal with your own responsibility, which includes not trusting any arbitrary/unknowable/potentially-unverified inputs you're given. Otherwise you're in cop-out city. If the caller doesn't check then the caller has screwed up; you checked, your own ass is covered, you can tell whoever wrote the caller that at least you did things right. –  Jimmy Shelter Feb 6 '13 at 1:46
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.