[libvirt] [PATCH 0/3] Couple of mem leak fixes

*** BLURB HERE *** Michal Prívozník (3): qemucaps2xmltest: Don't leak @binary libxlDriverConfigDispose: Free @configBaseDir too vmx: Free @firmware in virVMXParseConfig src/libxl/libxl_conf.c | 1 + src/vmx/vmx.c | 1 + tests/qemucaps2xmltest.c | 3 +-- 3 files changed, 3 insertions(+), 2 deletions(-) -- 2.21.0

There's no need to keep @binary around. virQEMUCapsInitGuestFromBinary() duplicates the string anyway. 1,002 bytes in 36 blocks are definitely lost in loss record 54 of 59 at 0x483579F: malloc (vg_replace_malloc.c:299) by 0x796B1C7: vasprintf (vasprintf.c:73) by 0x4C3F2C6: virVasprintfInternal (virstring.c:740) by 0x4C3F3DC: virAsprintfInternal (virstring.c:761) by 0x13AFC9: testGetCaps (qemucaps2xmltest.c:105) by 0x13B200: testQemuCapsXML (qemucaps2xmltest.c:157) by 0x13B642: virTestRun (testutils.c:174) by 0x13B366: doCapsTest (qemucaps2xmltest.c:191) by 0x13FF2B: testQemuCapsIterate (testutilsqemu.c:941) by 0x13B427: mymain (qemucaps2xmltest.c:215) by 0x13D706: virTestMain (testutils.c:1096) by 0x13B489: main (qemucaps2xmltest.c:221) Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemucaps2xmltest.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/qemucaps2xmltest.c b/tests/qemucaps2xmltest.c index d107b32221..08dc598541 100644 --- a/tests/qemucaps2xmltest.c +++ b/tests/qemucaps2xmltest.c @@ -100,7 +100,7 @@ testGetCaps(char *capsData, const testQemuData *data) virQEMUCapsPtr qemuCaps = NULL; virCapsPtr caps = NULL; virArch arch = virArchFromString(data->archName); - char *binary = NULL; + VIR_AUTOFREE(char *) binary = NULL; if (virAsprintf(&binary, "/usr/bin/qemu-system-%s", data->archName) < 0) goto error; @@ -129,7 +129,6 @@ testGetCaps(char *capsData, const testQemuData *data) error: virObjectUnref(qemuCaps); virObjectUnref(caps); - VIR_FREE(binary); return NULL; } -- 2.21.0

On Tue, Apr 23, 2019 at 09:39:31 +0200, Michal Privoznik wrote:
There's no need to keep @binary around. virQEMUCapsInitGuestFromBinary() duplicates the string anyway.
1,002 bytes in 36 blocks are definitely lost in loss record 54 of 59 at 0x483579F: malloc (vg_replace_malloc.c:299) by 0x796B1C7: vasprintf (vasprintf.c:73) by 0x4C3F2C6: virVasprintfInternal (virstring.c:740) by 0x4C3F3DC: virAsprintfInternal (virstring.c:761) by 0x13AFC9: testGetCaps (qemucaps2xmltest.c:105) by 0x13B200: testQemuCapsXML (qemucaps2xmltest.c:157) by 0x13B642: virTestRun (testutils.c:174) by 0x13B366: doCapsTest (qemucaps2xmltest.c:191) by 0x13FF2B: testQemuCapsIterate (testutilsqemu.c:941) by 0x13B427: mymain (qemucaps2xmltest.c:215) by 0x13D706: virTestMain (testutils.c:1096) by 0x13B489: main (qemucaps2xmltest.c:221)
Caused by 562990849a9de255b5fefa39be3f301b1bddfa6e
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemucaps2xmltest.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/tests/qemucaps2xmltest.c b/tests/qemucaps2xmltest.c index d107b32221..08dc598541 100644 --- a/tests/qemucaps2xmltest.c +++ b/tests/qemucaps2xmltest.c @@ -100,7 +100,7 @@ testGetCaps(char *capsData, const testQemuData *data) virQEMUCapsPtr qemuCaps = NULL; virCapsPtr caps = NULL; virArch arch = virArchFromString(data->archName); - char *binary = NULL; + VIR_AUTOFREE(char *) binary = NULL;
if (virAsprintf(&binary, "/usr/bin/qemu-system-%s", data->archName) < 0) goto error; @@ -129,7 +129,6 @@ testGetCaps(char *capsData, const testQemuData *data) error: virObjectUnref(qemuCaps); virObjectUnref(caps); - VIR_FREE(binary); return NULL;
ACK

