dbus-python: Potential Memory Leak
Lawrence D'Oliveiro
ldo at geek-central.gen.nz
Sun Sep 11 09:12:16 UTC 2022
Looking over the source code for dbus-python
<https://gitlab.freedesktop.org/dbus/dbus-python/-/tree/master>, in
dbus_bindings/message.c, I noticed this:
for (ptr = paths; *ptr; ptr++) {
PyObject *str = PyUnicode_FromString(*ptr);
if (!str) {
Py_CLEAR(ret);
break;
}
if (PyList_Append(ret, str) < 0) {
Py_CLEAR(ret);
break;
}
Py_CLEAR(str);
str = NULL;
}
If there is an error on the PyList_Append call, then str is not freed.
I have a preferred way of writing this sort of thing. My reworking of
the whole Message_get_path_decomposed routine would look more like this
(not tested, but you get the idea):
static PyObject *
Message_get_path_decomposed(Message *self, PyObject *unused UNUSED)
{
PyObject *ret = NULL;
PyObject *ret_tmp = NULL; /* build result list here */
char **paths = NULL;
do /*once*/
{
if (!self->msg) {
DBusPy_RaiseUnusableMessage();
break;
}
if (!dbus_message_get_path_decomposed(self->msg, &paths)) {
PyErr_NoMemory();
break;
}
if (!paths) {
Py_INCREF(Py_None);
ret = Py_None;
break;
}
ret_tmp = PyList_New(0);
if (PyErr_Occurred())
break;
for (char **ptr = paths;;) {
if (*ptr == NULL)
break;
PyObject *str = NULL;
do /*once*/
{
str = PyUnicode_FromString(*ptr);
if (PyErr_Occurred())
break;
if (PyList_Append(ret_tmp, str) < 0) {
break;
}
while (false);
Py_XDECREF(str);
if (PyErr_Occurred())
break;
++ptr;
} /*for*/
/* all done */
ret = ret_tmp;
ret_tmp = NULL; /* so I don’t dispose of it yet */
}
while (false);
dbus_free_string_array(paths);
Py_XDECREF(ret_tmp);
return ret;
}
It may seem more long-winded, but it can deal with complicated cases
better.
I expect I can find more cases in the code where this sort of technique
would be helpful.
More information about the dbus
mailing list