[libvirt] [PATCH 11/24] hostdev: Remove redundant check

[John Ferlan]

On 03/10/2016 12:21 PM, Andrea Bolognani wrote:
> On Thu, 2016-03-10 at 12:01 -0500, John Ferlan wrote:
>> On 03/07/2016 12:24 PM, Andrea Bolognani wrote:
>>>  
>>> If 'last_processed_hostdev_vf != -1' is false then, since the
>>> loop counter 'i' starts at 0, 'i <= last_processed_hostdev_vf'
>>> can't possibly be true and the loop body will never be executed.
>>>  
>>> Hence, the first check is completely redundant and can be safely
>>> removed.
>>> ---
>>>   src/util/virhostdev.c | 3 +--
>>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>>  
>> Premise understood; however, Coverity has an issue...
> 
> Well, that's a first :P
> 
>> Way back here:
>>  
>> 507       virPCIDeviceListPtr pcidevs = NULL;
>>  
>> (1) Event var_tested_neg:     Assigning: "last_processed_hostdev_vf" = a negative value.
>> Also see events:  [negative_returns]
>>  
>> 508       int last_processed_hostdev_vf = -1;
>>  
>>  
>> Eventually we enter this loop:
>>  
>>      for (i = 0; i < nhostdevs; i++) {
>>           virDomainHostdevDefPtr hostdev = hostdevs[i];
>>           if (!virHostdevIsPCINetDevice(hostdev))
>>               continue;
>>           if (virHostdevNetConfigReplace(hostdev, uuid,
>>                                          mgr->stateDir) < 0) {
>>               goto resetvfnetconfig;
>>           }
>>           last_processed_hostdev_vf = i;
>>      }
>>  
>> 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.


John