[libvirt] [PATCH 0/5] Fix couple of memleaks
*** BLURB HERE *** Michal Privoznik (5): tests: Trace re-executing processes with valgrind virusbmock: Link with libvirt_utils testUSBList: don't leak @dev qemuMonitorCPUModelInfoFree: Don't leak model_info->props xenFoxenFormatXLDisk: Don't leak @target src/qemu/qemu_monitor.c | 1 + src/xenconfig/xen_xl.c | 5 +++-- tests/Makefile.am | 6 ++++-- tests/virusbtest.c | 1 + 4 files changed, 9 insertions(+), 4 deletions(-) -- 2.11.0
A lot of our tests re-execute themeselves after loading their mock library. This, however, makes valgrind sad because currently we do not tell it to trace the process after exec(). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/Makefile.am | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index e923178f2..521728a3d 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -470,7 +470,8 @@ TESTS_ENVIRONMENT = \ $(VG) -VALGRIND = valgrind --quiet --leak-check=full \ +VALGRIND = valgrind --quiet --leak-check=full --trace-children=yes \ + --trace-children-skip="*/tools/virsh","*/tests/commandhelper" \ --suppressions=$(srcdir)/.valgrind.supp valgrind: $(MAKE) check VG="libtool --mode=execute $(VALGRIND)" -- 2.11.0
On 02/06/2017 03:21 AM, Michal Privoznik wrote:
A lot of our tests re-execute themeselves after loading their mock library. This, however, makes valgrind sad because currently we do not tell it to trace the process after exec().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/Makefile.am | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
ACK (seems reasonable) John
We are using couple of functions from there (e.g. virStrdup) and rely that the binary linking us has the libvirt_utils linked already. Well, this makes valgrind sad. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/Makefile.am | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index 521728a3d..7149a865e 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1203,7 +1203,8 @@ virnetdevbandwidthtest_LDADD = $(LDADDS) $(LIBXML_LIBS) virusbmock_la_SOURCES = virusbmock.c virusbmock_la_CFLAGS = $(AM_CFLAGS) virusbmock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS) -virusbmock_la_LIBADD = $(MOCKLIBS_LIBS) +virusbmock_la_LIBADD = $(MOCKLIBS_LIBS) \ + ../src/libvirt_util.la virnetdevbandwidthmock_la_SOURCES = \ virnetdevbandwidthmock.c -- 2.11.0
On 02/06/2017 03:21 AM, Michal Privoznik wrote:
We are using couple of functions from there (e.g. virStrdup) and rely that the binary linking us has the libvirt_utils linked already. Well, this makes valgrind sad.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/Makefile.am | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
ACK John (I did have some weirdness w/ virusbtest and failing to run because libvirt_event_poll_purge_timeout_semaphore wasn't found, but after a make clean it got cleared up - I dunno...)
==22187== 77 (56 direct, 21 indirect) bytes in 1 blocks are definitely lost in loss record 23 of 37 ==22187== at 0x4C2BC75: calloc (vg_replace_malloc.c:624) ==22187== by 0x4E75685: virAlloc (viralloc.c:144) ==22187== by 0x4F0613A: virUSBDeviceNew (virusb.c:332) ==22187== by 0x4F05BA2: virUSBDeviceSearch (virusb.c:183) ==22187== by 0x4F05F95: virUSBDeviceFind (virusb.c:296) ==22187== by 0x403514: testUSBList (virusbtest.c:209) ==22187== by 0x403BD8: virTestRun (testutils.c:180) ==22187== by 0x4039E5: mymain (virusbtest.c:285) ==22187== by 0x4056BC: virTestMain (testutils.c:992) ==22187== by 0x403A4A: main (virusbtest.c:293) Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/virusbtest.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/virusbtest.c b/tests/virusbtest.c index 4bbfe4af9..7cddb89af 100644 --- a/tests/virusbtest.c +++ b/tests/virusbtest.c @@ -217,6 +217,7 @@ testUSBList(const void *opaque ATTRIBUTE_UNUSED) } virUSBDeviceListDel(list, dev); + virUSBDeviceFree(dev); dev = NULL; if (testCheckNdevs("After deleting one", -- 2.11.0
On 02/06/2017 03:21 AM, Michal Privoznik wrote:
==22187== 77 (56 direct, 21 indirect) bytes in 1 blocks are definitely lost in loss record 23 of 37 ==22187== at 0x4C2BC75: calloc (vg_replace_malloc.c:624) ==22187== by 0x4E75685: virAlloc (viralloc.c:144) ==22187== by 0x4F0613A: virUSBDeviceNew (virusb.c:332) ==22187== by 0x4F05BA2: virUSBDeviceSearch (virusb.c:183) ==22187== by 0x4F05F95: virUSBDeviceFind (virusb.c:296) ==22187== by 0x403514: testUSBList (virusbtest.c:209) ==22187== by 0x403BD8: virTestRun (testutils.c:180) ==22187== by 0x4039E5: mymain (virusbtest.c:285) ==22187== by 0x4056BC: virTestMain (testutils.c:992) ==22187== by 0x403A4A: main (virusbtest.c:293)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/virusbtest.c | 1 + 1 file changed, 1 insertion(+)
ACK John
==11846== 240 bytes in 1 blocks are definitely lost in loss record 81 of 107 ==11846== at 0x4C2BC75: calloc (vg_replace_malloc.c:624) ==11846== by 0x18C74242: virAllocN (viralloc.c:191) ==11846== by 0x4A05E8: qemuMonitorCPUModelInfoCopy (qemu_monitor.c:3677) ==11846== by 0x446E3C: virQEMUCapsNewCopy (qemu_capabilities.c:2171) ==11846== by 0x437335: testQemuCapsCopy (qemucapabilitiestest.c:108) ==11846== by 0x437CD2: virTestRun (testutils.c:180) ==11846== by 0x437AD8: mymain (qemucapabilitiestest.c:176) ==11846== by 0x4397B6: virTestMain (testutils.c:992) ==11846== by 0x437B44: main (qemucapabilitiestest.c:188) Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_monitor.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index b7be5e7f4..565339921 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3660,6 +3660,7 @@ qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info) for (i = 0; i < model_info->nprops; i++) VIR_FREE(model_info->props[i].name); + VIR_FREE(model_info->props); VIR_FREE(model_info->name); VIR_FREE(model_info); } -- 2.11.0
On 02/06/2017 03:21 AM, Michal Privoznik wrote:
==11846== 240 bytes in 1 blocks are definitely lost in loss record 81 of 107 ==11846== at 0x4C2BC75: calloc (vg_replace_malloc.c:624) ==11846== by 0x18C74242: virAllocN (viralloc.c:191) ==11846== by 0x4A05E8: qemuMonitorCPUModelInfoCopy (qemu_monitor.c:3677) ==11846== by 0x446E3C: virQEMUCapsNewCopy (qemu_capabilities.c:2171) ==11846== by 0x437335: testQemuCapsCopy (qemucapabilitiestest.c:108) ==11846== by 0x437CD2: virTestRun (testutils.c:180) ==11846== by 0x437AD8: mymain (qemucapabilitiestest.c:176) ==11846== by 0x4397B6: virTestMain (testutils.c:992) ==11846== by 0x437B44: main (qemucapabilitiestest.c:188)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_monitor.c | 1 + 1 file changed, 1 insertion(+)
ACK John
==11260== 1,006 bytes in 1 blocks are definitely lost in loss record 106 of 111 ==11260== at 0x4C2AE5F: malloc (vg_replace_malloc.c:297) ==11260== by 0x4C2BDFF: realloc (vg_replace_malloc.c:693) ==11260== by 0x4EA430B: virReallocN (viralloc.c:245) ==11260== by 0x4EA7C52: virBufferGrow (virbuffer.c:130) ==11260== by 0x4EA7D28: virBufferAdd (virbuffer.c:165) ==11260== by 0x4EA8E10: virBufferStrcat (virbuffer.c:718) ==11260== by 0x42D263: xenFormatXLDiskSrcNet (xen_xl.c:960) ==11260== by 0x42D4EB: xenFormatXLDiskSrc (xen_xl.c:1015) ==11260== by 0x42D870: xenFormatXLDisk (xen_xl.c:1101) ==11260== by 0x42DA89: xenFormatXLDomainDisks (xen_xl.c:1148) ==11260== by 0x42EAF8: xenFormatXL (xen_xl.c:1558) ==11260== by 0x40E85F: testCompareParseXML (xlconfigtest.c:105) Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/xenconfig/xen_xl.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c index 18d9fe369..2c9174e53 100644 --- a/src/xenconfig/xen_xl.c +++ b/src/xenconfig/xen_xl.c @@ -1034,6 +1034,7 @@ xenFormatXLDisk(virConfValuePtr list, virDomainDiskDefPtr disk) int format = virDomainDiskGetFormat(disk); const char *driver = virDomainDiskGetDriver(disk); char *target = NULL; + int ret = -1; /* format */ virBufferAddLit(&buf, "format="); @@ -1119,12 +1120,12 @@ xenFormatXLDisk(virConfValuePtr list, virDomainDiskDefPtr disk) tmp->next = val; else list->list = val; - return 0; + ret = 0; cleanup: VIR_FREE(target); virBufferFreeAndReset(&buf); - return -1; + return ret; } -- 2.11.0
Michal Privoznik wrote:
==11260== 1,006 bytes in 1 blocks are definitely lost in loss record 106 of 111 ==11260== at 0x4C2AE5F: malloc (vg_replace_malloc.c:297) ==11260== by 0x4C2BDFF: realloc (vg_replace_malloc.c:693) ==11260== by 0x4EA430B: virReallocN (viralloc.c:245) ==11260== by 0x4EA7C52: virBufferGrow (virbuffer.c:130) ==11260== by 0x4EA7D28: virBufferAdd (virbuffer.c:165) ==11260== by 0x4EA8E10: virBufferStrcat (virbuffer.c:718) ==11260== by 0x42D263: xenFormatXLDiskSrcNet (xen_xl.c:960) ==11260== by 0x42D4EB: xenFormatXLDiskSrc (xen_xl.c:1015) ==11260== by 0x42D870: xenFormatXLDisk (xen_xl.c:1101) ==11260== by 0x42DA89: xenFormatXLDomainDisks (xen_xl.c:1148) ==11260== by 0x42EAF8: xenFormatXL (xen_xl.c:1558) ==11260== by 0x40E85F: testCompareParseXML (xlconfigtest.c:105)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Strange function name in $subject. s/xenFoxenFormatXLDisk/xenFormatXLDisk/ ACK. Regards, Jim
--- src/xenconfig/xen_xl.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c index 18d9fe369..2c9174e53 100644 --- a/src/xenconfig/xen_xl.c +++ b/src/xenconfig/xen_xl.c @@ -1034,6 +1034,7 @@ xenFormatXLDisk(virConfValuePtr list, virDomainDiskDefPtr disk) int format = virDomainDiskGetFormat(disk); const char *driver = virDomainDiskGetDriver(disk); char *target = NULL; + int ret = -1;
/* format */ virBufferAddLit(&buf, "format="); @@ -1119,12 +1120,12 @@ xenFormatXLDisk(virConfValuePtr list, virDomainDiskDefPtr disk) tmp->next = val; else list->list = val; - return 0; + ret = 0;
cleanup: VIR_FREE(target); virBufferFreeAndReset(&buf); - return -1; + return ret; }
On 02/06/2017 08:18 PM, Jim Fehlig wrote:
Michal Privoznik wrote:
==11260== 1,006 bytes in 1 blocks are definitely lost in loss record 106 of 111 ==11260== at 0x4C2AE5F: malloc (vg_replace_malloc.c:297) ==11260== by 0x4C2BDFF: realloc (vg_replace_malloc.c:693) ==11260== by 0x4EA430B: virReallocN (viralloc.c:245) ==11260== by 0x4EA7C52: virBufferGrow (virbuffer.c:130) ==11260== by 0x4EA7D28: virBufferAdd (virbuffer.c:165) ==11260== by 0x4EA8E10: virBufferStrcat (virbuffer.c:718) ==11260== by 0x42D263: xenFormatXLDiskSrcNet (xen_xl.c:960) ==11260== by 0x42D4EB: xenFormatXLDiskSrc (xen_xl.c:1015) ==11260== by 0x42D870: xenFormatXLDisk (xen_xl.c:1101) ==11260== by 0x42DA89: xenFormatXLDomainDisks (xen_xl.c:1148) ==11260== by 0x42EAF8: xenFormatXL (xen_xl.c:1558) ==11260== by 0x40E85F: testCompareParseXML (xlconfigtest.c:105)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Strange function name in $subject. s/xenFoxenFormatXLDisk/xenFormatXLDisk/
Ah, indeed. I've started writing the function name and then copy-pasted it. I am a giddy goat :-)
ACK.
Thanks, I've pushed this one. Michal
participants (3)
-
Jim Fehlig -
John Ferlan -
Michal Privoznik