- Devel - libvirt List Archives

[libvirt] [PATCH] Fix several memory leaks
by Ryota Ozaki 05 Sep '09

05 Sep '09

Hi, This patch fixes several memory leaks being scattered over libvirt. Thanks, ozaki-r >From 6fc282eae5192cccda208d8d4fd14c5c0676992b Mon Sep 17 00:00:00 2001 From: Ryota Ozaki <ozaki.ryota(a)gmail.com> Date: Tue, 1 Sep 2009 01:34:40 +0900 Subject: [PATCH] Fix several memory leaks --- qemud/qemud.c | 2 +- src/domain_conf.c | 1 + src/network_conf.c | 5 +++-- src/qemu_conf.c | 13 ++++++++----- src/storage_backend_fs.c | 5 +++++ 5 files changed, 18 insertions(+), 8 deletions(-) diff --git a/qemud/qemud.c b/qemud/qemud.c index df275e6..17ba44a 100644 --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -1733,7 +1733,7 @@ readmore: /* Possibly need to create another receive buffer */ if ((client->nrequests < max_client_requests && - VIR_ALLOC(client->rx) < 0)) { + !client->rx && VIR_ALLOC(client->rx) < 0)) { qemudDispatchClientFailure(client); } else { if (client->rx) diff --git a/src/domain_conf.c b/src/domain_conf.c index 1d2cc7c..4b64219 100644 --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -4361,6 +4361,7 @@ int virDomainSaveXML(virConnectPtr conn, cleanup: if (fd != -1) close(fd); + VIR_FREE(configFile); return ret; } diff --git a/src/network_conf.c b/src/network_conf.c index bb649a4..58a4f32 100644 --- a/src/network_conf.c +++ b/src/network_conf.c @@ -820,6 +820,7 @@ int virNetworkDeleteConfig(virConnectPtr conn, { char *configFile = NULL; char *autostartLink = NULL; + int ret = -1; if ((configFile = virNetworkConfigFile(conn, configDir, net->def->name)) == NULL) goto error; @@ -836,12 +837,12 @@ int virNetworkDeleteConfig(virConnectPtr conn, goto error; } - return 0; + ret = 0; error: VIR_FREE(configFile); VIR_FREE(autostartLink); - return -1; + return ret; } char *virNetworkConfigFile(virConnectPtr conn, diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 22f5edd..32d6a48 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -1034,7 +1034,7 @@ qemudNetworkIfaceConnect(virConnectPtr conn, virDomainNetDefPtr net, int qemuCmdFlags) { - char *brname; + char *brname = NULL; int err; int tapfd = -1; int vnet_hdr = 0; @@ -1053,7 +1053,7 @@ qemudNetworkIfaceConnect(virConnectPtr conn, if (brname == NULL) return -1; } else if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) { - brname = net->data.bridge.brname; + brname = strdup(net->data.bridge.brname); } else { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("Network type %d is not supported"), net->type); @@ -1063,7 +1063,7 @@ qemudNetworkIfaceConnect(virConnectPtr conn, if (!driver->brctl && (err = brInit(&driver->brctl))) { virReportSystemError(conn, err, "%s", _("cannot initialize bridge support")); - return -1; + goto cleanup; } if (!net->ifname || @@ -1072,7 +1072,7 @@ qemudNetworkIfaceConnect(virConnectPtr conn, VIR_FREE(net->ifname); if (!(net->ifname = strdup("vnet%d"))) { virReportOOMError(conn); - return -1; + goto cleanup; } /* avoid exposing vnet%d in dumpxml or error outputs */ template_ifname = 1; @@ -1100,9 +1100,12 @@ qemudNetworkIfaceConnect(virConnectPtr conn, } if (template_ifname) VIR_FREE(net->ifname); - return -1; + tapfd = -1; } +cleanup: + VIR_FREE(brname); + return tapfd; } diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c index ca6d329..222e591 100644 --- a/src/storage_backend_fs.c +++ b/src/storage_backend_fs.c @@ -856,6 +856,8 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn, vol->type = VIR_STORAGE_VOL_FILE; vol->target.format = VIR_STORAGE_VOL_FILE_RAW; /* Real value is filled in during probe */ + if (vol->target.path) + VIR_FREE(vol->target.path); if (virAsprintf(&vol->target.path, "%s/%s", pool->def->target.path, vol->name) == -1) @@ -1022,6 +1024,9 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn, vol->type = VIR_STORAGE_VOL_FILE; + if (vol->target.path) + VIR_FREE(vol->target.path); + if (virAsprintf(&vol->target.path, "%s/%s", pool->def->target.path, vol->name) == -1) { -- 1.6.0.6

4 7

[libvirt] [PATCH] storage_driver.c: remove two useless calls to virStorageBackendForType
by Jim Meyering 04 Sep '09

04 Sep '09

>From dead-store warnings: >From c925b02828083a016f7b9a7ae93eb3a7bf5f4609 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 17:18:29 +0200 Subject: [PATCH] storage_driver.c: remove two useless calls to virStorageBackendForType * src/storage_driver.c (storagePoolGetInfo, storagePoolDefine): The result was never used, and the function has no side effects. --- src/storage_driver.c | 8 -------- 1 files changed, 0 insertions(+), 8 deletions(-) diff --git a/src/storage_driver.c b/src/storage_driver.c index e9ecb20..e8d43b7 100644 --- a/src/storage_driver.c +++ b/src/storage_driver.c @@ -519,15 +519,11 @@ storagePoolDefine(virConnectPtr conn, virStoragePoolDefPtr def; virStoragePoolObjPtr pool = NULL; virStoragePoolPtr ret = NULL; - virStorageBackendPtr backend; storageDriverLock(driver); if (!(def = virStoragePoolDefParseString(conn, xml))) goto cleanup; - if ((backend = virStorageBackendForType(def->type)) == NULL) - goto cleanup; - if (!(pool = virStoragePoolObjAssignDef(conn, &driver->pools, def))) goto cleanup; @@ -847,7 +843,6 @@ storagePoolGetInfo(virStoragePoolPtr obj, virStoragePoolInfoPtr info) { virStorageDriverStatePtr driver = obj->conn->storagePrivateData; virStoragePoolObjPtr pool; - virStorageBackendPtr backend; int ret = -1; storageDriverLock(driver); @@ -860,9 +855,6 @@ storagePoolGetInfo(virStoragePoolPtr obj, goto cleanup; } - if ((backend = virStorageBackendForType(pool->def->type)) == NULL) - goto cleanup; - memset(info, 0, sizeof(virStoragePoolInfo)); if (pool->active) info->state = VIR_STORAGE_POOL_RUNNING; -- 1.6.4.2.409.g85dc3

2 3

[libvirt] [PATCH 6/7] VMware ESX: Add esxDomainUndefine() based on esxVI_UnregisterVM()
by Matthias Bolte 04 Sep '09

04 Sep '09

* src/esx/esx_driver.c: add esxDomainUndefine() based on esxVI_UnregisterVM() * src/esx/esx_vi_methods.[ch]: add esxVI_UnregisterVM()

2 3

[libvirt] [PATCH 5/7] VMware ESX: Add esxGetCapabilities() with basic default capabilities
by Matthias Bolte 04 Sep '09

04 Sep '09

* src/esx/esx_driver.c: add esxCapsInit() with default caps and add esxGetCapabilities()

2 3

[libvirt] [PATCH 1/7] VMware ESX: Fix VMX path parsing and URL encoding
by Matthias Bolte 04 Sep '09

04 Sep '09

* src/esx/esx_driver.c: handle spaces in VMX file path and use a virBuffer to encode spaces correctly in the resulting URL * src/esx/esx_vi.c: include the URL in the error message in case of a download error

2 3

[libvirt] [PATCH] domain_conf.c: remove dead store
by Jim Meyering 04 Sep '09

04 Sep '09

Another dead store: >From 06c5ed162cbdcfec74461e9d09c82244242cc6a2 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 17:27:34 +0200 Subject: [PATCH] domain_conf.c: remove dead store * src/domain_conf.c (virDomainSaveXML): ...and decl of "err". --- src/domain_conf.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/src/domain_conf.c b/src/domain_conf.c index 8dde5dd..4e8ad37 100644 --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -4456,12 +4456,11 @@ int virDomainSaveXML(virConnectPtr conn, char *configFile = NULL; int fd = -1, ret = -1; size_t towrite; - int err; if ((configFile = virDomainConfigFile(conn, configDir, def->name)) == NULL) goto cleanup; - if ((err = virFileMakePath(configDir))) { + if (virFileMakePath(configDir)) { virReportSystemError(conn, errno, _("cannot create config directory '%s'"), configDir); -- 1.6.4.2.409.g85dc3

2 1

[libvirt] [PATCH] xm_internal.c: remove two ret=... dead stores
by Jim Meyering 04 Sep '09

04 Sep '09

Two more dead stores: >From 92bf654b02e5c1eb3334fc13ef52f5b0679512ea Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 17:22:19 +0200 Subject: [PATCH] xm_internal.c: remove two ret=... dead stores * src/xm_internal.c (xenXMDomainCreate): Remove dead stores. --- src/xm_internal.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/xm_internal.c b/src/xm_internal.c index e675672..e948fa0 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -1851,10 +1851,10 @@ int xenXMDomainCreate(virDomainPtr domain) { goto error; domain->id = ret; - if ((ret = xend_wait_for_devices(domain->conn, domain->name)) < 0) + if (xend_wait_for_devices(domain->conn, domain->name) < 0) goto error; - if ((ret = xenDaemonDomainResume(domain)) < 0) + if (xenDaemonDomainResume(domain) < 0) goto error; xenUnifiedUnlock(priv); -- 1.6.4.2.409.g85dc3

2 1

[libvirt] [PATCH] lxc_container.c: avoid a leak on error paths
by Jim Meyering 04 Sep '09

04 Sep '09

This started with a dead-store report: File: lxc_container.c Location: line 417, column 10 Description: Although the value stored to 'rc' is used in the enclosing expression, the value is never actually read from 'rc' But there was a leak, too, upon any but the first failure. This fixes both. >From 141ba8bab2de00c911017bff6fca240c72a80633 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 16:12:35 +0200 Subject: [PATCH] lxc_container.c: avoid a leak on error paths * src/lxc_container.c (lxcContainerMountBasicFS): Don't leak upon failure. Add "cleanup:" label and change each post-allocation failure to use "goto cleanup" rather than returning immediately. --- src/lxc_container.c | 20 ++++++++++++-------- 1 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/lxc_container.c b/src/lxc_container.c index 2073864..f4381e7 100644 --- a/src/lxc_container.c +++ b/src/lxc_container.c @@ -1,6 +1,6 @@ /* * Copyright IBM Corp. 2008 - * Copyright Red Hat 2008 + * Copyright Red Hat 2008-2009 * * lxc_container.c: file description * @@ -384,12 +384,12 @@ static int lxcContainerMountBasicFS(virDomainFSDefPtr root) { "none", "/selinux", "selinuxfs" }, #endif }; - int i, rc; + int i, rc = -1; char *devpts; if (virAsprintf(&devpts, "/.oldroot%s/dev/pts", root->src) < 0) { virReportOOMError(NULL); - return -1; + return rc; } for (i = 0 ; i < ARRAY_CARDINALITY(mnts) ; i++) { @@ -397,31 +397,35 @@ static int lxcContainerMountBasicFS(virDomainFSDefPtr root) virReportSystemError(NULL, errno, _("failed to mkdir %s"), mnts[i].src); - return -1; + goto cleanup; } if (mount(mnts[i].src, mnts[i].dst, mnts[i].type, 0, NULL) < 0) { virReportSystemError(NULL, errno, _("failed to mount %s on %s"), mnts[i].type, mnts[i].type); - return -1; + goto cleanup; } } if ((rc = virFileMakePath("/dev/pts") < 0)) { virReportSystemError(NULL, rc, "%s", _("cannot create /dev/pts")); - return -1; + goto cleanup; } VIR_DEBUG("Trying to move %s to %s", devpts, "/dev/pts"); if ((rc = mount(devpts, "/dev/pts", NULL, MS_MOVE, NULL)) < 0) { virReportSystemError(NULL, errno, "%s", _("failed to mount /dev/pts in container")); - return -1; + goto cleanup; } + + rc = 0; + + cleanup: VIR_FREE(devpts); - return 0; + return rc; } static int lxcContainerPopulateDevices(void) -- 1.6.4.2.409.g85dc3

2 2

[libvirt] Dead code vs. XXX comment: remove or not?
by Jim Meyering 04 Sep '09

04 Sep '09

clang reported that this assignment to type is a dead store, since type is never used after this point. This is xm_internal.c, line 1074: /* XXX Forcing to pretend its a bridge */ if (type == -1) { type = 1; } Normally I'd just remove the whole if block, but the XXX comment makes me think someone has plans for this code. I'll wait for a second opinion.

2 3

[libvirt] address 3 more dead stores
by Jim Meyering 04 Sep '09

04 Sep '09

These changes fix two dead stores and add a comment suggesting why *not* to remove the third one. >From 41c778fdbaef9abf52bc4c3e278d3389d8198c34 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 10:55:55 +0200 Subject: [PATCH 1/3] interface_conf.c: remove a dead-store and declaration * src/interface_conf.c (virInterfaceDefParseDhcp): Remove unused "old". --- src/interface_conf.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/src/interface_conf.c b/src/interface_conf.c index a74d17e..96e0710 100644 --- a/src/interface_conf.c +++ b/src/interface_conf.c @@ -228,11 +228,9 @@ static int virInterfaceDefParseDhcp(virConnectPtr conn, virInterfaceDefPtr def, xmlNodePtr dhcp, xmlXPathContextPtr ctxt) { char *tmp; - xmlNodePtr old; int ret = 0; def->proto.dhcp = 1; - old = ctxt->node; ctxt->node = dhcp; /* Not much to do in the current version */ tmp = virXPathString(conn, "string(./@peerdns)", ctxt); -- 1.6.4.2.409.g85dc3 >From 10037ca58c8d93e427c480082704e7dd52424525 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 11:01:00 +0200 Subject: [PATCH 2/3] hash.c: remove a dead store * src/hash.c (virHashFree): Remove useless assignment to inside_table. --- src/hash.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/src/hash.c b/src/hash.c index 9308c0c..45c5747 100644 --- a/src/hash.c +++ b/src/hash.c @@ -231,7 +231,6 @@ virHashFree(virHashTablePtr table, virHashDeallocator f) inside_table = 0; iter = next; } - inside_table = 0; } VIR_FREE(table->table); } -- 1.6.4.2.409.g85dc3 >From 3b16e6d50d364a5307284a78a7b23d8631146663 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 11:20:23 +0200 Subject: [PATCH 3/3] qemu_conf.c: add a comment suggesting why we leave a dead-store * src/qemu_conf.c (qemuBuildHostNetStr): Do not remove the type_sep=',' dead store, since not having it would be a problem if we ever add a new attribute=VAL option. --- src/qemu_conf.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 2c4a37d..6ffca2f 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -1269,7 +1269,7 @@ qemuBuildHostNetStr(virConnectPtr conn, if (net->hostnet_name) { virBufferVSprintf(&buf, "%cname=%s", type_sep, net->hostnet_name); - type_sep = ','; + type_sep = ','; /* dead-store, but leave it, in case... */ } if (virBufferError(&buf)) { virReportOOMError(conn); -- 1.6.4.2.409.g85dc3

2 1