[Libvirt-cim] [PATCH] device_parsing: Fixing potential leaks

Chip Vincent cvincent at linux.vnet.ibm.com
Thu Aug 18 05:26:01 UTC 2011


On 08/11/2011 11:09 AM, Eduardo Lima (Etrunko) wrote:
> # HG changeset patch
> # User Eduardo Lima (Etrunko)<eblima at br.ibm.com>
> # Date 1313075262 10800
> # Node ID c0f20dbefb8fd73c278a619bbbd2efb567da07d4
> # Parent  2b1b79a72ab0288d649399118dffce26fd05e066
> device_parsing: Fixing potential leaks
>
> As reported in https://bugzilla.redhat.com/show_bug.cgi?id=728245
>
> line 106 - Function cleanup_virt_device does not free its parameter dev (this
>             causes a lot of Coverity Resource leak warnings).
>
> In future use cases, please consider using cleanup_virt_devices function
> instead of cleanup_virt_device if the virt_device structure is allocated
> dynamically.
>
> Signed-off-by: Eduardo Lima (Etrunko)<eblima at br.ibm.com>
>
> diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c
> --- a/libxkutil/device_parsing.c
> +++ b/libxkutil/device_parsing.c
> @@ -829,12 +829,10 @@
>           if (ret<= 0)
>                   return ret;
>
> -        mdev = malloc(sizeof(*mdev));
> +        mdev = calloc(1, sizeof(*mdev));

What are the benefits to this change?

