3

I have this C code:

#include <stdio.h>                                                              
#include <stdlib.h>                                                             
                                                                                
typedef struct {                                                                
    int value;                                                                  
} pointer_t;                                                                    
                                                                                
int some_func (pointer_t *p)                                                    
{                                                                               
    int v  =  (           rand () ) ?  99 :                                     
              (                !p ) ?   0 :                                     
              ( p->value % 2 == 0 ) ?   1 : /* << This is line 12 */ 
                                       -1 ;           
                                                                                
    return v;                                                                   
}                                                                               
                                                                                
int main () {                                                                   
    pointer_t p = { .value = 7 };                                               
                                                                                
    printf ("With a pointer : %d\n", some_func (&p));                           
    printf ("With a NULL    : %d\n", some_func (NULL));                         
}

When I run:

cppcheck prog.c

I get the following warning:

[prog.c:12]: (warning) Possible null pointer dereference: p

But I don't think there is a bug in my code: the previous line already checks the case when p is NULL, so in line 12 p must be non-NULL

Is this a false positive in cppcheck, or is there a corner case I haven't checked?

Edit

If it helps, these are some checks I did:

/* Generates a warning */
v  =  (           rand () ) ?  99 :
      (                !p ) ?   0 : 
      ( p->value % 2 == 0 ) ?   1 : 
                               -1 ;

/* Does not generate a warning */
v  =  (        p == NULL  ) ?   0 : 
      ( p->value % 2 == 0 ) ?   1 : 
                               -1 ;

/* Does not generate a warning */
if ( rand () )
    v = 99; 
else if (!p)
    v = 0;
else if (p->value % 2 == 0)
    v =  1;  
else
    v = -1; 

/* Generates a warning */
v  =  (           rand () ) ?  99 :
      (         p == NULL ) ?   0 : 
      ( p->value % 2 == 0 ) ?   1 : 
                               -1 ;
Hobber
  • 194
  • 1
  • 13
  • 1
    Please edit and remove the useless `/* x */` comments on each line. – Jabberwocky Feb 05 '21 at 11:55
  • 2
    @Jabberwocky: They are not useless; they enabled me to identify line 12 without counting. – Eric Postpischil Feb 05 '21 at 11:58
  • 1
    @EricPostpischil I agree more or less. He should a put a comment like `// << this is line 12` – Jabberwocky Feb 05 '21 at 11:59
  • 1
    The code looks a little bit obscure. E.g., the effective `typedef int pointer_t` is completely misleading. Your warning looks like a false positive though. – Wör Du Schnaffzig Feb 05 '21 at 12:12
  • Seriously, who approved this code formatting edit... I'll rollback, then also fix the slip that Hobber did in last valid edit. – Lundin Feb 05 '21 at 12:16
  • Hell, you just agree on what you want!!! :P – Hobber Feb 05 '21 at 12:17
  • 1
    I have to wonder what can possibly be gained from triply-nesting ternary operators - other than programmer confusion. – Andrew Henle Feb 05 '21 at 12:18
  • 2
    @AndrewHenle They are awesome for code golf, or for creating extra sequence points to prevent too good optimization, or for introducing subtle precedence bugs or implicit type promotion bugs between 2nd and 3rd operand. If you go for if-else, you risk faster, safer and more readable code. – Lundin Feb 05 '21 at 12:31
  • (On a serious note, there _is_ a place for icky code like this, in some specialized function-like macros. But pretty much only there.) – Lundin Feb 05 '21 at 12:32
  • I would say this is super readable. The `if` version is not so straightforward and forces the reader to check "what is happening here ?". As long as the assignment really does what it seems, I think it is okay to use it. – Hobber Feb 05 '21 at 12:46
  • Hmm come to think of it, what happens if you do this? `int null = 0;`.... `( !p ) ? null : `. Because `?:` comes with some rules regarding null pointer constants that might trip the static analyser. `0` being a null pointer constant, unlike the variable in this example. – Lundin Feb 05 '21 at 12:46
  • 1
    @Hobber No it's not super readable, it immediately tripped me when I first read it at a glance. Furthermore, can you truly describe all the sequence point locations, order of evaluation and usual arithmetic conversions going on in this function? Because the average C programmer most likely can't, not without hiring a language lawyer. – Lundin Feb 05 '21 at 12:54
  • 1
    @Hobber *I would say this is super readable. The if version is not so straightforward and forces the reader to check "what is happening here ?"* So it makes the reader actually think about what's going on in a step-by-step manner? Instead of just looking at an overly-dense blob of code and merely ***feeling*** like they know what's going on? How is that bad again? – Andrew Henle Feb 05 '21 at 12:56
  • @Andrew Henle Then let's write everything in a single line to really force the reader to pay attention to the code. This is about seeing the big picture at a glance instead of having to decipher what the code does. Specially when the indentation guides you to the correct view of the picture. – Hobber Feb 05 '21 at 14:23
  • @Hobber I hope you have noted that you have gotten comments from multiple posters here about what's wrong with your use of nested ternary operators. The combined time those posters have been active on this site is probably pushing half a century. And that probably highly underestimates the combined experience we all have writing ***and maintaining*** code. How long have you been writing code? – Andrew Henle Feb 05 '21 at 15:21
  • Okay, I'm a code kiddy. I just wanted to explain my point. I guess you guys are not prepared for this (just kidding) – Hobber Feb 05 '21 at 21:14

2 Answers2

3

It looks like a false positive, because cppcheck doesn't seem to follow all branches.

This is basically:

 if (rand())
    return 99;
 else if(!p)
    return 0;
 else if(p->value %2 == 0)
    return 1;
 else
    return -1;
Devolus
  • 21,661
  • 13
  • 66
  • 113
  • 2
    @Hobber: Their example is demonstrating that logic of the original code shows it cannot dereference a null pointer, by showing equivalent code in a more readable form. The fact cppcheck does not report a problem in it supports the issue being a bug in cppcheck. – Eric Postpischil Feb 05 '21 at 12:13
1

Using Cppcheck 2.3 I don't get that warning. Maybe you are using an older version?

Hobber
  • 194
  • 1
  • 13