Joachim Reichel <joachim.reic...@posteo.de> writes:
> On 14.12.20 01:23, Russ Allbery wrote:

>> In the second case, something about adding retval to the test messes up
>> its understanding of the data flow.

> Removing retval from the if() condition does not change anything for
> me. Could you double-check?

Ah, it's not the presence in the condition that does it, it's the
assignment of the function return value (?!).  The following produces no
warnings:

int
test_b(void)
{
    char **string = NULL;

    some_other_call(&string);
    if (string && string[0])
        return 0;
    return -1;
}

but the following does:

int result;

int
test_b(void)
{
    char **string = NULL;

    result = some_other_call(&string);
    if (string && string[0])
        return 0;
    return -1;
}

% cppcheck --enable=warning,style foo-2.c 
Checking foo-2.c ...
foo-2.c:6:12: warning: Either the condition 'if(string&&string[0])' is 
redundant or there is possible null pointer dereference: string. 
[nullPointerRedundantCheck]
    char **string = NULL;
           ^
foo-2.c:9:8: note: Assuming that condition 'if(string&&string[0])' is not 
redundant
    if (string && string[0])
       ^
foo-2.c:6:12: note: Null pointer dereference
    char **string = NULL;
           ^

>> The third case seems similar to the previous set of bugs, although note
>> that it only happens with assignment.  If that line is instead replaced
>> with something like call_c(foo->flag), there is no error.

> I suppose you meant replacing "blah.flag = foo->flag;" by
> "call_c(foo->flag)"? This does not change anything for me.

Indeed, you're right.  I'm not sure what it was that I was trying that
made me think that.  Here's a more minimal test case:

void
test_c(void)
{
    some_type *foo = NULL;

    retval = call_a(&foo);
    if (retval != 0)
        goto done;
    call_b(foo->flag);

done:
    if (foo != NULL)
        free_foo(foo);
}

% cppcheck --enable=warning,style foo-3.c 
Checking foo-3.c ...
foo-3.c:9:12: warning: Either the condition 'foo!=NULL' is redundant or there 
is possible null pointer dereference: foo. [nullPointerRedundantCheck]
    call_b(foo->flag);
           ^
foo-3.c:12:13: note: Assuming that condition 'foo!=NULL' is not redundant
    if (foo != NULL)
            ^
foo-3.c:9:12: note: Null pointer dereference
    call_b(foo->flag);
           ^

> Is "call_b(&blah);" relevant in this test?

It was there just to prevent an unused variable warning for blah from
cluttering up the output, but the above test works better.

Thank you so much for looking into this and passing along the bug reports!
I love cppcheck and run it on all the C software I release.

-- 
Russ Allbery (r...@debian.org)              <https://www.eyrie.org/~eagle/>

Reply via email to