Re: [PATCH] Fix memory leak in virDomainDefParseXML()

[Daniel Veillard <veillard@xxxxxxxxxx>]

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@xxxxxxxxxx  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list