[libvirt] [PATCHv2] python: Avoid memory leaks on libvirt_virNodeGetMemoryStats
Peter Krempa
pkrempa at redhat.com
Tue Feb 14 13:45:27 UTC 2012
On 02/14/2012 12:19 PM, 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>
> ---
--- see end of message for patch of suggested changes ---
> python/libvirt-override.c | 43 +++++++++++++++++++++++++++++--------------
> 1 files changed, 29 insertions(+), 14 deletions(-)
>
> diff --git a/python/libvirt-override.c b/python/libvirt-override.c
> index 4e8a97e..594aae6 100644
> --- a/python/libvirt-override.c
> +++ b/python/libvirt-override.c
> @@ -2426,7 +2426,9 @@ libvirt_virNodeGetCPUStats(PyObject *self ATTRIBUTE_UNUSED, PyObject *args)
> static PyObject *
> libvirt_virNodeGetMemoryStats(PyObject *self ATTRIBUTE_UNUSED, PyObject *args)
> {
> - PyObject *ret;
I realized, that "VIR_PY_NONE" actualy increases the reference count on
the "none" object. We should not do it unless we need the reference. Set
ret to NULL please.
> + PyObject *ret = VIR_PY_NONE;
> + PyObject *key = NULL;
> + PyObject *val = NULL;
> PyObject *pyobj_conn;
> virConnectPtr conn;
> unsigned int flags;
> @@ -2442,32 +2444,45 @@ libvirt_virNodeGetMemoryStats(PyObject *self ATTRIBUTE_UNUSED, PyObject *args)
> c_retval = virNodeGetMemoryStats(conn, cellNum, NULL,&nparams, flags);
> LIBVIRT_END_ALLOW_THREADS;
> if (c_retval< 0)
> - return VIR_PY_NONE;
> + return ret;
Use "goto error;" here for consistency.
>
> 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);
> LIBVIRT_END_ALLOW_THREADS;
> - if (c_retval< 0) {
> - VIR_FREE(stats);
> - return VIR_PY_NONE;
> - }
> - }
> - if (!(ret = PyDict_New())) {
> - VIR_FREE(stats);
> - return VIR_PY_NONE;
> + if (c_retval< 0)
> + goto error;
> }
> + 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)
> + goto error;
> +
> + if (PyDict_SetItem(ret, key, val)< 0) {
> + Py_DECREF(ret);
Move this statement to the error section to free the returned dictionary
even in case of other errors.
> + 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;
You'll need to return VIR_PY_NONE here as the
> }
>
> static PyObject *
ACK with those changes.
Here is a patch of changes that I suggest:
diff --git a/python/libvirt-override.c b/python/libvirt-override.c
index 594aae6..0409dd4 100644
--- a/python/libvirt-override.c
+++ b/python/libvirt-override.c
@@ -2426,7 +2426,7 @@ libvirt_virNodeGetCPUStats(PyObject *self
ATTRIBUTE_UNUSED, PyObject *args)
static PyObject *
libvirt_virNodeGetMemoryStats(PyObject *self ATTRIBUTE_UNUSED,
PyObject *args)
{
- PyObject *ret = VIR_PY_NONE;
+ PyObject *ret = NULL;
PyObject *key = NULL;
PyObject *val = NULL;
PyObject *pyobj_conn;
@@ -2444,7 +2444,7 @@ libvirt_virNodeGetMemoryStats(PyObject *self
ATTRIBUTE_UNUSED, PyObject *args)
c_retval = virNodeGetMemoryStats(conn, cellNum, NULL, &nparams,
flags);
LIBVIRT_END_ALLOW_THREADS;
if (c_retval < 0)
- return ret;
+ goto error;
if (nparams) {
if (VIR_ALLOC_N(stats, nparams) < 0)
@@ -2466,10 +2466,8 @@ libvirt_virNodeGetMemoryStats(PyObject *self
ATTRIBUTE_UNUSED, PyObject *args)
if (!key || !val)
goto error;
- if (PyDict_SetItem(ret, key, val) < 0) {
- Py_DECREF(ret);
+ if (PyDict_SetItem(ret, key, val) < 0)
goto error;
- }
Py_DECREF(key);
Py_DECREF(val);
@@ -2482,7 +2480,8 @@ error:
VIR_FREE(stats);
Py_XDECREF(key);
Py_XDECREF(val);
- return ret;
+ Py_XDECREF(ret);
+ return VIR_PY_NONE;
}
static PyObject *
More information about the libvir-list
mailing list