0

I am cleaning up some code from a Coverity issue and have a situation where a pointer local to a function is used earlier on is a function for comparison, then later on it is assigned to point else where but it is never de-referenced or used to assign something to its value. Coverity is saying that it is an unused pointer value - so I am thinking to cast it to a void to indicate the pointer is not used after that point. I am wondering when is a value or variable considered used in a program? Here is a code sample explaining my situation:

In the sample below Coverity would flag the fltr_ptr as not being used after its two assignments at the end of the sample

int initialise (void) 
{
    // pointer first decalred and defined
    FILTER_PTR  fltr_ptr = NULL;

    // pointer given valid value
    fltr_ptr = global_val->filter_group[index];

    // second use of fltr_ptr 
    if ( TRUE == fltr_ptr -> being_used)
    {
        // first use of fltr_ptr 
        if ( TRUE != close_filter ( fltr_ptr -> Filter)
        {
            // print error
        }
        // do more code

        // fltr_ptr assigned first time , value not used should it be (void)fltr_ptr?
        fltr_ptr = Free_Memory (fltr_ptr, sizeof(FILTER_PTR));
    }
    else
    {
        return 1;
    }

    for ( number of iterations )
    {
        // fltr_ptr assigned again but value not used should it be (void)fltr_ptr?
        fltr_ptr = global_val->filter_group[index];
    }
    return 0;
}
The_Neo
  • 308
  • 1
  • 4
  • 13
  • I don't see why this a question worth down voting , if you think it is a bad Q say why in a comment and let me know why? – The_Neo Oct 21 '14 at 10:15
  • the variable that is flagged as unused in fltr_ptr – The_Neo Oct 21 '14 at 10:16
  • regarding these kinds of lines: if ( TRUE == fltr_ptr -> being_used) TRUE is different, depending on the compiler and related header files. So comparison to TRUE is not a valid comparison. However, FALSE is always = 0 so a valid comparison would be: if ( FALSE != fltr_ptr -> being_used) or even better: if ( fltr_ptr -> being_used ) – user3629249 Oct 21 '14 at 13:54
  • This loop. starting with: for ( number of iterations ) does absolutely nothing. Therefore, the whole loop could be removed. – user3629249 Oct 21 '14 at 13:58
  • This statement: fltr_ptr = Free_Memory (fltr_ptr, sizeof(FILTER_PTR)); seems very likely to be invalid, as fltr_ptr is a local variable, on the stack, so should NEVER be free'd. – user3629249 Oct 21 '14 at 13:59

2 Answers2

4

Coverity points to you that you assign to fltr_ptr in the last for loop but you do nothing with this value. Why assign at all? Casting to void could possibly fix the warning, but the first thing to fix should be either to use the pointer somehow, or stop assigning to it.

Wojtek Surowka
  • 20,535
  • 4
  • 44
  • 51
1

To answer the title question, a variable is considered unused when "it is initialised or assigned to and then disposed of without being read."

int main()
{
    int i;
    int j = 1;     // both i and j are unused at this point

    int i = j * 2; // j is now 'used', the new value of i is unused
    printf("%d",j);//j is used again
}                  // i goes out of scope without being used.

note that the definition isn't also "if it is assigned to without being read" as this would indicate that there was a problem with the following:

unsigned int find_max_index(int a[], int size)
{
    unsigned int i;
    unsigned int maxval   = 0;
    unsigned int maxindex = 0;
    for (i = 0; i< size; i++){
        if (a[i]>maxval){
            maxval = a[i];
            maxindex = i;
        }
    }
    return maxindex;
}

As in this code maxindex can be assigned to multiple times without being read.

Looking back at my original example, we can eliminate i without any change to the program. This reduces the complexity of the program, removes redundant operations (though the compiler should do this when optimising too) and reduces the chance of programmer error in the future:

//FUNCTIONALLY THE SAME AND SIMPLER
int main()
{
    int j = 1;     // j is unused at this point
    printf("%d",j);// j is used
}

In the same way, you can remove this entire loop:

for ( number of iterations )
{
    // fltr_ptr assigned again but value not used should it be (void)fltr_ptr?
    fltr_ptr = global_val->filter_group[index];
}

(You remove the assignment and get an empty loop. as this is a long nop, it can also be removed)

Baldrickk
  • 4,291
  • 1
  • 15
  • 27