Re: [PATCH] python: Avoid memory leaks on libvirt_virNodeGetMemoryStats

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 02/14/2012 11:48 AM, Alex Jia wrote:
On 02/14/2012 06:08 PM, Peter Krempa wrote:
On 02/14/2012 09:31 AM, ajia@xxxxxxxxxx wrote:
From: Alex Jia<ajia@xxxxxxxxxx>

Detected by valgrind. Leaks are introduced in commit 17c7795.

* python/libvirt-override.c (libvirt_virNodeGetMemoryStats): fix
memory leaks
and improve codes return value.

For details, please see the following link:
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=770944

Signed-off-by: Alex Jia<ajia@xxxxxxxxxx>
---
python/libvirt-override.c | 41 ++++++++++++++++++++++++++++++-----------
1 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/python/libvirt-override.c b/python/libvirt-override.c
index 4e8a97e..ecb11ea 100644
--- a/python/libvirt-override.c
+++ b/python/libvirt-override.c

Relevant function header

static PyObject *
libvirt_virNodeGetMemoryStats(PyObject *self ATTRIBUTE_UNUSED,
PyObject *args)

With code like this, the variable err isn't used anywhere.

Introduction of the new variable "info" is probably not needed, and
you may use "ret" for this purpose.
If so, the 'ret' will have 2 different function, the one is return
value, the other is dictionary object,
I'm not sure it's a good idea to override previous variable 'ret',
although the codes are quite simple.

The return value of this function in case everything went OK actualy is the dictionary Python object we construct in the code. Your "info" variable actualy copies the function of ret.

Well, looking back at the code now, the simplification of cleanup section probably won't be that easy, so you probably will have to use it as an error section.

Peter


Thanks for your comment,
Alex

It should be possible to rewrite the code (especialy the cleanup
section) so that also the success path may pass that way. That would
simplify the code.

Peter

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

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


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]