[libvirt] [PATCH 0/3] Some random fixes
*** BLURB HERE *** Michal Prívozník (3): virPCIGetNetName: Initialize @netname to NULL virpci: Fix memleak in virPCIDeviceIterDevices domain_conf: Free egl render node in virDomainGraphicsDefFree src/conf/domain_conf.c | 3 +++ src/util/virnetdev.c | 2 -- src/util/virpci.c | 4 ++++ 3 files changed, 7 insertions(+), 2 deletions(-) -- 2.19.2
This is a return argument that is to be compared against NULL on successful return. However, it is not initialized and therefore relies on callers setting it to NULL prior calling the function. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virnetdev.c | 2 -- src/util/virpci.c | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 92ef008ca1..f3e3d442ed 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1445,8 +1445,6 @@ virNetDevGetVirtualFunctionInfo(const char *vfname, char **pfname, { int ret = -1; - *pfname = NULL; - if (virNetDevGetPhysicalFunction(vfname, pfname) < 0) return -1; diff --git a/src/util/virpci.c b/src/util/virpci.c index 537876bcba..ef578bf774 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -2853,6 +2853,8 @@ virPCIGetNetName(const char *device_link_sysfs_path, struct dirent *entry = NULL; size_t i = 0; + *netname = NULL; + if (virBuildPath(&pcidev_sysfs_net_path, device_link_sysfs_path, "net") == -1) { virReportOOMError(); -- 2.19.2
On Wed, Jan 23, 2019 at 04:45:06PM +0100, Michal Privoznik wrote:
This is a return argument that is to be compared against NULL on successful return. However, it is not initialized and therefore relies on callers setting it to NULL prior calling the function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- Reviewed-by: Erik Skultety <eskultet@redhat.com>
This partially reverts 00dc991ca167302c7a72f4fb16be061d05b12a32. 2,030 (1,456 direct, 574 indirect) bytes in 14 blocks are definitely lost in loss record 77 of 80 at 0x4C30E96: calloc (vg_replace_malloc.c:711) by 0x50F83AA: virAlloc (viralloc.c:143) by 0x5178DFA: virPCIDeviceNew (virpci.c:1753) by 0x51753E9: virPCIDeviceIterDevices (virpci.c:468) by 0x5175EB5: virPCIDeviceGetParent (virpci.c:759) by 0x517AB55: virPCIDeviceIsBehindSwitchLackingACS (virpci.c:2476) by 0x517AC24: virPCIDeviceIsAssignable (virpci.c:2494) by 0x10BF27: testVirPCIDeviceIsAssignable (virpcitest.c:229) by 0x10D14C: virTestRun (testutils.c:174) by 0x10C535: mymain (virpcitest.c:422) by 0x10F1B6: virTestMain (testutils.c:1112) by 0x10CF93: main (virpcitest.c:455) Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virpci.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util/virpci.c b/src/util/virpci.c index ef578bf774..fb22460bf5 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -482,6 +482,8 @@ virPCIDeviceIterDevices(virPCIDeviceIterPredicate predicate, *matched = check; ret = 1; break; + } else { + virPCIDeviceFree(check); } } VIR_DIR_CLOSE(dir); -- 2.19.2
On Wed, Jan 23, 2019 at 04:45:07PM +0100, Michal Privoznik wrote:
This partially reverts 00dc991ca167302c7a72f4fb16be061d05b12a32.
2,030 (1,456 direct, 574 indirect) bytes in 14 blocks are definitely lost in loss record 77 of 80 at 0x4C30E96: calloc (vg_replace_malloc.c:711) by 0x50F83AA: virAlloc (viralloc.c:143) by 0x5178DFA: virPCIDeviceNew (virpci.c:1753) by 0x51753E9: virPCIDeviceIterDevices (virpci.c:468) by 0x5175EB5: virPCIDeviceGetParent (virpci.c:759) by 0x517AB55: virPCIDeviceIsBehindSwitchLackingACS (virpci.c:2476) by 0x517AC24: virPCIDeviceIsAssignable (virpci.c:2494) by 0x10BF27: testVirPCIDeviceIsAssignable (virpcitest.c:229) by 0x10D14C: virTestRun (testutils.c:174) by 0x10C535: mymain (virpcitest.c:422) by 0x10F1B6: virTestMain (testutils.c:1112) by 0x10CF93: main (virpcitest.c:455)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virpci.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/src/util/virpci.c b/src/util/virpci.c index ef578bf774..fb22460bf5 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -482,6 +482,8 @@ virPCIDeviceIterDevices(virPCIDeviceIterPredicate predicate, *matched = check;
I think that instead of adding a new condition block, we could do VIR_STEAL_PTR ^here and then...
ret = 1; break; + } else { + virPCIDeviceFree(check); }
do virPCIDeviceFree(check) unconditionally. IMHO it would also look more clean. Reviewed-by: Erik Skultety <eskultet@redhat.com>
On 1/23/19 5:24 PM, Erik Skultety wrote:
On Wed, Jan 23, 2019 at 04:45:07PM +0100, Michal Privoznik wrote:
This partially reverts 00dc991ca167302c7a72f4fb16be061d05b12a32.
2,030 (1,456 direct, 574 indirect) bytes in 14 blocks are definitely lost in loss record 77 of 80 at 0x4C30E96: calloc (vg_replace_malloc.c:711) by 0x50F83AA: virAlloc (viralloc.c:143) by 0x5178DFA: virPCIDeviceNew (virpci.c:1753) by 0x51753E9: virPCIDeviceIterDevices (virpci.c:468) by 0x5175EB5: virPCIDeviceGetParent (virpci.c:759) by 0x517AB55: virPCIDeviceIsBehindSwitchLackingACS (virpci.c:2476) by 0x517AC24: virPCIDeviceIsAssignable (virpci.c:2494) by 0x10BF27: testVirPCIDeviceIsAssignable (virpcitest.c:229) by 0x10D14C: virTestRun (testutils.c:174) by 0x10C535: mymain (virpcitest.c:422) by 0x10F1B6: virTestMain (testutils.c:1112) by 0x10CF93: main (virpcitest.c:455)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virpci.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/src/util/virpci.c b/src/util/virpci.c index ef578bf774..fb22460bf5 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -482,6 +482,8 @@ virPCIDeviceIterDevices(virPCIDeviceIterPredicate predicate, *matched = check;
I think that instead of adding a new condition block, we could do VIR_STEAL_PTR ^here and then...
ret = 1; break; + } else { + virPCIDeviceFree(check); }
do virPCIDeviceFree(check) unconditionally. IMHO it would also look more clean.
Or, even better, @check can be declared as VIR_AUTOPTR(virPCIDevice), VIR_STREAL_PTR can be used to set @matched and then all virPCIDeviceFree() calls can be removed. Does this work for you? diff --git c/src/util/virpci.c w/src/util/virpci.c index ef578bf774..017fe72f19 100644 --- c/src/util/virpci.c +++ w/src/util/virpci.c @@ -449,7 +449,7 @@ virPCIDeviceIterDevices(virPCIDeviceIterPredicate predicate, while ((ret = virDirRead(dir, &entry, PCI_SYSFS "devices")) > 0) { unsigned int domain, bus, slot, function; - virPCIDevicePtr check; + VIR_AUTOPTR(virPCIDevice) check = NULL; char *tmp; /* expected format: <domain>:<bus>:<slot>.<function> */ @@ -474,12 +474,11 @@ virPCIDeviceIterDevices(virPCIDeviceIterPredicate predicate, rc = predicate(dev, check, data); if (rc < 0) { /* the predicate returned an error, bail */ - virPCIDeviceFree(check); ret = -1; break; } else if (rc == 1) { VIR_DEBUG("%s %s: iter matched on %s", dev->id, dev->name, check->name); - *matched = check; + VIR_STEAL_PTR(*matched, check); ret = 1; break; } Michal
On Thu, Jan 24, 2019 at 10:11:15AM +0100, Michal Privoznik wrote:
On 1/23/19 5:24 PM, Erik Skultety wrote:
On Wed, Jan 23, 2019 at 04:45:07PM +0100, Michal Privoznik wrote:
This partially reverts 00dc991ca167302c7a72f4fb16be061d05b12a32.
2,030 (1,456 direct, 574 indirect) bytes in 14 blocks are definitely lost in loss record 77 of 80 at 0x4C30E96: calloc (vg_replace_malloc.c:711) by 0x50F83AA: virAlloc (viralloc.c:143) by 0x5178DFA: virPCIDeviceNew (virpci.c:1753) by 0x51753E9: virPCIDeviceIterDevices (virpci.c:468) by 0x5175EB5: virPCIDeviceGetParent (virpci.c:759) by 0x517AB55: virPCIDeviceIsBehindSwitchLackingACS (virpci.c:2476) by 0x517AC24: virPCIDeviceIsAssignable (virpci.c:2494) by 0x10BF27: testVirPCIDeviceIsAssignable (virpcitest.c:229) by 0x10D14C: virTestRun (testutils.c:174) by 0x10C535: mymain (virpcitest.c:422) by 0x10F1B6: virTestMain (testutils.c:1112) by 0x10CF93: main (virpcitest.c:455)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virpci.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/src/util/virpci.c b/src/util/virpci.c index ef578bf774..fb22460bf5 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -482,6 +482,8 @@ virPCIDeviceIterDevices(virPCIDeviceIterPredicate predicate, *matched = check;
I think that instead of adding a new condition block, we could do VIR_STEAL_PTR ^here and then...
ret = 1; break; + } else { + virPCIDeviceFree(check); }
do virPCIDeviceFree(check) unconditionally. IMHO it would also look more clean.
Or, even better, @check can be declared as VIR_AUTOPTR(virPCIDevice), VIR_STREAL_PTR can be used to set @matched and then all virPCIDeviceFree() calls can be removed. Does this work for you?
Sure, go ahead, you already have my RB :). Erik
13 bytes in 1 blocks are definitely lost in loss record 44 of 179 at 0x4C2EE6F: malloc (vg_replace_malloc.c:299) by 0x9514A69: strdup (in /lib64/libc-2.27.so) by 0x5E60C0B: virStrdup (virstring.c:956) by 0x54C856F: virHostGetDRMRenderNode (qemuxml2argvmock.c:190) by 0x57CB4E3: qemuProcessGraphicsSetupRenderNode (qemu_process.c:4860) by 0x57CB571: qemuProcessSetupGraphics (qemu_process.c:4881) by 0x57CE01B: qemuProcessPrepareDomain (qemu_process.c:6040) by 0x57D102E: qemuProcessCreatePretendCmd (qemu_process.c:6975) by 0x114C1C: testCompareXMLToArgv (qemuxml2argvtest.c:611) by 0x134B90: virTestRun (testutils.c:174) by 0x123478: mymain (qemuxml2argvtest.c:1697) by 0x136BFA: virTestMain (testutils.c:1112) Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 54d6364f4f..f3d541bc12 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1423,6 +1423,9 @@ void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def) break; case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: + VIR_FREE(def->data.egl_headless.rendernode); + break; + case VIR_DOMAIN_GRAPHICS_TYPE_LAST: break; } -- 2.19.2
On Wed, Jan 23, 2019 at 04:45:08PM +0100, Michal Privoznik wrote:
13 bytes in 1 blocks are definitely lost in loss record 44 of 179 at 0x4C2EE6F: malloc (vg_replace_malloc.c:299) by 0x9514A69: strdup (in /lib64/libc-2.27.so) by 0x5E60C0B: virStrdup (virstring.c:956) by 0x54C856F: virHostGetDRMRenderNode (qemuxml2argvmock.c:190) by 0x57CB4E3: qemuProcessGraphicsSetupRenderNode (qemu_process.c:4860) by 0x57CB571: qemuProcessSetupGraphics (qemu_process.c:4881) by 0x57CE01B: qemuProcessPrepareDomain (qemu_process.c:6040) by 0x57D102E: qemuProcessCreatePretendCmd (qemu_process.c:6975) by 0x114C1C: testCompareXMLToArgv (qemuxml2argvtest.c:611) by 0x134B90: virTestRun (testutils.c:174) by 0x123478: mymain (qemuxml2argvtest.c:1697) by 0x136BFA: virTestMain (testutils.c:1112)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
participants (2)
-
Erik Skultety -
Michal Privoznik