Allocated in libxlDriverConfigNew(), the @configBaseDir is never freed. 13 bytes in 1 blocks are definitely lost in loss record 36 of 125 at 0x483579F: malloc (vg_replace_malloc.c:299) by 0x8012469: strdup (strdup.c:42) by 0x52926DE: virStrdup (virstring.c:966) by 0x11D46B: libxlDriverConfigNew (libxl_conf.c:1749) by 0x114D78: testCompareXMLToDomConfig (libxlxml2domconfigtest.c:62) by 0x1152A3: testCompareXMLToDomConfigHelper (libxlxml2domconfigtest.c:160) by 0x115925: virTestRun (testutils.c:174) by 0x1154A4: mymain (libxlxml2domconfigtest.c:216) by 0x1179E9: virTestMain (testutils.c:1096) by 0x1154FD: main (libxlxml2domconfigtest.c:224) Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libxl/libxl_conf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index c701df3370..315c14652b 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -79,6 +79,7 @@ libxlDriverConfigDispose(void *obj) if (cfg->logger) libxlLoggerFree(cfg->logger); + VIR_FREE(cfg->configBaseDir); VIR_FREE(cfg->configDir); VIR_FREE(cfg->autostartDir); VIR_FREE(cfg->logDir); -- 2.21.0

On Tue, Apr 23, 2019 at 09:39:32 +0200, Michal Privoznik wrote:
Allocated in libxlDriverConfigNew(), the @configBaseDir is never freed.
13 bytes in 1 blocks are definitely lost in loss record 36 of 125 at 0x483579F: malloc (vg_replace_malloc.c:299) by 0x8012469: strdup (strdup.c:42) by 0x52926DE: virStrdup (virstring.c:966) by 0x11D46B: libxlDriverConfigNew (libxl_conf.c:1749) by 0x114D78: testCompareXMLToDomConfig (libxlxml2domconfigtest.c:62) by 0x1152A3: testCompareXMLToDomConfigHelper (libxlxml2domconfigtest.c:160) by 0x115925: virTestRun (testutils.c:174) by 0x1154A4: mymain (libxlxml2domconfigtest.c:216) by 0x1179E9: virTestMain (testutils.c:1096) by 0x1154FD: main (libxlxml2domconfigtest.c:224)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libxl/libxl_conf.c | 1 + 1 file changed, 1 insertion(+)
ACK

The @firmware string is allocated, but never freed. 4 bytes in 1 blocks are definitely lost in loss record 1 of 44 at 0x483579F: malloc (vg_replace_malloc.c:299) by 0x76FB469: strdup (strdup.c:42) by 0x497B6DE: virStrdup (virstring.c:966) by 0x48F6FD3: virConfGetValueString (virconf.c:908) by 0x4B3E9B6: virVMXGetConfigStringHelper (vmx.c:736) by 0x4B3EA6B: virVMXGetConfigString (vmx.c:756) by 0x4B41AEA: virVMXParseConfig (vmx.c:1832) by 0x10B8E4: testCompareFiles (vmx2xmltest.c:79) by 0x10BAB8: testCompareHelper (vmx2xmltest.c:124) by 0x10D058: virTestRun (testutils.c:174) by 0x10CDDA: mymain (vmx2xmltest.c:288) by 0x10F11C: virTestMain (testutils.c:1096) Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/vmx/vmx.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 35b83a2320..7bb5eb956d 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -1861,6 +1861,7 @@ virVMXParseConfig(virVMXContext *ctx, VIR_FREE(sched_cpu_shares); VIR_FREE(guestOS); virCPUDefFree(cpu); + VIR_FREE(firmware); return def; } -- 2.21.0

On Tuesday, 23 April 2019 09:39:33 CEST Michal Privoznik wrote:
The @firmware string is allocated, but never freed.
4 bytes in 1 blocks are definitely lost in loss record 1 of 44 at 0x483579F: malloc (vg_replace_malloc.c:299) by 0x76FB469: strdup (strdup.c:42) by 0x497B6DE: virStrdup (virstring.c:966) by 0x48F6FD3: virConfGetValueString (virconf.c:908) by 0x4B3E9B6: virVMXGetConfigStringHelper (vmx.c:736) by 0x4B3EA6B: virVMXGetConfigString (vmx.c:756) by 0x4B41AEA: virVMXParseConfig (vmx.c:1832) by 0x10B8E4: testCompareFiles (vmx2xmltest.c:79) by 0x10BAB8: testCompareHelper (vmx2xmltest.c:124) by 0x10D058: virTestRun (testutils.c:174) by 0x10CDDA: mymain (vmx2xmltest.c:288) by 0x10F11C: virTestMain (testutils.c:1096)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/vmx/vmx.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 35b83a2320..7bb5eb956d 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -1861,6 +1861,7 @@ virVMXParseConfig(virVMXContext *ctx, VIR_FREE(sched_cpu_shares); VIR_FREE(guestOS); virCPUDefFree(cpu); + VIR_FREE(firmware);
Oops, my bad. The alternative is to make @firmware an autofree variable, although for consistency this is better. Reviewed-by: Pino Toscano <ptoscano@redhat.com> -- Pino Toscano
participants (3)
-
Michal Privoznik
-
Peter Krempa
-
Pino Toscano