[patch] python-dbus 0.80.1 memory leak issue

Simon McVittie simon.mcvittie at collabora.co.uk
Wed Feb 7 05:04:00 PST 2007


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Tue, 06 Feb 2007 at 20:28:24 +0100, Luka Renko wrote:
> Is 0.80.1 so much different than 0.71 that we used in Ubuntu before?

Yes, the native-code bits are a complete rewrite (that's why it no
longer needs Pyrex, and works on Python 2.5 without Fedora's patches
to Pyrex.)

Anyway, I've found and fixed the problem with String leaking memory - I'd
assumed that the __dict__ was automatically freed by Python, but this
turns out not to be the case. The patch below solves this for String,
Struct, _LongBase and _StrBase - with this change, and the patches I
previously posted to the list, I no longer get increasing memory consumption
when running your simplified test case. Could you please verify this for your
real app?

I also now get stable memory consumption when allocating and
deallocating the D-Bus data types in a loop.

J5, could you review these patches for 0.80.2? I've pushed them all to
git+ssh://people.freedesktop.org/home/smcv/public_html/git/dbus-python/.git
(http://people.freedesktop.org/~smcv/git/dbus-python/.git for people
without fd.o logins).

	Simon


- From 5135a35677e25c473db0e8a463f97c15359c9e34 Mon Sep 17 00:00:00 2001
From: Simon McVittie <simon.mcvittie at collabora.co.uk>
Date: Wed, 7 Feb 2007 12:50:48 +0000
Subject: [PATCH] Fix memory leak where Struct, _LongBase, _StrBase, String leaked their __dict__ on deallocation.
* Use a fixed-size struct for String (unicode objects are in fact fixed-size)
  and store its variant_level that way.
* Don't store Struct, _LongBase, _StrBase variant_level and Struct signature
  in a __dict__, but instead have a global dict mapping object IDs to variant
  levels, and a global dict mapping Struct IDs to signatures. This is a bit
  strange, but easier than correctly freeing the __dict__ (which is stored
  at the end of a variable-length struct, so somewhat hard to get at).
* With this change, allocating objects in a loop no longer leaks memory, and
  neither does the test case supplied by Luka Renko.
- ---
 _dbus_bindings/abstract.c       |  194 +++++++++++++++++++++++++++++++-------
 _dbus_bindings/containers.c     |  132 +++++++++++++++++++++-----
 _dbus_bindings/message-append.c |    6 +-
 _dbus_bindings/string.c         |   75 ++++++---------
 _dbus_bindings/types-internal.h |   10 ++
 5 files changed, 309 insertions(+), 108 deletions(-)

diff --git a/_dbus_bindings/abstract.c b/_dbus_bindings/abstract.c
index 57a4e04..f33996b 100644
- --- a/_dbus_bindings/abstract.c
+++ b/_dbus_bindings/abstract.c
@@ -28,6 +28,123 @@
 #include "dbus_bindings-internal.h"
 #include "types-internal.h"
 
