Re: [libvirt] [PATCHv4] python: Avoid memory leaks on libvirt_virNodeGetMemoryStats

2012-03-21 Thread Alex Jia 
Eric, thanks for your review and comment,
I will change patch with your advice then
directly push.

Alex


- Original Message -
From: Eric Blake ebl...@redhat.com
To: Alex Jia a...@redhat.com
Cc: libvir-list@redhat.com
Sent: Tuesday, March 20, 2012 11:46:51 PM
Subject: Re: [libvirt] [PATCHv4] python: Avoid memory leaks on 
libvirt_virNodeGetMemoryStats

On 03/20/2012 01:14 AM, Alex Jia wrote:
 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 a...@redhat.com
 ---
  python/libvirt-override.c |   41 +++--
  1 files changed, 31 insertions(+), 10 deletions(-)

  for (i = 0; i  nparams; i++) {
 -PyDict_SetItem(ret,
 -   libvirt_constcharPtrWrap(stats[i].field),
 -   libvirt_ulonglongWrap(stats[i].value));
 +key = libvirt_constcharPtrWrap(stats[i].field);
 +val = libvirt_ulonglongWrap(stats[i].value);
 +
 +if (!key || !val) {
 +ret = NULL;
 +goto error;

Memory leak of ret.

 +}
 +
 +if (PyDict_SetItem(ret, key, val)  0) {
 +Py_DECREF(ret);
 +ret = NULL;
 +goto error;
 +}

Fix it by doing:

if (!key || !val || PyDict_SetItem(ret, key, val)  0) {
Py_DECREF(ret);
ret = NULL;
goto error;
}

ACK with that fix.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org

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

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

2012-03-20 Thread Alex Jia 
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 a...@redhat.com
---
 python/libvirt-override.c |   41 +++--
 1 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/python/libvirt-override.c b/python/libvirt-override.c
index 792cfa3..634e8bb 100644
--- a/python/libvirt-override.c
+++ b/python/libvirt-override.c
@@ -2550,7 +2550,9 @@ libvirt_virNodeGetCPUStats(PyObject *self 
ATTRIBUTE_UNUSED, PyObject *args)
 static PyObject *
 libvirt_virNodeGetMemoryStats(PyObject *self ATTRIBUTE_UNUSED, PyObject *args)
 {
-PyObject *ret;
+PyObject *ret = NULL;
+PyObject *key = NULL;
+PyObject *val = NULL;
 PyObject *pyobj_conn;
 virConnectPtr conn;
 unsigned int flags;
@@ -2559,7 +2561,7 @@ libvirt_virNodeGetMemoryStats(PyObject *self 
ATTRIBUTE_UNUSED, PyObject *args)
 virNodeMemoryStatsPtr stats = NULL;
 
 if (!PyArg_ParseTuple(args, (char *)Oii:virNodeGetMemoryStats, 
pyobj_conn, cellNum, flags))
-return(NULL);
+return ret;
 conn = (virConnectPtr)(PyvirConnect_Get(pyobj_conn));
 
 LIBVIRT_BEGIN_ALLOW_THREADS;
@@ -2570,7 +2572,7 @@ libvirt_virNodeGetMemoryStats(PyObject *self 
ATTRIBUTE_UNUSED, PyObject *args)
 
 if (nparams) {
 if (VIR_ALLOC_N(stats, nparams)  0)
-return VIR_PY_NONE;
+return PyErr_NoMemory();
 
 LIBVIRT_BEGIN_ALLOW_THREADS;
 c_retval = virNodeGetMemoryStats(conn, cellNum, stats, nparams, 
flags);
@@ -2580,18 +2582,37 @@ libvirt_virNodeGetMemoryStats(PyObject *self 
ATTRIBUTE_UNUSED, PyObject *args)
 return VIR_PY_NONE;
 }
 }
-if (!(ret = PyDict_New())) {
-VIR_FREE(stats);
-return VIR_PY_NONE;
-}
+
+if (!(ret = PyDict_New()))
+goto error;
+
 for (i = 0; i  nparams; i++) {
-PyDict_SetItem(ret,
-   libvirt_constcharPtrWrap(stats[i].field),
-   libvirt_ulonglongWrap(stats[i].value));
+key = libvirt_constcharPtrWrap(stats[i].field);
+val = libvirt_ulonglongWrap(stats[i].value);
+
+if (!key || !val) {
+ret = NULL;
+goto error;
+}
+
+if (PyDict_SetItem(ret, key, val)  0) {
+Py_DECREF(ret);
+ret = NULL;
+goto error;
+}
+
+Py_DECREF(key);
+Py_DECREF(val);
 }
 
 VIR_FREE(stats);
 return ret;
+
+error:
+VIR_FREE(stats);
+Py_XDECREF(key);
+Py_XDECREF(val);
+return ret;
 }
 
 static PyObject *
-- 
1.7.1

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

Re: [libvirt] [PATCHv4] python: Avoid memory leaks on libvirt_virNodeGetMemoryStats

2012-03-20 Thread Eric Blake 
On 03/20/2012 01:14 AM, Alex Jia wrote:
 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 a...@redhat.com
 ---
  python/libvirt-override.c |   41 +++--
  1 files changed, 31 insertions(+), 10 deletions(-)

  for (i = 0; i  nparams; i++) {
 -PyDict_SetItem(ret,
 -   libvirt_constcharPtrWrap(stats[i].field),
 -   libvirt_ulonglongWrap(stats[i].value));
 +key = libvirt_constcharPtrWrap(stats[i].field);
 +val = libvirt_ulonglongWrap(stats[i].value);
 +
 +if (!key || !val) {
 +ret = NULL;
 +goto error;

Memory leak of ret.

 +}
 +
 +if (PyDict_SetItem(ret, key, val)  0) {
 +Py_DECREF(ret);
 +ret = NULL;
 +goto error;
 +}

Fix it by doing:

if (!key || !val || PyDict_SetItem(ret, key, val)  0) {
Py_DECREF(ret);
ret = NULL;
goto error;
}

ACK with that fix.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list