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

Peter Krempa pkrempa at redhat.com
Tue Feb 14 10:52:19 UTC 2012


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 at redhat.com wrote:
>>> From: Alex Jia<ajia at redhat.com>
>>>
>>> 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 at redhat.com>
>>> ---
>>> 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 at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list




More information about the libvir-list mailing list