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@xxxxxxxxxxxxx>
> Reviewed-by: Bjoern Walk <bwalk@xxxxxxxxxxxxx>
> Reviewed-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxx>
> ---
> 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;
> }
> }
>
>
