[PATCH] node_device: pacify grumpy coverity due to addr override

[Boris Fiuczynski]

On 12/11/20 3:33 PM, Michal Privoznik wrote:
> On 12/10/20 6:32 PM, Boris Fiuczynski wrote:
>> With commit 09364608b4 node_device: refactor address retrieval of node 
>> device
>> "if-else if" was replaced by "switch".
>> The contained break statement now is no longer in context of the for loop
>> but instead of the switch causing the legitimate grumpiness of coverity.
>>
>> Signed-off-by: Boris Fiuczynski <fiuczy at linux.ibm.com>
>> Suggested-by: John Ferlan <jferlan at redhat.com>
>> ---
>>   src/node_device/node_device_driver.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/node_device/node_device_driver.c 
>> b/src/node_device/node_device_driver.c
>> index e254b49244..da1bc8a545 100644
>> --- a/src/node_device/node_device_driver.c
>> +++ b/src/node_device/node_device_driver.c
>> @@ -637,7 +637,7 @@ nodeDeviceFindAddressByName(const char *name)
>>       }
>>       def = virNodeDeviceObjGetDef(dev);
>> -    for (caps = def->caps; caps != NULL; caps = caps->next) {
>> +    for (caps = def->caps; caps != NULL && addr == NULL; caps = 
>> caps->next) {
>>           switch (caps->data.type) {
>>           case VIR_NODE_DEV_CAP_PCI_DEV: {
>>               virPCIDeviceAddress pci_addr = {
>>
> 
> Yep, this is a genuine bug. Because those 'break' statements will end 
> the switch() and not the for() loop. But what I worry is that this 
> compound check is easy to overlook. So how about explicit:
> 
>    if (addr)
>        break;
> 
> at the end of the loop, after the switch()?
> 
> Either way:
> 
> Reviewed-by: Michal Privoznik <mprivozn at redhat.com>
> 
> Michal
> 

Yes, agree that would make it more obvious.
Like this...

diff --git a/src/node_device/node_device_driver.c 
b/src/node_device/node_device_driver.c
index e254b49244..51b20848ce 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -687,6 +687,9 @@ nodeDeviceFindAddressByName(const char *name)
          case VIR_NODE_DEV_CAP_LAST:
              break;
          }
+
+        if (addr)
+            break;
      }

      virNodeDeviceObjEndAPI(&dev);


If you want me to resend the patch please let me know.


-- 
Mit freundlichen Grüßen/Kind regards
    Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294