Re: [PATCH libvirt v4 09/12] node_device: refactor address retrieval of node device
On 12/10/20 1:01 PM, John Ferlan wrote: Coverity got pretty grumpy this morning with RESOURCE_LEAKS due to override of @addr as changing from a break; inside a switch/case statement sent us back to the for statement rather than outside the loop which would occur on the break; inside the if {} else if {} Suggestion... add "&& addr != NULL" as an additional for loop exit. John Thanks John. I sent a patch with your suggested change slightly modified. -- 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
Re: [PATCH libvirt v4 09/12] node_device: refactor address retrieval of node device
On 12/3/20 12:59 PM, Shalini Chellathurai Saroja wrote: > Use switch statements instead of if-else condition in the method > nodeDeviceFindAddressByName to retrieve address of a node device. > > Signed-off-by: Shalini Chellathurai Saroja > Reviewed-by: Bjoern Walk > Reviewed-by: Boris Fiuczynski > --- > src/node_device/node_device_driver.c | 30 ++-- > 1 file changed, 28 insertions(+), 2 deletions(-) > > diff --git a/src/node_device/node_device_driver.c > b/src/node_device/node_device_driver.c > index f5ea973c..65c647f5 100644 > --- a/src/node_device/node_device_driver.c > +++ b/src/node_device/node_device_driver.c > @@ -638,7 +638,8 @@ nodeDeviceFindAddressByName(const char *name) > > def = virNodeDeviceObjGetDef(dev); > for (caps = def->caps; caps != NULL; caps = caps->next) { > -if (caps->data.type == VIR_NODE_DEV_CAP_PCI_DEV) { > +switch (caps->data.type) { > +case VIR_NODE_DEV_CAP_PCI_DEV: { Coverity got pretty grumpy this morning with RESOURCE_LEAKS due to override of @addr as changing from a break; inside a switch/case statement sent us back to the for statement rather than outside the loop which would occur on the break; inside the if {} else if {} Suggestion... add "&& addr != NULL" as an additional for loop exit. John > virPCIDeviceAddress pci_addr = { > .domain = caps->data.pci_dev.domain, > .bus = caps->data.pci_dev.bus, > @@ -648,7 +649,9 @@ nodeDeviceFindAddressByName(const char *name) > > addr = virPCIDeviceAddressAsString(&pci_addr); > break; > -} else if (caps->data.type == VIR_NODE_DEV_CAP_CSS_DEV) { > +} > + > +case VIR_NODE_DEV_CAP_CSS_DEV: { > virDomainDeviceCCWAddress ccw_addr = { > .cssid = caps->data.ccw_dev.cssid, > .ssid = caps->data.ccw_dev.ssid, > @@ -657,6 +660,29 @@ nodeDeviceFindAddressByName(const char *name) > > addr = virDomainCCWAddressAsString(&ccw_addr); > break; > +} > + > +case VIR_NODE_DEV_CAP_SYSTEM: > +case VIR_NODE_DEV_CAP_USB_DEV: > +case VIR_NODE_DEV_CAP_USB_INTERFACE: > +case VIR_NODE_DEV_CAP_NET: > +case VIR_NODE_DEV_CAP_SCSI_HOST: > +case VIR_NODE_DEV_CAP_SCSI_TARGET: > +case VIR_NODE_DEV_CAP_SCSI: > +case VIR_NODE_DEV_CAP_STORAGE: > +case VIR_NODE_DEV_CAP_FC_HOST: > +case VIR_NODE_DEV_CAP_VPORTS: > +case VIR_NODE_DEV_CAP_SCSI_GENERIC: > +case VIR_NODE_DEV_CAP_DRM: > +case VIR_NODE_DEV_CAP_MDEV_TYPES: > +case VIR_NODE_DEV_CAP_MDEV: > +case VIR_NODE_DEV_CAP_CCW_DEV: > +case VIR_NODE_DEV_CAP_VDPA: > +case VIR_NODE_DEV_CAP_AP_CARD: > +case VIR_NODE_DEV_CAP_AP_QUEUE: > +case VIR_NODE_DEV_CAP_AP_MATRIX: > +case VIR_NODE_DEV_CAP_LAST: > +break; > } > } > >
[PATCH libvirt v4 09/12] node_device: refactor address retrieval of node device
Use switch statements instead of if-else condition in the method nodeDeviceFindAddressByName to retrieve address of a node device. Signed-off-by: Shalini Chellathurai Saroja Reviewed-by: Bjoern Walk Reviewed-by: Boris Fiuczynski --- src/node_device/node_device_driver.c | 30 ++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index f5ea973c..65c647f5 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -638,7 +638,8 @@ nodeDeviceFindAddressByName(const char *name) def = virNodeDeviceObjGetDef(dev); for (caps = def->caps; caps != NULL; caps = caps->next) { -if (caps->data.type == VIR_NODE_DEV_CAP_PCI_DEV) { +switch (caps->data.type) { +case VIR_NODE_DEV_CAP_PCI_DEV: { virPCIDeviceAddress pci_addr = { .domain = caps->data.pci_dev.domain, .bus = caps->data.pci_dev.bus, @@ -648,7 +649,9 @@ nodeDeviceFindAddressByName(const char *name) addr = virPCIDeviceAddressAsString(&pci_addr); break; -} else if (caps->data.type == VIR_NODE_DEV_CAP_CSS_DEV) { +} + +case VIR_NODE_DEV_CAP_CSS_DEV: { virDomainDeviceCCWAddress ccw_addr = { .cssid = caps->data.ccw_dev.cssid, .ssid = caps->data.ccw_dev.ssid, @@ -657,6 +660,29 @@ nodeDeviceFindAddressByName(const char *name) addr = virDomainCCWAddressAsString(&ccw_addr); break; +} + +case VIR_NODE_DEV_CAP_SYSTEM: +case VIR_NODE_DEV_CAP_USB_DEV: +case VIR_NODE_DEV_CAP_USB_INTERFACE: +case VIR_NODE_DEV_CAP_NET: +case VIR_NODE_DEV_CAP_SCSI_HOST: +case VIR_NODE_DEV_CAP_SCSI_TARGET: +case VIR_NODE_DEV_CAP_SCSI: +case VIR_NODE_DEV_CAP_STORAGE: +case VIR_NODE_DEV_CAP_FC_HOST: +case VIR_NODE_DEV_CAP_VPORTS: +case VIR_NODE_DEV_CAP_SCSI_GENERIC: +case VIR_NODE_DEV_CAP_DRM: +case VIR_NODE_DEV_CAP_MDEV_TYPES: +case VIR_NODE_DEV_CAP_MDEV: +case VIR_NODE_DEV_CAP_CCW_DEV: +case VIR_NODE_DEV_CAP_VDPA: +case VIR_NODE_DEV_CAP_AP_CARD: +case VIR_NODE_DEV_CAP_AP_QUEUE: +case VIR_NODE_DEV_CAP_AP_MATRIX: +case VIR_NODE_DEV_CAP_LAST: +break; } } -- 2.26.2