[Rpm-maint] [PATCH] fix memory leaks in invocations of PyObject_Call
David Malcolm
dmalcolm at redhat.com
Fri Dec 16 03:22:56 UTC 2011
Various functions in the Python bindings have expressions of the form:
PyObject_Call(callable,
Py_BuildValue(fmtstring, ...), NULL);
This leaks memory for the case when Py_BuildValue succeeds (it returns
a new reference, which is never freed; PyObject_Call doesn't steal the
reference): the argument tuple and all of its components will not be
freed (until the process exits).
There's also a possible segfault for the case where Py_BuildValue fails:
PyObject_Call will likely segfault with a NULL 2nd argument.
This affects:
python/header-py.c: hdr_fiFromHeader rpm.hdr.fiFromHeader()
python/header-py.c: hdr_dsFromHeader rpm.hdr.dsFromHeader()
python/header-py.c: hdr_dsOfHeader rpm.hdr.dsOfHeader()
python/rpmfd-py.c: rpmfdFromPyObject
It's better to use PyObject_CallObject:
http://docs.python.org/c-api/object.html#PyObject_CallFunction
which can construct the argument tuple in a similar fashion to Py_BuildValue,
but handles cleanup and failures for you.
Also, if all arguments are already PyObject*, PyObject_CallFunctionObjArgs
is faster:
http://docs.python.org/c-api/object.html#PyObject_CallFunctionObjArgs
Caveat: patch below compiles, but is untested.
Found using an experimental static analysis tool I've been writing:
http://gcc-python-plugin.readthedocs.org/en/latest/cpychecker.html
which is part of https://fedorahosted.org/gcc-python-plugin
A sample HTML report from the tool on one of these cases (specifically
hdr_fiFromHeader) can be seen here:
http://fedorapeople.org/~dmalcolm/gcc-python-plugin/2011-12-15/header-py.c.hdr_fiFromHeader-refcount-errors.html
---
python/header-py.c | 11 +++++------
python/rpmfd-py.c | 4 ++--
2 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/python/header-py.c b/python/header-py.c
index 96cf200..cef457b 100644
--- a/python/header-py.c
+++ b/python/header-py.c
@@ -295,8 +295,7 @@ static PyObject * hdrWrite(hdrObject *s, PyObject *args, PyObject *kwds)
*/
static PyObject * hdr_fiFromHeader(PyObject * s, PyObject * args, PyObject * kwds)
{
- return PyObject_Call((PyObject *) &rpmfi_Type,
- Py_BuildValue("(O)", s), NULL);
+ return PyObject_CallFunctionObjArgs((PyObject *) &rpmfi_Type, s, NULL);
}
/* Backwards compatibility. Flags argument is just a dummy and discarded. */
@@ -309,14 +308,14 @@ static PyObject * hdr_dsFromHeader(PyObject * s, PyObject * args, PyObject * kwd
tagNumFromPyObject, &tag, &flags))
return NULL;
- return PyObject_Call((PyObject *) &rpmds_Type,
- Py_BuildValue("(Oi)", s, tag), NULL);
+ return PyObject_CallFunction((PyObject *) &rpmds_Type,
+ "(Oi)", s, tag);
}
static PyObject * hdr_dsOfHeader(PyObject * s)
{
- return PyObject_Call((PyObject *) &rpmds_Type,
- Py_BuildValue("(Oi)", s, RPMTAG_NEVR), NULL);
+ return PyObject_CallFunction((PyObject *) &rpmds_Type,
+ "(Oi)", s, RPMTAG_NEVR);
}
static long hdr_hash(PyObject * h)
diff --git a/python/rpmfd-py.c b/python/rpmfd-py.c
index 89a70cd..1150aa1 100644
--- a/python/rpmfd-py.c
+++ b/python/rpmfd-py.c
@@ -23,8 +23,8 @@ int rpmfdFromPyObject(PyObject *obj, rpmfdObject **fdop)
Py_INCREF(obj);
fdo = (rpmfdObject *) obj;
} else {
- fdo = (rpmfdObject *) PyObject_Call((PyObject *)&rpmfd_Type,
- Py_BuildValue("(O)", obj), NULL);
+ fdo = (rpmfdObject *) PyObject_CallFunctionObjArgs((PyObject *)&rpmfd_Type,
+ obj, NULL);
}
if (fdo == NULL) return 0;
--
1.7.6.2
More information about the Rpm-maint
mailing list