4:45 p.m.
*** 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
4:45 p.m.
New subject: [libvirt] [PATCH 1/3] virPCIGetNetName: Initialize @netname to NULL
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(a)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
5:33 p.m.
New subject: [libvirt] [PATCH 1/3] virPCIGetNetName: Initialize @netname to NULL
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(a)redhat.com>
---
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
4:45 p.m.
New subject: [libvirt] [PATCH 2/3] virpci: Fix memleak in virPCIDeviceIterDevices
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(a)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
5:24 p.m.
New subject: [libvirt] [PATCH 2/3] virpci: Fix memleak in virPCIDeviceIterDevices
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(a)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(a)redhat.com>
10:11 a.m.
New subject: [libvirt] [PATCH 2/3] virpci: Fix memleak in virPCIDeviceIterDevices
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(a)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
10:14 a.m.
New subject: [libvirt] [PATCH 2/3] virpci: Fix memleak in virPCIDeviceIterDevices
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(a)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
4:45 p.m.
New subject: [libvirt] [PATCH 3/3] domain_conf: Free egl render node in virDomainGraphicsDefFree
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(a)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
5:04 p.m.
New subject: [libvirt] [PATCH 3/3] domain_conf: Free egl render node in virDomainGraphicsDefFree
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(a)redhat.com>
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
2064
days inactive
2065
days old
8 comments
2 participants
participants (2)
-
Erik Skultety -
Michal Privoznik