Coverity warns on default initialization of local variables

4.3k views Asked by At

There is a coverity warning type: UNUSED_VALUE. This is defined by tool under "Code maintainability issues"

UNUSED_VALUE: When a variable is assigned a pointer value returned from a function call and is never used anywhere else in the source code, it can not only cause inefficient use of resources but can also result in undetermined behavior. This checker identifies all variables that are never used anywhere else in the program after a value is assigned to them.

This checker seems to be picking up some of the good programming practice also as a warning.

My question is that is there some better way to do the things? Or should such a warning be ignored (and reported to Coverity team for any possible improvements)?

Example 1: default iniailization of local variables

int func()
{
   int retval = SUCCESS;  //COVERITY says: Unused value     (UNUSED_VALUE)assigned_value: Value SUCCESS is assigned to retval here, but that     stored value is not used before it is overwritten

   retval = recvMessage();  //COVERITY says: value_overwrite: Value SUCCESS     is overwritten with value fromrecvMessage

   ....

}

Example 2: setting pointer to NULL after memory is freed

void func2()
    {
       char *p = NULL;
       ....
       p = alloc_mem_wrapper();
       ... do something
       free_mem_wrapper(p);
       p = NULL; //Coverity says: Unused value (UNUSED_VALUE)assigned_pointer: Value NULL is assigned to p here, but that stored value is not used    
       ... do rest of the job (p is not used again)
    }

In my case, 90% of all the warnings are of above nature only!

2

There are 2 answers

1
alk On

Why not do it like this:

int retval = recvMessage();

and this

char * p = alloc_mem_wrapper();

(Mostly) if you do not know how to initialise a variable you probably do not needed (where you defined it).

0
Mohammad Azim On

All above suggestions are good, but still people do stumble on this error and see it as a problem. I guess people relate it to a violation of good practice because they think it will appear in following scenario

int status = -1;
char c = getch();

switch(c){
    case 'w': {
        status = walk();
    }
    break;
    case 's': {
        status = sit();
    }
    break;
}

printf("Status %d\n", status);

Above, it makes total sense to declare status on top and print it once the code in between has updated it. However, Coverity does not report it UNUSED_VALUE in this scenario. It actually complains on following code

int status = -1;
int steps = 0;
char c = getch();

switch(c){
    case 'w': {
        status = walk();
        steps = get_steps();
        status = (steps > 0)?status:-1;
    }
    break;
    case 's': {
        status = sit();
    }
    break;
}

printf("Status %d\n", status);

Above, steps can simply be a scope variable. Hence, the Coverity error has more to do with the scope than initialization.