Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Possible null pointer dereference - otherwise it is redundant to check it against null

Tags:

c++

cppcheck

I have the following code, which is working properly:

int result = ERRORCODE_OK;
if (dataObj == NULL || dataObj->inputSignal == NULL)
{
   result = ERRORCODE_MISSING_DATAOBJ;
}
if (result == ERRORCODE_OK && dataObj->spectrum == NULL) // CPP-Check error
{
   result = Calculate(dataObj->inputSignal, .. );
} 
return result;

But CppCheck gives me the following error:

Possible null pointer dereference: dataObj - otherwise it is redundant to check it against null.

I don't understand why. If the dataobj is NULL, then the result will be something else then ERRORCODE_OK.

like image 654
S.Spieker Avatar asked Dec 10 '22 20:12

S.Spieker


2 Answers

CppCheck doesn't inspect deep enough to see that your second condition won't be fully evaluated if the first one succeeds:

int result = ERRORCODE_OK;
if (dataObj == NULL || dataObj->inputSignal == NULL)
    result = ERRORCODE_MISSING_DATAOBJ;

// Here, you can see that either dataObj!=NULL or result!=ERRORCODE_OK.
// But CppCheck can't!

if (result == ERRORCODE_OK && dataObj->spectrum == NULL)
    result = Calculate(dataObj->inputSignal, .. );
return result;

Three alternative ways of pacifying the checker present themselves. Firstly, just repeat the check that dataObj is non-null in the second if. Secondly, change the second if to else if:

int result = ERRORCODE_OK;
if (dataObj == NULL || dataObj->inputSignal == NULL)
{
    result = ERRORCODE_MISSING_DATAOBJ;
}
else if (result == ERRORCODE_OK && dataObj->spectrum == NULL)
{
    result = Calculate(dataObj->inputSignal, .. );
} 
return result;

Thirdly, return as soon as you find one of the error cases:

if (!dataObj || !dataObj->inputSignal)
    return ERRORCODE_MISSING_DATAOBJ;
if (dataObj->spectrum)
    return ERRORCODE_OK;
return Calculate(dataObj->inputSignal, .. );
like image 134
Toby Speight Avatar answered Dec 13 '22 10:12

Toby Speight


It happens because you check that variable for NULL here:

if (dataObj == NULL || dataObj->inputSignal == NULL)

This makes the analyzer think that dataObj can, in some circumstances, be NULL.

Now, Cppcheck can't know the logic behind your code, there is no way for it to know that result == ERRORCODE_OK ensures that dataObj != NULL, so it gives you the warning about your second if. Basically it assumes that if you check a variable for NULL in the first condition, then it makes sence to check it for NULL in the second condition too.

Note that it says "Possible null pointer dereference", so in your case it's just a false-positive.

like image 42
SingerOfTheFall Avatar answered Dec 13 '22 09:12

SingerOfTheFall