returning a list of tuples -- C API

[John Hunter]

I am writing a python extension module and have a reference counting
question

My function looks like

static PyObject *
pokereval_seven_cards(PyObject *self, PyObject *args)
{

int i;

PyObject * tup;
PyObject * list;

// ... snip ...

list = PyList_New(HandType_LAST+1);
for (i = HandType_FIRST; i <= HandType_LAST; i++) {
tup = Py_BuildValue("(s,h)", handTypeNamesPadded, totals);
PyList_SetItem(list, i, tup);
totals = 0;
}
Py_INCREF(list);
return list;

}

Should I be incrementing the ref of tup each time I call
Py_BuildValue? Is it correct to increment the ref of list before I
return it? Does it make a difference vis-a-vis ref counting if I
create the tuple with Py_BuildValue or PyTuple_New?

Thanks,
John Hunter

 

[David Rushby]

"""
My function looks like

static PyObject *
pokereval_seven_cards(PyObject *self, PyObject *args)
{

int i;

PyObject * tup;
PyObject * list;

// ... snip ...

list = PyList_New(HandType_LAST+1);
for (i = HandType_FIRST; i <= HandType_LAST; i++) {
tup = Py_BuildValue("(s,h)", handTypeNamesPadded, totals);
PyList_SetItem(list, i, tup);
totals = 0;
}
Py_INCREF(list);
return list;

}
"""

Should I be incrementing the ref of tup each time I call
Py_BuildValue?

No. Py_BuildValue is declared by the docs to return a new reference. In
your case, the 'tup = Py_BuildValue(...)' call creates a new object and
gives you a new reference to it. PyList_SetItem is declared by the docs (in
7.3.5 List Objects and 1.2.1.1 Reference Count Details) to "steal" a
reference to its third argument, so your 'PyList_SetItem(list, i, tup);'
call transfers ownership of the tuple reference (which you acquired in the
previous statement) to the list object. So you're first gaining ownership
of a reference to the tuple, then losing that ownership to the list, which
will decrement the reference count of the tuple when the list itself is
released.

Is it correct to increment the ref of list before I return it?

No. You created list within this C function, so you already own the only
reference to it; that ownership must be either discarded or transferred
before you exit the function. In your case, you should transfer to the
caller ownership of the only reference to list (i.e., get rid of the
'Py_INCREF(list); return list;' in favor of only 'return list;').

Does it make a difference vis-a-vis ref counting if I
create the tuple with Py_BuildValue or PyTuple_New?

No. The docs declare that both return a new reference.


P.S. For optimal robustness, you ought to check the return value of
PyList_New and Py_BuildValue to make sure there was adequate memory to
allocate the new object. E.g.,
list = PyList_New(HandType_LAST+1);
if (list == NULL) {
return PyErr_NoMemory();
}

In the case of the Py_BuildValue call, you'd need to discard the reference
to list before raising a MemoryError, e.g.:
tup = Py_BuildValue("(s,h)", handTypeNamesPadded, totals);
if (tup == NULL) {
Py_DECREF(list);
return PyErr_NoMemory();
}

In large C functions, it becomes error-prone to discard all necessary
references in each place an exception might be raised. The typical solution
(AFAIK) is to set to NULL at the beginning of the function all PyObject
pointers that might need to be released if an error arises, then have a
labelled error handler that discards those references, e.g. (warning:
untested code),

--------------
static PyObject *
pokereval_seven_cards(PyObject *self, PyObject *args)
{
int i;

PyObject *tup = NULL;
PyObject *list = NULL;

// ... snip ...

list = PyList_New(HandType_LAST+1);
if (list == NULL) {
PyErr_NoMemory();
goto err_handler;
}
for (i = HandType_FIRST; i <= HandType_LAST; i++) {
tup = Py_BuildValue("(s,h)", handTypeNamesPadded, totals);
if (tup == NULL) {
PyErr_NoMemory();
goto err_handler;
}
if (PyList_SetItem(list, i, tup) == -1) {
goto err_handler;
}
totals = 0;
}
return list;

err_handler:
assert(PyErr_Occurred()); /* An exception must already be set. */

if (tup != NULL && !PySequence_Contains(list, tup)) {
Py_DECREF(tup);
}
Py_XDECREF(list);

return NULL;
}