On 08/11/2011 11:09 AM, Eduardo Lima (Etrunko) wrote:

# HG changeset patch # User Eduardo Lima (Etrunko)<eblima@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@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@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim

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