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

[Michal Privoznik]

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