[libvirt] [PATCH 0/6] fix a serial of memleaks
Zhang Bo (6): tests: fix some memleaks in tests util: fix memleak in virFindSCSIHostByPCI qemu: fix memleaks in qemuBuildCommandLine qemu: fix memleak in virCapabilitiesDomainDataLookup conf: fix memleak in virDomainNetIpParseXML conf: fix memleak in virDomainHostdevDefClear src/conf/capabilities.c | 5 +++-- src/conf/domain_conf.c | 18 ++++++++++-------- src/qemu/qemu_command.c | 2 ++ src/util/virutil.c | 3 +++ tests/commandtest.c | 1 + tests/domaincapstest.c | 1 + tests/qemucommandutiltest.c | 1 + 7 files changed, 21 insertions(+), 10 deletions(-) -- 1.7.12.4
Signed-off-by: Zhang Bo <oscar.zhangbo@huawei.com> --- tests/commandtest.c | 1 + tests/domaincapstest.c | 1 + tests/qemucommandutiltest.c | 1 + 3 files changed, 3 insertions(+) diff --git a/tests/commandtest.c b/tests/commandtest.c index 6400ea2..f001a39 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -1081,6 +1081,7 @@ static int test24(const void *unused ATTRIBUTE_UNUSED) if (pidfile) unlink(pidfile); VIR_FREE(pidfile); + VIR_FREE(prefix); virCommandFree(cmd); VIR_FORCE_CLOSE(newfd1); /* coverity[double_close] */ diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c index f6a060e..ecefdb9 100644 --- a/tests/domaincapstest.c +++ b/tests/domaincapstest.c @@ -242,6 +242,7 @@ mymain(void) ret = -1; \ } else if (virtTestRun(Filename, test_virDomainCapsFormat, &data) < 0) \ ret = -1; \ + virObjectUnref(qemuCaps); \ } while (0) DO_TEST_QEMU("qemu_1.6.50-1", "caps_1.6.50-1", "/usr/bin/qemu-system-x86_64", diff --git a/tests/qemucommandutiltest.c b/tests/qemucommandutiltest.c index 8c52f02..bd457f8 100644 --- a/tests/qemucommandutiltest.c +++ b/tests/qemucommandutiltest.c @@ -66,6 +66,7 @@ testQemuCommandBuildObjectFromJSON(const void *opaque) cleanup: virJSONValueFree(val); VIR_FREE(result); + VIR_FREE(expect); return ret; } -- 1.7.12.4
free buf in cleanup. Signed-off-by: Zhang Bo <oscar.zhangbo@huawei.com> --- src/util/virutil.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/util/virutil.c b/src/util/virutil.c index 79cdb7a..0426517 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1815,6 +1815,8 @@ virFindSCSIHostByPCI(const char *sysfs_prefix, if (virStrToLong_ui(buf, NULL, 10, &read_unique_id) < 0) goto cleanup; + VIR_FREE(buf); + if (read_unique_id != unique_id) { VIR_FREE(unique_path); continue; @@ -1829,6 +1831,7 @@ virFindSCSIHostByPCI(const char *sysfs_prefix, VIR_FREE(unique_path); VIR_FREE(host_link); VIR_FREE(host_path); + VIR_FREE(buf); return ret; } -- 1.7.12.4
free boot_opts_str and boot_order_str both in normal and error paths. Signed-off-by: Zhang Bo <oscar.zhangbo@huawei.com> --- src/qemu/qemu_command.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 29b876e..a54f3a3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9266,6 +9266,7 @@ qemuBuildCommandLine(virConnectPtr conn, } } VIR_FREE(boot_opts_str); + VIR_FREE(boot_order_str); if (def->os.kernel) virCommandAddArgList(cmd, "-kernel", def->os.kernel, NULL); @@ -10746,6 +10747,7 @@ qemuBuildCommandLine(virConnectPtr conn, error: VIR_FREE(boot_order_str); + VIR_FREE(boot_opts_str); virBufferFreeAndReset(&boot_buf); virObjectUnref(cfg); /* free up any resources in the network driver -- 1.7.12.4
virBufferContentAndReset() doesn't free buf contents, we should use virBufferFreeAndReset() to get buf freed. Signed-off-by: Zhang Bo <oscar.zhangbo@huawei.com> --- src/conf/capabilities.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 2c674a8..922741f 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -701,13 +701,14 @@ virCapabilitiesDomainDataLookup(virCapsPtr caps, !virBufferCurrentContent(&buf)[0]) virBufferAsprintf(&buf, "%s", _("any configuration")); if (virBufferCheckError(&buf) < 0) { - virBufferContentAndReset(&buf); + virBufferFreeAndReset(&buf); goto error; } virReportError(VIR_ERR_INVALID_ARG, _("could not find capabilities for %s"), - virBufferContentAndReset(&buf)); + virBufferCurrentContent(&buf)); + virBufferFreeAndReset(&buf); goto error; } -- 1.7.12.4
use cleanup instead of error, so that the allocated strings could also get freed when there's no error. Signed-off-by: Zhang Bo <oscar.zhangbo@huawei.com> --- src/conf/domain_conf.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 41963cc..8350fe7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5129,6 +5129,7 @@ virDomainNetIpParseXML(xmlNodePtr node) char *familyStr = NULL; int family = AF_UNSPEC; char *address = NULL; + int ret = -1; if (!(prefixStr = virXMLPropString(node, "prefix")) || (virStrToLong_ui(prefixStr, NULL, 10, &prefixValue) < 0)) { @@ -5139,7 +5140,7 @@ virDomainNetIpParseXML(xmlNodePtr node) if (!(address = virXMLPropString(node, "address"))) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("Missing network address")); - goto error; + goto cleanup; } familyStr = virXMLPropString(node, "family"); @@ -5151,24 +5152,25 @@ virDomainNetIpParseXML(xmlNodePtr node) family = virSocketAddrNumericFamily(address); if (VIR_ALLOC(ip) < 0) - goto error; + goto cleanup; if (virSocketAddrParse(&ip->address, address, family) < 0) { virReportError(VIR_ERR_INVALID_ARG, _("Failed to parse IP address: '%s'"), address); - goto error; + goto cleanup; } ip->prefix = prefixValue; - return ip; + ret = 0; - error: + cleanup: VIR_FREE(prefixStr); VIR_FREE(familyStr); VIR_FREE(address); - VIR_FREE(ip); - return NULL; + if (ret) + VIR_FREE(ip); + return ip; } static int -- 1.7.12.4
On Mon, Apr 27, 2015 at 02:41:44PM +0800, Zhang Bo wrote:
use cleanup instead of error, so that the allocated strings could also get freed when there's no error.
Signed-off-by: Zhang Bo <oscar.zhangbo@huawei.com> --- src/conf/domain_conf.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 41963cc..8350fe7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5129,6 +5129,7 @@ virDomainNetIpParseXML(xmlNodePtr node) char *familyStr = NULL; int family = AF_UNSPEC; char *address = NULL; + int ret = -1;
The 'ret' variable should contain the return value of the function. I am squashing this in: diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b2640b0..1b520b9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5124,13 +5124,12 @@ static virDomainNetIpDefPtr virDomainNetIpParseXML(xmlNodePtr node) { /* Parse the prefix in every case */ - virDomainNetIpDefPtr ip = NULL; + virDomainNetIpDefPtr ip = NULL, ret = NULL; char *prefixStr = NULL; unsigned int prefixValue = 0; char *familyStr = NULL; int family = AF_UNSPEC; char *address = NULL; - int ret = -1; if (!(prefixStr = virXMLPropString(node, "prefix")) || (virStrToLong_ui(prefixStr, NULL, 10, &prefixValue) < 0)) { @@ -5163,15 +5162,15 @@ virDomainNetIpParseXML(xmlNodePtr node) } ip->prefix = prefixValue; - ret = 0; + ret = ip; + ip = NULL; cleanup: VIR_FREE(prefixStr); VIR_FREE(familyStr); VIR_FREE(address); - if (ret) - VIR_FREE(ip); - return ip; + VIR_FREE(ip); + return ret; } Jan
use virNetworkRouteDefFree() instead of VIR_FREE to free routes, otherwise the element 'family' would not be freed. Signed-off-by: Zhang Bo <oscar.zhangbo@huawei.com> --- src/conf/domain_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8350fe7..f383104 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1881,7 +1881,7 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def) VIR_FREE(def->source.caps.u.net.ips[i]); VIR_FREE(def->source.caps.u.net.ips); for (i = 0; i < def->source.caps.u.net.nroutes; i++) - VIR_FREE(def->source.caps.u.net.routes[i]); + virNetworkRouteDefFree(def->source.caps.u.net.routes[i]); VIR_FREE(def->source.caps.u.net.routes); break; } -- 1.7.12.4
On Mon, Apr 27, 2015 at 02:41:39PM +0800, Zhang Bo wrote:
Zhang Bo (6): tests: fix some memleaks in tests util: fix memleak in virFindSCSIHostByPCI qemu: fix memleaks in qemuBuildCommandLine qemu: fix memleak in virCapabilitiesDomainDataLookup conf: fix memleak in virDomainNetIpParseXML conf: fix memleak in virDomainHostdevDefClear
src/conf/capabilities.c | 5 +++-- src/conf/domain_conf.c | 18 ++++++++++-------- src/qemu/qemu_command.c | 2 ++ src/util/virutil.c | 3 +++ tests/commandtest.c | 1 + tests/domaincapstest.c | 1 + tests/qemucommandutiltest.c | 1 + 7 files changed, 21 insertions(+), 10 deletions(-)
ACK series and pushed. Jan
participants (2)
-
Ján Tomko -
Zhang Bo