+/* Dict indexed by object IDs, whose values are nonzero variant levels
+ * for immutable variable-sized D-Bus data types (_LongBase, _StrBase, Struct).
+ *
+ * This is a strange way to store them, but adding a __dict__ to the offending
+ * objects seems even more error-prone, given that their sizes are variable!
+ */
+PyObject *_dbus_py_variant_levels = NULL;
+
+long
+dbus_py_variant_level_get(PyObject *obj)
+{
+    PyObject *vl_obj;
+    PyObject *key = PyLong_FromVoidPtr(obj);
+
+    if (!key) {
+        return 0;
+    }
+
+    vl_obj = PyDict_GetItem(_dbus_py_variant_levels, key);
+    Py_DECREF(key);
+
+    if (!vl_obj)
+        return 0;
+    return PyInt_AsLong(vl_obj);
+}
+
+dbus_bool_t
+dbus_py_variant_level_set(PyObject *obj, long variant_level)
+{
+    /* key is the object's ID (= pointer) to avoid referencing it */
+    PyObject *key = PyLong_FromVoidPtr(obj);
+
+    if (!key) {
+        return FALSE;
+    }
+
+    if (variant_level <= 0) {
+        if (PyDict_GetItem (_dbus_py_variant_levels, key)) {
+            if (PyDict_DelItem (_dbus_py_variant_levels, key) < 0) {
+                Py_DECREF(key);
+                return FALSE;
+            }
+        }
+    }
+    else {
+        PyObject *vl_obj = PyInt_FromLong(variant_level);
+        if (!vl_obj) {
+            Py_DECREF(key);
+            return FALSE;
+        }
+        if (PyDict_SetItem (_dbus_py_variant_levels, key, vl_obj) < 0) {
+            Py_DECREF(key);
+            return FALSE;
+        }
+    }
+    Py_DECREF(key);
+    return TRUE;
+}
+
+PyObject *
+dbus_py_variant_level_getattro(PyObject *obj, PyObject *name)
+{
+    PyObject *key, *value;
+
+    if (PyString_Check(name)) {
+        Py_INCREF(name);
+    }
+    else if (PyUnicode_Check(name)) {
+        name = PyUnicode_AsEncodedString(name, NULL, NULL);
+        if (!name) {
+            return NULL;
+        }
+    }
+    else {
+        PyErr_SetString(PyExc_TypeError, "attribute name must be string");
+        return NULL;
+    }
+
+    if (strcmp(PyString_AS_STRING(name), "variant_level")) {
+        value = PyObject_GenericGetAttr(obj, name);
+        Py_DECREF(name);
+        return value;
+    }
+
+    Py_DECREF(name);
+
+    key = PyLong_FromVoidPtr(obj);
+
+    if (!key) {
+        return NULL;
+    }
+
+    value = PyDict_GetItem(_dbus_py_variant_levels, key);
+    Py_DECREF(key);
+
+    if (!value)
+        return PyInt_FromLong(0);
+    Py_INCREF(value);
+    return value;
+}
+
+/* To be invoked by destructors. Clear the variant level without touching the
+ * exception state */
+void
+dbus_py_variant_level_clear(PyObject *self)
+{
+    PyObject *et, *ev, *etb;
+
+    /* avoid clobbering any pending exception */
+    PyErr_Fetch(&et, &ev, &etb);
+    if (!dbus_py_variant_level_set(self, 0)) {
+        /* should never happen */
+        PyErr_WriteUnraisable(self);
+    }
+    PyErr_Restore(et, ev, etb);
+}
+
 /* Support code for int subclasses. ================================== */
 
 PyDoc_STRVAR(DBusPythonInt_tp_doc,\
@@ -259,7 +376,7 @@ static PyObject *
 DBusPythonString_tp_new(PyTypeObject *cls, PyObject *args, PyObject *kwargs)
 {
     PyObject *self;
- -    PyObject *variantness = NULL;
+    long variantness = 0;
     static char *argnames[] = {"variant_level", NULL};
 
     if (PyTuple_Size(args) > 1) {
@@ -268,13 +385,9 @@ DBusPythonString_tp_new(PyTypeObject *cls, PyObject *args, PyObject *kwargs)
         return NULL;
     }
     if (!PyArg_ParseTupleAndKeywords(dbus_py_empty_tuple, kwargs,
- -                                     "|O!:__new__", argnames,
- -                                     &PyInt_Type, &variantness)) return NULL;
- -    if (!variantness) {
- -        variantness = PyInt_FromLong(0);
- -        if (!variantness) return NULL;
- -    }
- -    if (PyInt_AS_LONG(variantness) < 0) {
+                                     "|l:__new__", argnames,
+                                     &variantness)) return NULL;
+    if (variantness < 0) {
         PyErr_SetString(PyExc_ValueError,
                         "variant_level must be non-negative");
         return NULL;
@@ -282,7 +395,10 @@ DBusPythonString_tp_new(PyTypeObject *cls, PyObject *args, PyObject *kwargs)
 
     self = (PyString_Type.tp_new)(cls, args, NULL);
     if (self) {
- -        PyObject_GenericSetAttr(self, dbus_py_variant_level_const, variantness);
+        if (!dbus_py_variant_level_set(self, variantness)) {
+            Py_DECREF(self);
+            return NULL;
+        }
     }
     return self;
 }
@@ -318,13 +434,20 @@ DBusPythonString_tp_repr(PyObject *self)
     return my_repr;
 }
 
