Coverity warns on default initialization of local variables

Question

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!

In both cases I'd consider the code poor style, you're making unused assignments which the checker rightfully points out. — M.M, Jun 11 '15 at 07:20
"Good programming practice" is very subjective. And, for example, do you really need to initialize a variable when it's defined, if the first thing you will do with it is re-initializing it? — Some programmer dude, Jun 11 '15 at 07:25
Now that even MSVC finally supports C99-style declarations-mixed-with-statements in C code, the better way to handle this generally is to declare & initialize the variable right at the spot it needs to be declared, no sooner. If using a compiler that forces the older style of declaring all variables at the start of a block, in my opinion even 'pointless' zero initialization is fine. But if you're committed to using Coverity, you may want to trust it to tell you about uninitialized variable use - it should be very, very good at that. Run of the mill compilers *should* be better at it. — Michael Burr, Jun 11 '15 at 07:39
to me it looks like defensive programming and IMHO you should report (at least the second one) as a false negative. The first assignment, even though a common practice, it is a pointless initialization (retval=SUCCESS) — Pandrei, Jun 11 '15 at 08:33
Dont agree that its a poor coding style. Its defensive. There are two flavours I know about: 1. JSF-AV-rules: "it is considered good practice to initialize all variables, not just automatic/stack variables, to an initial value for purposes of 1) clarity and 2) bringing focused attention to the initialization of each variable." 2. MISRA2004: "All automatic variables shall have been assigned a value before being used: The intent of this rule is that all variables shall have been written to before they are read. This does not necessarily require initialisation at declaration." — , Jun 12 '15 at 06:00
btw, I found that Coverity allows checker settings to disable such warnings, if programmer believes they are false in his case: - UNUSED_VALUE:report_overwritten_initializer: - When this C/C++, C#, and Java option is true, the checker will report cases where a value that initialized a variable is overwritten before it is used. - UNUSED_VALUE:report_unused_final_assignment: - When this C/C++, C#, and Java option is true, the checker will report cases where a variable is assigned a final value, but that value is never used before the variable goes out of scope. — , Jun 12 '15 at 06:08

score 0 · Answer 1 · answered Jun 11 '15 at 08:08

0

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).

answered Jun 11 '15 at 08:08

alk

69,737
10
105
255

I think the warning is saying that 'p=NULL' is an un-used and pointless assignment; which is not true! – Pandrei Jun 11 '15 at 08:29

score 0 · Answer 2 · answered Mar 23 '20 at 21:36

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.

Coverity warns on default initialization of local variables

2 Answers2