python extension: incref question

[John Hunter]

I am writing a python extension and have a question about reference
counting on an attribute I set. I am using standard boilerplate code
for defining a new type.

I implement the setattr methods, and want to set some numeric values
from within the extension code. I create the new GlyphObject and set
the attributes with

GlyphObject *gm;
gm = PyObject_New(GlyphObject, &Glyph_Type);
//handle error
gm->x_attr = NULL;
Glyph_setattr(gm, "width", PyInt_FromLong(self->face->glyph->metrics.width));
Glyph_setattr(gm, "height", PyInt_FromLong(self->face->glyph->metrics.height));

where

static int
Glyph_setattr(GlyphObject *self, char *name, PyObject *v)
{
if (self->x_attr == NULL) {
self->x_attr = PyDict_New();
if (self->x_attr == NULL)
return -1;
}

if (v == NULL) {
int rv = PyDict_DelItemString(self->x_attr, name);
if (rv < 0)
PyErr_SetString(PyExc_AttributeError,
"delete non-existing Glyph attribute");
return rv;
}
else {
return PyDict_SetItemString(self->x_attr, name, v);
}
}

http://www.python.org/doc/current/ext/ownershipRules.html states

PyDict_SetItem() and friends don't take over ownership -- they are
``normal.''

I infer from this that since a new ref is created by PyInt_FromLong,
and PyDict_SetItemString doesn't take ownership, that I should
decrease the reference count in my types dealloc function. Is this
correct?

Thanks,
John Hunter

 

[David Rushby]

I am writing a python extension and have a question about reference
counting on an attribute I set.
...
I infer from this that since a new ref is created by PyInt_FromLong,
and PyDict_SetItemString doesn't take ownership, that I should
decrease the reference count in my types dealloc function. Is this
correct?

Mostly. You're right about the reference ownership (PyDict_SetItem
creates its own reference (see Python 2.3.3
Objects/dictobject.c:531)). However, I would recommend disposing of
the extra reference not in your type's dealloc function, but as soon
as you've sent the value to Glyph_setattr. Something along the lines
of:
---
PyObject *val = PyInt_FromLong(self->face->glyph->metrics.width);
if (val == NULL) {
return PyErr_NoMemory();
}
int gsetResult = Glyph_setattr(gm, "width", val);
Py_DECREF(val);
if (gsetResult == -1) {
...
}
---
That way, you will have disposed of the extra reference to val as soon
as it's no longer needed. In your type's dealloc function, you can
simply Py_XDECREF(self->x_attr), and x_attr will dispose of its own
reference to val.

If you were to wait to dispose of the extra reference until your
type's dealloc function were called, but in the meantime
Glyph_setattr(gm, "width", ...) were called again, the extra reference
to val would be lost in the shuffle.

 

[John Hunter]

David> Mostly. You're right about the reference ownership
David> (PyDict_SetItem creates its own reference (see Python 2.3.3
David> Objects/dictobject.c:531)). However, I would recommend
David> disposing of the extra reference not in your type's dealloc
David> function, but as soon as you've sent the value to


Thanks for the suggestions. Since I have to set a lot of these, I
wrote a macro.

#define SETATTR(o,setattr_func,name,PyBuilder,val) \
{ \
PyObject *pval =PyBuilder(val);\
if (pval == NULL) {PyErr_NoMemory(); return NULL;}\
int gsetResult = setattr_func(o, name, pval);\
Py_DECREF(pval);\
if (gsetResult == -1) {\
PyErr_SetString(PyExc_RuntimeError, "Could not set attr");\
return NULL;\
}\
}

(for some reason my compiler issued warnings on the return
PyErr_NoMemory() you suggested (incompatible return type) which is
surprising since PyErr_NoMemory returns NULL. Maybe a macro issue?

which I use like

SETATTR(self, FT2Font_setattr, "postscript_name", PyString_FromString, ps_name);
SETATTR(self, FT2Font_setattr, "num_faces", PyInt_FromLong, self->face->num_faces);

Do you see any problems with this approach?

Also, I was surprised to find that these attrs don't automagically
appear in a dir(myobj). I've been planning to consult the docs or
google since I'm sure it's a relatively obvious thing I'm missing, but
I thought I'd mention it while I had your ear .

JDH