On 03/10/2016 01:46 PM, Andrea Bolognani wrote:
On Thu, 2016-03-10 at 12:35 -0500, John Ferlan wrote:
If for some reason we "continue" (or not) and eventually "goto resetvfnetconfig;" before ever setting last_processed_hostdev_vf, then we get to the goto.,..
resetvfnetconfig: - for (i = 0; - last_processed_hostdev_vf != -1 && i <= last_processed_hostdev_vf; i++)
and last_processed_hostdev_vf still == -1
So that check needs to be there - perhaps just add an:
if (last_processed_hostdev_vf > -1) { } I fail to see how this is a problem: if last_processed_hostdev_vf has never been assigned a value after being declared, then the rewritten loop would be equivalent to
for (i = 0; i <= -1; i++) { ... }
which means the loop body will never be executed, just as expected. Am I missing something?
size_t i;
Coverity says:
(60) Event negative_returns: Using unsigned variable "last_processed_hostdev_vf" in a loop exit condition.
I admit I had to do some research, but I get it now.
Not having any experience with the tool, its output looks very confusing to me[1]; thankfully you are fluent in Coverity and pointed me in the right direction :)
Yeah it's like reading a whole new language sometimes. Especially confusing when you lookup the last*vf and see it's an 'int'. I would have felt better if it complained using an 'unsigned int' (i) and that shouldn't be compared with an 'int' (last*vf).
Will change it to move the check on last_processed_hostdev_vf outside the for loop as you suggested, it might not be as nice as getting rid of it altogether but it's still much more readable than the current code.
Well you could always go with a while loop and remove from the end.. while (last*vf >= 0) { hostdev = hostdevs[last*vf--]; virHostdevNetConfigRestore(hostdev,...); }
Cheers.
[1] Why does it claim that 'last_processed_hostdev_vf' is an unsigned variable when it's not? And why would using an unsigned variable in loop exit condition automatically be a problem?
Asking the wrong guy about compilers, size_t's, ssize_t's, int's, unsigned int's and what happens. In the long run it's all about compilers and types. That loop probably ends up being controlled in compiler code by using a 64-bit register doing unsigned variable comparisons (since that's what 'i' is). The last*vf will get moved into another register as an unsigned. That's a WAG. John