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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list