[Numpy-discussion] Questions on error handling/refcounting in some ufunc object loops.

[Charles R Harris]

Hi All (but mostly Travis),

I've been looking at the object loops now that they are cleaned up, and it
looks to me like the error handling is inconsistent and there are some
reference counting errors. Warning, code ahead!

/**begin repeat
 * #kind = equal, not_equal, greater, greater_equal, less, less_equal#
 * #OP = EQ, NE, GT, GE, LT, LE#
 */
static void
OBJECT_ at kind@(char **args, intp *dimensions, intp *steps, void
*NPY_UNUSED(func)) {
    BINARY_LOOP {
        PyObject *in1 = *(PyObject **)ip1;
        PyObject *in2 = *(PyObject **)ip2;
        *((Bool *)op1) = (Bool) PyObject_RichCompareBool(in1, in2, Py_ at OP@);
                                ^^^^^^^^^^^^^^^^^^^^^^^^
    }
}
/**end repeat**/

PyObject_RichCompareBool returns -1 on error. This might be OK, but we
probably don't want -1 in a boolean. Also, this behavior is inconsistent
with some other loops.

static void
OBJECT_sign(char **args, intp *dimensions, intp *steps, void
*NPY_UNUSED(func))
{
    PyObject *zero = PyInt_FromLong(0);
    UNARY_LOOP {
        PyObject *in1 = *(PyObject **)ip1;
        *((PyObject **)op1) = PyInt_FromLong(PyObject_Compare(in1, zero));
                              ^^^^^^^^^^^^^^
    }
    Py_DECREF(zero);
}

PyInt_FromLong returns NULL on error. Also, it looks to me like the object
in the array that is replaced needs its reference count decreased.

/*UFUNC_API*/
static void
PyUFunc_O_O_method(char **args, intp *dimensions, intp *steps, void *func)
{
    char *meth = (char *)func;
    UNARY_LOOP {
        PyObject *in1 = *(PyObject **)ip1;
        PyObject **out = (PyObject **)op1;
        PyObject *ret = PyObject_CallMethod(in1, meth, NULL);

        if (ret == NULL) {
            return;
        }
        Py_XDECREF(*out);
        *out = ret;
    }
}

Here the return value is checked and there is an early return on error.

/*UFUNC_API*/
static void
PyUFunc_O_O(char **args, intp *dimensions, intp *steps, void *func)
{
    unaryfunc f = (unaryfunc)func;
    UNARY_LOOP {
        PyObject *in1 = *(PyObject **)ip1;
        PyObject **out = (PyObject **)op1;
        PyObject *ret;

        if (in1 == NULL) {
            return;
        }
        ret = f(in1);
        if ((ret == NULL) || PyErr_Occurred()) {
            return;
        }
        Py_XDECREF(*out);
        *out = ret;
    }
}

Here the input value is also checked. The return value is checked, and
PyErr_Occurred is also checked.

I'm pretty sure about the reference leak. But what should be the standard
for checking arguments and error returns in these object loops?

Chuck
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/numpy-discussion/attachments/20081113/8693792e/attachment.html>