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 <shalini(a)linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
---
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;
}
}