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

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(a)linux.ibm.com&gt; > Suggested-by: John Ferlan <jferlan(a)redhat.com&gt; > --- >   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(a)redhat.com&gt; Michal