+static void
+DBusPyStrBase_tp_dealloc(PyObject *self)
+{
+    dbus_py_variant_level_clear(self);
+    (PyString_Type.tp_dealloc)(self);
+}
+
 PyTypeObject DBusPyStrBase_Type = {
     PyObject_HEAD_INIT(DEFERRED_ADDRESS(&PyType_Type))
     0,
     "_dbus_bindings._StrBase",
- -    INT_MAX, /* placeholder */
+    0,                           
     0,
- -    0,                                      /* tp_dealloc */
+    DBusPyStrBase_tp_dealloc,               /* tp_dealloc */
     0,                                      /* tp_print */
     0,                                      /* tp_getattr */
     0,                                      /* tp_setattr */
@@ -336,7 +459,7 @@ PyTypeObject DBusPyStrBase_Type = {
     0,                                      /* tp_hash */
     0,                                      /* tp_call */
     0,                                      /* tp_str */
- -    PyObject_GenericGetAttr,                /* tp_getattro */
+    dbus_py_variant_level_getattro,         /* tp_getattro */
     dbus_py_immutable_setattro,             /* tp_setattro */
     0,                                      /* tp_as_buffer */
     Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */
@@ -354,7 +477,7 @@ PyTypeObject DBusPyStrBase_Type = {
     0,                                      /* tp_dict */
     0,                                      /* tp_descr_get */
     0,                                      /* tp_descr_set */
- -    -sizeof(void *),                        /* tp_dictoffset */
+    0,                                      /* tp_dictoffset */
     0,                                      /* tp_init */
     0,                                      /* tp_alloc */
     DBusPythonString_tp_new,                /* tp_new */
@@ -371,7 +494,7 @@ static PyObject *
 DBusPythonLong_tp_new(PyTypeObject *cls, PyObject *args, PyObject *kwargs)
 {
     PyObject *self;
- -    PyObject *variantness = NULL;
+    long variantness = 0;
     static char *argnames[] = {"variant_level", NULL};
 
     if (PyTuple_Size(args) > 1) {
@@ -380,13 +503,9 @@ DBusPythonLong_tp_new(PyTypeObject *cls, PyObject *args, PyObject *kwargs)
         return NULL;
     }
     if (!PyArg_ParseTupleAndKeywords(dbus_py_empty_tuple, kwargs,
- -                                     "|O!:__new__", argnames,
- -                                     &PyInt_Type, &variantness)) return NULL;
- -    if (!variantness) {
- -        variantness = PyInt_FromLong(0);
- -        if (!variantness) return NULL;
- -    }
- -    if (PyInt_AS_LONG(variantness) < 0) {
+                                     "|l:__new__", argnames,
+                                     &variantness)) return NULL;
+    if (variantness < 0) {
         PyErr_SetString(PyExc_ValueError,
                         "variant_level must be non-negative");
         return NULL;
@@ -394,7 +513,10 @@ DBusPythonLong_tp_new(PyTypeObject *cls, PyObject *args, PyObject *kwargs)
 
     self = (PyLong_Type.tp_new)(cls, args, NULL);
     if (self) {
- -        PyObject_GenericSetAttr(self, dbus_py_variant_level_const, variantness);
+        if (!dbus_py_variant_level_set(self, variantness)) {
+            Py_DECREF(self);
+            return NULL;
+        }
     }
     return self;
 }
@@ -430,13 +552,20 @@ DBusPythonLong_tp_repr(PyObject *self)
     return my_repr;
 }
 
+static void
+DBusPyLongBase_tp_dealloc(PyObject *self)
+{
+    dbus_py_variant_level_clear(self);
+    (PyLong_Type.tp_dealloc)(self);
+}
+
 PyTypeObject DBusPyLongBase_Type = {
     PyObject_HEAD_INIT(DEFERRED_ADDRESS(&PyType_Type))
     0,
     "_dbus_bindings._LongBase",
- -    INT_MAX, /* placeholder */
+    0,                           
     0,
- -    0,                                      /* tp_dealloc */
+    DBusPyLongBase_tp_dealloc,              /* tp_dealloc */
     0,                                      /* tp_print */
     0,                                      /* tp_getattr */
     0,                                      /* tp_setattr */
@@ -448,7 +577,7 @@ PyTypeObject DBusPyLongBase_Type = {
     0,                                      /* tp_hash */
     0,                                      /* tp_call */
     0,                                      /* tp_str */
- -    PyObject_GenericGetAttr,                /* tp_getattro */
+    dbus_py_variant_level_getattro,         /* tp_getattro */
     dbus_py_immutable_setattro,             /* tp_setattro */
     0,                                      /* tp_as_buffer */
     Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */
@@ -466,7 +595,7 @@ PyTypeObject DBusPyLongBase_Type = {
     0,                                      /* tp_dict */
     0,                                      /* tp_descr_get */
     0,                                      /* tp_descr_set */
- -    -sizeof(PyObject *),                    /* tp_dictoffset */
+    0,                                      /* tp_dictoffset */
     0,                                      /* tp_init */
     0,                                      /* tp_alloc */
     DBusPythonLong_tp_new,                  /* tp_new */
@@ -479,6 +608,9 @@ PyObject *dbus_py__dbus_object_path__const = NULL;
 dbus_bool_t
 dbus_py_init_abstract(void)
 {
+    _dbus_py_variant_levels = PyDict_New();
+    if (!_dbus_py_variant_levels) return 0;
+
     dbus_py__dbus_object_path__const = PyString_InternFromString("__dbus_object_path__");
     if (!dbus_py__dbus_object_path__const) return 0;
 
@@ -498,20 +630,10 @@ dbus_py_init_abstract(void)
     if (PyType_Ready(&DBusPyFloatBase_Type) < 0) return 0;
     DBusPyFloatBase_Type.tp_print = NULL;
 
- -    /* Add a pointer for the instance dict, aligning to sizeof(PyObject *)
- -     * to make sure the offset of -sizeof(PyObject *) is right. */
- -    DBusPyLongBase_Type.tp_basicsize = PyLong_Type.tp_basicsize
- -                                       + 2*sizeof(PyObject *) - 1;
- -    DBusPyLongBase_Type.tp_basicsize /= sizeof(PyObject *);
- -    DBusPyLongBase_Type.tp_basicsize *= sizeof(PyObject *);
     DBusPyLongBase_Type.tp_base = &PyLong_Type;
     if (PyType_Ready(&DBusPyLongBase_Type) < 0) return 0;
     DBusPyLongBase_Type.tp_print = NULL;
 
- -    DBusPyStrBase_Type.tp_basicsize = PyString_Type.tp_basicsize
- -                                      + 2*sizeof(PyObject *) - 1;
- -    DBusPyStrBase_Type.tp_basicsize /= sizeof(PyObject *);
- -    DBusPyStrBase_Type.tp_basicsize *= sizeof(PyObject *);
     DBusPyStrBase_Type.tp_base = &PyString_Type;
     if (PyType_Ready(&DBusPyStrBase_Type) < 0) return 0;
     DBusPyStrBase_Type.tp_print = NULL;
diff --git a/_dbus_bindings/containers.c b/_dbus_bindings/containers.c
index 4dc9046..ee38032 100644
- --- a/_dbus_bindings/containers.c
+++ b/_dbus_bindings/containers.c
@@ -474,6 +474,8 @@ PyTypeObject DBusPyDict_Type = {
 
 /* Struct =========================================================== */
 
+static PyObject *struct_signatures;
+
 PyDoc_STRVAR(Struct_tp_doc,
 "An structure containing items of possibly distinct types.\n"
 "\n"
@@ -505,20 +507,21 @@ static PyObject *
 Struct_tp_repr(PyObject *self)
 {
     PyObject *parent_repr = (PyTuple_Type.tp_repr)((PyObject *)self);
- -    PyObject *sig = NULL;
+    PyObject *sig;
     PyObject *sig_repr = NULL;
- -    PyObject *vl_obj = NULL;
+    PyObject *key;
     long variant_level;
     PyObject *my_repr = NULL;
 
     if (!parent_repr) goto finally;
- -    sig = PyObject_GetAttr(self, dbus_py_signature_const);
- -    if (!sig) goto finally;
+    key = PyLong_FromVoidPtr(self);
+    if (!key) goto finally;
+    sig = PyDict_GetItem(struct_signatures, key);
+    Py_DECREF(key);
+    if (!sig) sig = Py_None;
     sig_repr = PyObject_Repr(sig);
     if (!sig_repr) goto finally;
- -    vl_obj = PyObject_GetAttr(self, dbus_py_variant_level_const);
- -    if (!vl_obj) goto finally;
- -    variant_level = PyInt_AsLong(vl_obj);
+    variant_level = dbus_py_variant_level_get(self);
     if (variant_level > 0) {
         my_repr = PyString_FromFormat("%s(%s, signature=%s, "
                                       "variant_level=%ld)",
@@ -536,9 +539,7 @@ Struct_tp_repr(PyObject *self)
 
 finally:
     Py_XDECREF(parent_repr);
- -    Py_XDECREF(sig);
     Py_XDECREF(sig_repr);
- -    Py_XDECREF(vl_obj);
     return my_repr;
 }
 
@@ -546,8 +547,8 @@ static PyObject *
 Struct_tp_new (PyTypeObject *cls, PyObject *args, PyObject *kwargs)
 {
     PyObject *signature = NULL;
- -    PyObject *variantness = NULL;
- -    PyObject *self;
+    long variantness = 0;
+    PyObject *self, *key;
     static char *argnames[] = {"signature", "variant_level", NULL};
 
     if (PyTuple_Size(args) != 1) {
@@ -556,13 +557,14 @@ Struct_tp_new (PyTypeObject *cls, PyObject *args, PyObject *kwargs)
         return NULL;
     }
     if (!PyArg_ParseTupleAndKeywords(dbus_py_empty_tuple, kwargs,
- -                                     "|OO!:__new__", argnames,
- -                                     &signature, &PyInt_Type,
- -                                     &variantness)) {
+                                     "|Ol:__new__", argnames,
+                                     &signature, &variantness)) {
         return NULL;
     }
- -    if (!variantness) {
- -        variantness = PyInt_FromLong(0);
+    if (variantness < 0) {
+        PyErr_SetString(PyExc_ValueError,
+                        "variant_level must be non-negative");
+        return NULL;
     }
 
     self = (PyTuple_Type.tp_new)(cls, args, NULL);
@@ -574,7 +576,7 @@ Struct_tp_new (PyTypeObject *cls, PyObject *args, PyObject *kwargs)
         return NULL;
     }
 
- -    if (PyObject_GenericSetAttr(self, dbus_py_variant_level_const, variantness) < 0) {
+    if (!dbus_py_variant_level_set(self, variantness)) {
         Py_DECREF(self);
         return NULL;
     }
@@ -595,22 +597,101 @@ Struct_tp_new (PyTypeObject *cls, PyObject *args, PyObject *kwargs)
         }
     }
 
- -    if (PyObject_GenericSetAttr(self, dbus_py_signature_const, signature) < 0) {
+    key = PyLong_FromVoidPtr(self);
+    if (!key) {
+        Py_DECREF(self);
+        Py_DECREF(signature);
+        return NULL;
+    }
+    if (PyDict_SetItem(struct_signatures, key, signature) < 0) {
+        Py_DECREF(key);
         Py_DECREF(self);
         Py_DECREF(signature);
         return NULL;
     }
+
+    Py_DECREF(key);
     Py_DECREF(signature);
     return self;
 }
 
+static void
+Struct_tp_dealloc(PyObject *self)
+{
+    PyObject *et, *ev, *etb, *key;
+
+    dbus_py_variant_level_clear(self);
+    PyErr_Fetch(&et, &ev, &etb);
+
+    key = PyLong_FromVoidPtr(self);
+    if (key) {
+        if (PyDict_GetItem(struct_signatures, key)) {
+            if (PyDict_DelItem(struct_signatures, key) < 0) {
+                /* should never happen */
+                PyErr_WriteUnraisable(self);
+            }
+        }
+        Py_DECREF(key);
+    }
+    else {
+        /* not enough memory to free all the memory... leak the signature,
+         * there's not much else we could do here */
+        PyErr_WriteUnraisable(self);
+    }
+
+    PyErr_Restore(et, ev, etb);
+    (PyTuple_Type.tp_dealloc)(self);
+}
+
+PyObject *
+Struct_tp_getattro(PyObject *obj, PyObject *name)
+{
+    PyObject *key, *value;
+
+    if (PyString_Check(name)) {
+        Py_INCREF(name);
+    }
+    else if (PyUnicode_Check(name)) {
+        name = PyUnicode_AsEncodedString(name, NULL, NULL);
+        if (!name) {
+            return NULL;
+        }
+    }
+    else {
+        PyErr_SetString(PyExc_TypeError, "attribute name must be string");
+        return NULL;
+    }
+
+    if (strcmp(PyString_AS_STRING(name), "signature")) {
+        value = dbus_py_variant_level_getattro(obj, name);
+        Py_DECREF(name);
+        return value;
+    }
+
+    Py_DECREF(name);
+
+    key = PyLong_FromVoidPtr(obj);
+
+    if (!key) {
+        return NULL;
+    }
+
+    value = PyDict_GetItem(struct_signatures, key);
+    Py_DECREF(key);
+
+    if (!value)
+        value = Py_None;
+    Py_INCREF(value);
+    return value;
+}
+
 PyTypeObject DBusPyStruct_Type = {
     PyObject_HEAD_INIT(DEFERRED_ADDRESS(&PyType_Type))
     0,
     "dbus.Struct",
- -    INT_MAX, /* placeholder */
     0,
- -    0,                                      /* tp_dealloc */
+    0,
+    Struct_tp_dealloc,                      /* tp_dealloc */
     0,                                      /* tp_print */
     0,                                      /* tp_getattr */
     0,                                      /* tp_setattr */
@@ -622,7 +703,7 @@ PyTypeObject DBusPyStruct_Type = {
     0,                                      /* tp_hash */
     0,                                      /* tp_call */
     0,                                      /* tp_str */
- -    PyObject_GenericGetAttr,                /* tp_getattro */
+    Struct_tp_getattro,                     /* tp_getattro */
     dbus_py_immutable_setattro,             /* tp_setattro */
     0,                                      /* tp_as_buffer */
     Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */
@@ -640,7 +721,7 @@ PyTypeObject DBusPyStruct_Type = {
     0,                                      /* tp_dict */
     0,                                      /* tp_descr_get */
     0,                                      /* tp_descr_set */
- -    -sizeof(PyObject *),                    /* tp_dictoffset */
+    0,                                      /* tp_dictoffset */
     0,                                      /* tp_init */
     0,                                      /* tp_alloc */
     Struct_tp_new,                          /* tp_new */
@@ -649,6 +730,9 @@ PyTypeObject DBusPyStruct_Type = {
 dbus_bool_t
 dbus_py_init_container_types(void)
 {
+    struct_signatures = PyDict_New();
+    if (!struct_signatures) return 0;
+
     DBusPyArray_Type.tp_base = &PyList_Type;
     if (PyType_Ready(&DBusPyArray_Type) < 0) return 0;
     DBusPyArray_Type.tp_print = NULL;
@@ -657,10 +741,6 @@ dbus_py_init_container_types(void)
     if (PyType_Ready(&DBusPyDict_Type) < 0) return 0;
     DBusPyDict_Type.tp_print = NULL;
 
- -    DBusPyStruct_Type.tp_basicsize = PyTuple_Type.tp_basicsize
- -                                     + 2*sizeof(PyObject *) - 1;
- -    DBusPyStruct_Type.tp_basicsize /= sizeof(PyObject *);
- -    DBusPyStruct_Type.tp_basicsize *= sizeof(PyObject *);
     DBusPyStruct_Type.tp_base = &PyTuple_Type;
     if (PyType_Ready(&DBusPyStruct_Type) < 0) return 0;
     DBusPyStruct_Type.tp_print = NULL;
diff --git a/_dbus_bindings/message-append.c b/_dbus_bindings/message-append.c
index d4a56b0..b2a4afd 100644
- --- a/_dbus_bindings/message-append.c
+++ b/_dbus_bindings/message-append.c
@@ -43,11 +43,13 @@ get_variant_level(PyObject *obj)
     else if (DBusPyDict_Check(obj)) {
         return ((DBusPyDict *)obj)->variant_level;
     }
+    else if (DBusPyString_Check(obj)) {
+        return ((DBusPyString *)obj)->variant_level;
+    }
     else if (DBusPyLongBase_Check(obj) ||
              DBusPyStrBase_Check(obj) ||
- -             DBusPyString_Check(obj) ||
              DBusPyStruct_Check(obj)) {
- -        return PyInt_AsLong(PyObject_GetAttr(obj, dbus_py_variant_level_const));
+        return dbus_py_variant_level_get(obj);
     }
     else {
         return 0;
diff --git a/_dbus_bindings/string.c b/_dbus_bindings/string.c
index 096635c..56dfae4 100644
- --- a/_dbus_bindings/string.c
+++ b/_dbus_bindings/string.c
@@ -21,6 +21,7 @@
  */
 
 #include "types-internal.h"
+#include <structmember.h>
 
 /* UTF-8 string representation ====================================== */
 
@@ -226,11 +227,19 @@ PyDoc_STRVAR(String_tp_doc,
 "    String or UTF8String with variant_level==2.\n"
 );
 
+static PyMemberDef String_tp_members[] = {
+    {"variant_level", T_LONG, offsetof(DBusPyString, variant_level),
+     READONLY,
+     "The number of nested variants wrapping the real data. "
+     "0 if not in a variant"},
+    {NULL},
+};
+
 static PyObject *
 String_tp_new(PyTypeObject *cls, PyObject *args, PyObject *kwargs)
 {
     PyObject *self;
- -    PyObject *variantness = NULL;
+    long variantness = 0;
     static char *argnames[] = {"variant_level", NULL};
 
     if (PyTuple_Size(args) > 1) {
@@ -239,32 +248,17 @@ String_tp_new(PyTypeObject *cls, PyObject *args, PyObject *kwargs)
         return NULL;
     }
     if (!PyArg_ParseTupleAndKeywords(dbus_py_empty_tuple, kwargs,
- -                                     "|O!:__new__", argnames,
- -                                     &PyInt_Type, &variantness)) return NULL;
- -    if (variantness) {
- -        if (PyInt_AS_LONG(variantness) < 0) {
- -            PyErr_SetString(PyExc_ValueError,
- -                            "variant_level must be non-negative");
- -            return NULL;
- -        }
- -        /* own a reference */
- -        Py_INCREF(variantness);
- -    }
- -    else {
- -        variantness = PyInt_FromLong(0);
- -        if (!variantness) return NULL;
+                                     "|l:__new__", argnames,
+                                     &variantness)) return NULL;
+    if (variantness < 0) {
+        PyErr_SetString(PyExc_ValueError,
+                        "variant_level must be non-negative");
+        return NULL;
     }
- -
     self = (PyUnicode_Type.tp_new)(cls, args, NULL);
     if (self) {
- -        if (PyObject_GenericSetAttr(self, dbus_py_variant_level_const,
- -                                    variantness) < 0) {
- -            Py_DECREF(variantness);
- -            Py_DECREF(self);
- -            return NULL;
- -        }
+        ((DBusPyString *)self)->variant_level = variantness;
     }
- -    Py_DECREF(variantness);
     return self;
 }
 
@@ -272,23 +266,16 @@ static PyObject *
 String_tp_repr(PyObject *self)
 {
     PyObject *parent_repr = (PyUnicode_Type.tp_repr)(self);
- -    PyObject *vl_obj;
     PyObject *my_repr;
- -    long variant_level;
 
- -    if (!parent_repr) return NULL;
- -    vl_obj = PyObject_GetAttr(self, dbus_py_variant_level_const);
- -    if (!vl_obj) {
- -        Py_DECREF(parent_repr);
+    if (!parent_repr) {
         return NULL;
     }
- -    variant_level = PyInt_AsLong(vl_obj);
- -    Py_DECREF(vl_obj);
- -    if (variant_level > 0) {
+    if (((DBusPyString *)self)->variant_level > 0) {
         my_repr = PyString_FromFormat("%s(%s, variant_level=%ld)",
                                       self->ob_type->tp_name,
                                       PyString_AS_STRING(parent_repr),
- -                                      variant_level);
+                                      ((DBusPyString *)self)->variant_level);
     }
     else {
         my_repr = PyString_FromFormat("%s(%s)", self->ob_type->tp_name,
@@ -303,7 +290,7 @@ PyTypeObject DBusPyString_Type = {
     PyObject_HEAD_INIT(DEFERRED_ADDRESS(&PyType_Type))
     0,
     "dbus.String",
- -    INT_MAX, /* placeholder */
+    sizeof(DBusPyString),
     0,
     0,                                      /* tp_dealloc */
     0,                                      /* tp_print */
@@ -329,13 +316,13 @@ PyTypeObject DBusPyString_Type = {
     0,                                      /* tp_iter */
     0,                                      /* tp_iternext */
     0,                                      /* tp_methods */
- -    0,                                      /* tp_members */
+    String_tp_members,                      /* tp_members */
     0,                                      /* tp_getset */
     DEFERRED_ADDRESS(&PyUnicode_Type),      /* tp_base */
     0,                                      /* tp_dict */
     0,                                      /* tp_descr_get */
     0,                                      /* tp_descr_set */
- -    -sizeof(void *),                        /* tp_dictoffset */
+    0,                                      /* tp_dictoffset */
     0,                                      /* tp_init */
     0,                                      /* tp_alloc */
     String_tp_new,                          /* tp_new */
@@ -344,18 +331,18 @@ PyTypeObject DBusPyString_Type = {
 dbus_bool_t
 dbus_py_init_string_types(void)
 {
- -    DBusPyString_Type.tp_basicsize = PyUnicode_Type.tp_basicsize
- -                                     + 2*sizeof(PyObject *) - 1;
- -    DBusPyString_Type.tp_basicsize /= sizeof(PyObject *);
- -    DBusPyString_Type.tp_basicsize *= sizeof(PyObject *);
+    /* don't need to do strange contortions for unicode, since it's not a
+     * "variable-size" object (it has a pointer to its data instead)
+     */
+    if (PyUnicode_Type.tp_itemsize != 0) {
+        fprintf(stderr, "dbus-python is not compatible with this version of "
+                "Python (unicode objects are assumed to be fixed-size)");
+        return 0;
+    }
     DBusPyString_Type.tp_base = &PyUnicode_Type;
     if (PyType_Ready(&DBusPyString_Type) < 0) return 0;
     DBusPyString_Type.tp_print = NULL;
 
- -    DBusPyUTF8String_Type.tp_basicsize = PyUnicode_Type.tp_basicsize
- -                                  + 2*sizeof(PyObject *) - 1;
- -    DBusPyUTF8String_Type.tp_basicsize /= sizeof(PyObject *);
- -    DBusPyUTF8String_Type.tp_basicsize *= sizeof(PyObject *);
     DBusPyUTF8String_Type.tp_base = &DBusPyStrBase_Type;
     if (PyType_Ready(&DBusPyUTF8String_Type) < 0) return 0;
     DBusPyUTF8String_Type.tp_print = NULL;
diff --git a/_dbus_bindings/types-internal.h b/_dbus_bindings/types-internal.h
index f208421..3d7d866 100644
- --- a/_dbus_bindings/types-internal.h
+++ b/_dbus_bindings/types-internal.h
@@ -47,6 +47,11 @@ typedef struct {
     long variant_level;
 } DBusPyFloatBase;
 
+typedef struct {
+    PyUnicodeObject unicode;
+    long variant_level;
+} DBusPyString;
+
 extern PyTypeObject DBusPyStrBase_Type;
 DEFINE_CHECK(DBusPyStrBase)
 
@@ -79,4 +84,9 @@ typedef struct {
     long variant_level;
 } DBusPyDict;
 
+PyObject *dbus_py_variant_level_getattro(PyObject *obj, PyObject *name);
+dbus_bool_t dbus_py_variant_level_set(PyObject *obj, long variant_level);
+void dbus_py_variant_level_clear(PyObject *obj);
+long dbus_py_variant_level_get(PyObject *obj);
+
 #endif
- -- 
1.4.4.4

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: OpenPGP key: http://www.pseudorandom.co.uk/2003/contact/ or pgp.net

iD8DBQFFyc5AWSc8zVUw7HYRAqtbAJ9nWCCcHIt2tj3e+V2/cRXe3A2HQgCfU/EF
5eN71t2z3hLJLNi303TiXkg=
=QXFV
-----END PGP SIGNATURE-----


More information about the dbus mailing list