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