negativeIndex on

[Sébastien Gallou]

Hello,

I don't understand why this code generates a negativeIndex message with CppCheck 1.83 :

void f()
{
   std::vector<unsigned char> outLine;
   unsigned long outAddr = 0;
   unsigned long rowAddress = outAddr;
   unsigned int inAddr = 0;
   unsigned char rowData[256] = {0};
   while (inAddr < 256)
   {
      // Built the data line
      if (inAddr % 16 == 0)
      {
         outLine.clear();
         rowAddress = outAddr;
      }

      outLine.push_back(rowData[inAddr]);
      ++outAddr;
      ++inAddr;

      if (inAddr % 16 == 0 && inAddr != 0)   <== negativeIndex on this line (see full message below)
         programMemory[rowAddress] = outLine;
   }
}

inAddr is initialized to 0 and only incremented, to 256. So it can never be negative.
The test "inAddr != 0" is needed to make the test return false when inAddr is 0.

Exact message is :

[Temp.cpp:21] -> [Temp.cpp:17]: (warning) Either the condition 'inAddr!=0' is redundant, otherwise there is negative array index -1.

Any idea about this message ?

Thanks for your help

[Daniel Marjamäki]

You have more details in the GUI. Or in git head you get more details with --template=gcc:

1.cpp:21:38: note: Assuming that condition 'inAddr!=0' is not redundant
      if (inAddr % 16 == 0 && inAddr != 0)
                                     ^
1.cpp:19:9: note: inAddr is incremented, before this increment the value is -1
      ++inAddr;
        ^
1.cpp:17:33: note: Negative array index
      outLine.push_back(rowData[inAddr]);
                                ^

You do have a redundant condition. The inAddr can't be 0. If it would be 0.. then you have access of rowData[0-1] at line 17.

It is unfortunate that is says 'negative index' since the variable is unsigned... it should say that the index is 0xFFFFFFFF or something like that instead.

[versat]

As far as i understand:
Since you explicitly check if inAddr is or is not 0 (inAddr != 0) Cppcheck guesses that inAddr can possibly be 0 at this point, otherwise you would not (have to) check it.
But if it could be 0 at this point it must have been negative (which is not really correct here, it was very/maximum large, because it is an unsigned variable) before, this is the only way the increment of inAddr immediately before the check could make it become 0.
And if it was smaller than 0 before the increment, then there is a negative (or extremely large) index used in rowData[inAddr] which would be likely an error.

Static analyzers use assumptions like this to check for possible errors.
For example if you compare a pointer against NULL or nullptr then they often guess that this pointer could possibly be NULL and warn about null pointer dereference.

So IMHO the real hint here is that the comparision inAddr != 0 is redundant/not needed.

Edit: I was too slow :)

[Sébastien Gallou]

Yes, absolutely, you're both right !
I look at this code many times without seeing that the increment makes inAddr not possible to be 0, so the test "inAddr != 0" if redundant.
Thanks for your help, and sorry for this stupid question...

Sébastien

[versat]

I dont think that the question is stupid. I learned that the output with --template=gcc is more verbose and useful than i thought. I will use that more often now. :)