Re: [libvirt] [PATCH] Fix memory leak in virDomainDefParseXML()
On Fri, Nov 29, 2013 at 10:31:05PM +0530, Nehal J Wani wrote:
This patch fixes the memory leaks found while running
qemuxml2argvtest
==8260== 3 bytes in 1 blocks are definitely lost in loss record 1 of 129
==8260== at 0x4A0887C: malloc (vg_replace_malloc.c:270)
==8260== by 0x341F485E21: strdup (strdup.c:42)
==8260== by 0x4CADCFF: virStrdup (virstring.c:554)
==8260== by 0x4CBB839: virXPathString (virxml.c:90)
==8260== by 0x4CE753A: virDomainDefParseXML (domain_conf.c:11478)
==8260== by 0x4CEB4FE: virDomainDefParseNode (domain_conf.c:12742)
==8260== by 0x4CEB675: virDomainDefParse (domain_conf.c:12684)
==8260== by 0x425958: testCompareXMLToArgvHelper (qemuxml2argvtest.c:107)
==8260== by 0x427111: virtTestRun (testutils.c:138)
==8260== by 0x41D3FE: mymain (qemuxml2argvtest.c:452)
==8260== by 0x4277B2: virtTestMain (testutils.c:593)
==8260== by 0x341F421A04: (below main) (libc-start.c:225)
==8260==
==8260== 4 bytes in 1 blocks are definitely lost in loss record 5 of 129
==8260== at 0x4A0887C: malloc (vg_replace_malloc.c:270)
==8260== by 0x341F485E21: strdup (strdup.c:42)
==8260== by 0x4CADCFF: virStrdup (virstring.c:554)
==8260== by 0x4CBB839: virXPathString (virxml.c:90)
==8260== by 0x4CE753A: virDomainDefParseXML (domain_conf.c:11478)
==8260== by 0x4CEB4FE: virDomainDefParseNode (domain_conf.c:12742)
==8260== by 0x4CEB675: virDomainDefParse (domain_conf.c:12684)
==8260== by 0x425958: testCompareXMLToArgvHelper (qemuxml2argvtest.c:107)
==8260== by 0x427111: virtTestRun (testutils.c:138)
==8260== by 0x41D39A: mymain (qemuxml2argvtest.c:451)
==8260== by 0x4277B2: virtTestMain (testutils.c:593)
==8260== by 0x341F421A04: (below main) (libc-start.c:225)
==8260==
---
src/conf/domain_conf.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 140eb80..5449bd7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11486,6 +11486,7 @@ virDomainDefParseXML(xmlDocPtr xml,
def->features[val] = VIR_DOMAIN_FEATURE_STATE_ON;
}
ctxt->node = node;
+ VIR_FREE(tmp);
break;
case VIR_DOMAIN_FEATURE_LAST:
--
1.8.1.4
I agree that tmp need to be freed in the case block, but we should do
this only if virXPathString() succeeded, leading to the slightly
different patch which I pushed,
thanks,
Daniel
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 140eb80..e0e5395 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11482,6 +11482,7 @@ virDomainDefParseXML(xmlDocPtr xml,
tmp, virDomainFeatureTypeToString(val));
goto error;
}
+ VIR_FREE(tmp);
} else {
def->features[val] = VIR_DOMAIN_FEATURE_STATE_ON;
}
--
Daniel Veillard | Open Source and Standards, Red Hat
veillard(a)redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | virtualization library http://libvirt.org/