>           if (mdev == NULL)
>                   return 0;
>
> -        memset(mdev, 0, sizeof(*mdev));
> -
>           /* We could get one or two memory devices back, depending on
>            * if there is a currentMemory tag or not.  Coalesce these
>            * into a single device to return
> @@ -870,12 +868,10 @@
>           if (ret<= 0)
>                   return ret;
>
> -        proc_dev = malloc(sizeof(*proc_dev));
> +        proc_dev = calloc(1, sizeof(*proc_dev));
>           if (proc_dev == NULL)
>                   return 0;
>
> -        memset(proc_dev, 0, sizeof(*proc_dev));
> -
>           proc_dev->type = CIM_RES_TYPE_PROC;
>           proc_dev->id = strdup("proc");
>           proc_dev->dev.vcpu.quantity = proc_devs[0].dev.vcpu.quantity;
> @@ -1031,8 +1027,6 @@
>           xmlNode **nodes = nsv->nodeTab;
>           xmlNode *child;
>
> -        memset(dominfo, 0, sizeof(*dominfo));
> -
>           dominfo->typestr = get_attr_value(nodes[0], "type");
>
>           for (child = nodes[0]->children; child != NULL; child = child->next) {
> @@ -1098,7 +1092,7 @@
>           int ret;
>
>           CU_DEBUG("In get_dominfo_from_xml");
> -        *dominfo = malloc(sizeof(**dominfo));
> +        *dominfo = calloc(1, sizeof(**dominfo));
>           if (*dominfo == NULL)
>                   return 0;
>
> diff --git a/libxkutil/xmlgen.c b/libxkutil/xmlgen.c
> --- a/libxkutil/xmlgen.c
> +++ b/libxkutil/xmlgen.c
> @@ -832,7 +832,7 @@
>                   dominfo->dev_input = dev;
>                   break;
>           default:
> -                cleanup_virt_device(dev);
> +                cleanup_virt_devices(&dev, 1);
>                   goto out;
>           }
>
> diff --git a/src/Virt_AppliedFilterList.c b/src/Virt_AppliedFilterList.c
> --- a/src/Virt_AppliedFilterList.c
> +++ b/src/Virt_AppliedFilterList.c
> @@ -516,7 +516,7 @@
>           free(net_name);
>
>           cleanup_filter(filter);
> -        cleanup_virt_device(device);
> +        cleanup_virt_devices(&device, 1);

It seems to me that these two commands should be equal? Wouldn't the 
correct change be to ensure that they are?

>
>           virDomainFree(dom);
>           virConnectClose(conn);
> @@ -626,7 +626,7 @@
>           free(net_name);
>
>           cleanup_filter(filter);
> -        cleanup_virt_device(device);
> +        cleanup_virt_devices(&device, 1);
>
>           virDomainFree(dom);
>           virConnectClose(conn);
> diff --git a/src/Virt_Device.c b/src/Virt_Device.c
> --- a/src/Virt_Device.c
> +++ b/src/Virt_Device.c
> @@ -700,14 +700,15 @@
>           }
>
>           for (i = 0; i<  count; i++) {
> -                if (STREQC(device, list[i].id))
> +                if (STREQC(device, list[i].id)) {
>                           dev = virt_device_dup(&list[i]);
> +                        break;
> +                }
>
> -                cleanup_virt_device(&list[i]);
>           }
>
> +        cleanup_virt_devices(&list, count);
>    out:
> -        free(list);
>
>           return dev;
>   }
> @@ -785,7 +786,7 @@
>                                         &tmp_list);
>           }
>
> -        cleanup_virt_device(dev);
> +        cleanup_virt_devices(&dev, 1);
>
>           *_inst = tmp_list.list[0];
>
> diff --git a/src/Virt_RASD.c b/src/Virt_RASD.c
> --- a/src/Virt_RASD.c
> +++ b/src/Virt_RASD.c
> @@ -664,7 +664,7 @@
>           char *host = NULL;
>           char *devid = NULL;
>           virConnectPtr conn = NULL;
> -        struct virt_device *dev;
> +        struct virt_device *dev = NULL;
>
>           conn = connect_by_classname(broker, CLASSNAME(reference),&s);
>           if (conn == NULL) {
> @@ -700,8 +700,8 @@
>                              "Failed to set instance properties");
>           else
>                   *_inst = inst;
> -
> -        cleanup_virt_device(dev);
> +
> +        cleanup_virt_devices(&dev, 1);
>
>    out:
>           virConnectClose(conn);
> @@ -863,10 +863,7 @@
>
>                   tmp_dev->id = strdup("proc");
>
> -                for (i = 0; i<  count; i++)
> -                        cleanup_virt_device(&devs[i]);
> -
> -                free(devs);
> +                cleanup_virt_devices(&devs, count);
>                   devs = tmp_dev;
>                   count = 1;
>           }
> @@ -877,9 +874,6 @@
>                              CMPI_RC_ERR_FAILED,
>                              "Failed to get domain name");
>
> -                for (i = 0; i<  count; i++)
> -                        cleanup_virt_device(&devs[i]);
> -
>                   goto out;
>           }
>
> @@ -893,12 +887,10 @@
>                                        properties);
>                   if (dev)
>                           inst_list_add(list, dev);
> -
> -                cleanup_virt_device(&devs[i]);
>           }
>
>    out:
> -        free(devs);
> +        cleanup_virt_devices(&devs, count);
>           return s;
>   }
>
> diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c
> --- a/src/Virt_VirtualSystemManagementService.c
> +++ b/src/Virt_VirtualSystemManagementService.c
> @@ -200,12 +200,6 @@
>
>           cleanup_virt_device(domain->dev_emu);
>
> -        domain->dev_emu = calloc(1, sizeof(*domain->dev_emu));
> -        if (domain->dev_emu == NULL) {
> -                CU_DEBUG("Failed to allocate default emulator device");
> -                return false;
> -        }
> -
>           domain->dev_emu->type = CIM_RES_TYPE_EMU;
>           domain->dev_emu->dev.emu.path = strdup(emu);
>           domain->dev_emu->id = strdup("emulator");
>
> _______________________________________________
> Libvirt-cim mailing list
> Libvirt-cim at redhat.com
> https://www.redhat.com/mailman/listinfo/libvirt-cim

-- 
Chip Vincent
Open Virtualization
IBM Linux Technology Center
cvincent at linux.vnet.ibm.com




More information about the Libvirt-cim mailing list