Getting warning - Dereferencing before possibly being null in C code

sbhatla picture sbhatla · Oct 13, 2015 · Viewed 10k times · Source

I'm getting a warning while doing a Static Analysis (SA) on my code. I have simplified it below (with the first warning)-

typedef struct testStruct_ {
int *ptr;
} testStruct;

testStruct a;
testStruct *a_ptr;
a_ptr = &a;
a_ptr->ptr = NULL; #WARNING: Directly dereferencing pointer a_ptr.

The code goes on to do several operations with a_ptr. I've posted an example for the sake of completion-

rc = fn_a (filename, a_ptr);
rc = fn_b (a_ptr);
rc = fn_c (a_ptr->ptr);

fn_a is defined as-

fn_a (const char *filename, testStruct *a_ptr)
{
    a_ptr->ptr = fn_a_2(filename);
    if (!a_ptr->ptr) {
        ERR("Loading (%s) failed", filename);
        return (FALSE);
    }
    return (TRUE);
}

At one point later, I get another warning:

if (a_ptr && a_ptr->ptr) {
    freeFn(a_ptr->ptr);
}
#WARNING: Dereference before NULL check - NULL checking a_ptr suggests that it may be NULL, but it has already been dereferenced on all paths leading up to the check.

It seems like the line a_ptr->ptr = NULL is deemed incorrect/dangerous. Why is this error being shown and is there a way to correct it?

Answer

dbush picture dbush · Oct 13, 2015

Coverity is giving you a warning because you are in fact doing a NULL check:

if (a_ptr && a_ptr->ptr) {

Here, a_ptr is evaluated in a boolean context. It evaluates to true if a_ptr is not NULL.

This warning thrown by Coverity if you dereference a pointer and then later on do a NULL check on it. This means one of two things:

  • The pointer could in fact be NULL and any dereference prior to that NULL check could result in a NULL pointer dereference, so you need to either do the NULL check sooner or don't deereference at that point.
  • The NULL check is unnecessary because the pointer can't be NULL, so the NULL check should be removed.

In this particular case, you're explicitly setting a_ptr to the address of a variable, so it can't possibly be NULL at that point. If you don't set it again before the above if statement, that means that the NULL check is unnecessary and should be removed.