[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