question: numarray c extension error handling?

[Russell E. Owen]

I'm writing a C extension for numarray and am puzzled about the idiom
for error handling.

The documentation seems to say one should always decref an array after
calling NA_InputArray, etc., to convert numarray args to C arrays.

However, the example and also the numarray code suggests that it's OK to
return early via (for example) PyErr_Format without doing the DECREF.
For example, I have appended a very abbreviated version of the sample
code.

Comments? I'll plow ahead and follow the example (i.e. with early
return) but I was wondering if anyone knew why this was OK. Perhaps I
should put in a request to the numarray project for a clarification in
the documentation.

-- Russell

static PyObject * Py_Convolve1d(PyObject *obj, PyObject *args)
{
PyObject *okernel,...;
PyArrayObject *kernel...;

if (!PyArg_ParseTuple(args, "OO|O", &okernel, ...))
return PyErr_Format(_convolveError,
"Convolve1d: Invalid parameters.");

/* Align, Byteswap, Contiguous, Typeconvert */
kernel = NA_IoArray(okernel, tFloat64, C_ARRAY);
....
if (!kernel...)
return PyErr_Format( _convolveError,
"Convolve1d: error converting array inputs.");
....
Py_XDECREF(kernel);
....
}

The relevant section of the docs (section 12.3 High-level API):

The return value of each function (NA_InputArray, NA_OutputArray, or
NA_IoArray) is either a reference to the original numarray object, or a
reference to a temporary numarray. Following execution of the C-code in
the extension function body this pointer should *always* be DECREFed.

[David M. Cooke]

At some point, "Russell E. Owen" <[e-mail]> wrote:

I'm writing a C extension for numarray and am puzzled about the idiom
for error handling.

The documentation seems to say one should always decref an array after
calling NA_InputArray, etc., to convert numarray args to C arrays.

However, the example and also the numarray code suggests that it's OK to
return early via (for example) PyErr_Format without doing the DECREF.
For example, I have appended a very abbreviated version of the sample
code.

Comments? I'll plow ahead and follow the example (i.e. with early
return) but I was wondering if anyone knew why this was OK. Perhaps I
should put in a request to the numarray project for a clarification in
the documentation.

-- Russell

static PyObject * Py_Convolve1d(PyObject *obj, PyObject *args)
{
PyObject *okernel,...;
PyArrayObject *kernel...;

if (!PyArg_ParseTuple(args, "OO|O", &okernel, ...))
return PyErr_Format(_convolveError,
"Convolve1d: Invalid parameters.");
This is ok; if PyArg_ParseTuple is NULL, no objects are created (or
INCREF'd). Also, if it's not NULL, okernel is a borrowed reference;
you shouldn't DECREF it.

/* Align, Byteswap, Contiguous, Typeconvert */
kernel = NA_IoArray(okernel, tFloat64, C_ARRAY);
...
if (!kernel...)
return PyErr_Format( _convolveError,
"Convolve1d: error converting array inputs.");
This is also ok (assuming there is nothing between this and the above
which makes a reference to objects). If NA_IoArray returns NULL,
there isn't anything to DECREF.
...
Py_XDECREF(kernel);
...
}

Personally, I write it like this:

static PyObject * Py_Convolve1d(PyObject *obj, PyObject *args)
{
PyObject *okernel,...;
PyArrayObject *kernel = NULL;

if (!PyArg_ParseTuple(args, "OO|O", &okernel, ...)) {
PyErr_Format(_convolveError,
"Convolve1d: Invalid parameters.");
goto error;
}
kernel = NA_IoArray(okernel, tFloat64, C_ARRAY);
if (!kernel...) {
PyErr_Format( _convolveError,
"Convolve1d: error converting array inputs.");
goto error;
}

...more stuff...

Py_XDECREF(kernel)
return the result
error:
Py_XDECREF(kernel)
return NULL;
}

Notice that kernel was initialized to NULL, so Py_XDECREF will work in
all cases. Clean error-handling code like this is one of the few
places I use goto in C. You now have only two exits out of the
function, at the bottom, and so it's _much_ easier to keep track of
what needs to be DECREF'd.

--
|>|\/|<
/--------------------------------------------------------------------------\
|David M. Cooke
|cookedm(at)physics(dot)mcmaster(dot)ca

[Russell E. Owen]

In article <qn*************@arbutus.physics.mcmaster.ca>,
co**********@physics.mcmaster.ca (David M. Cooke) wrote:

...

/* Align, Byteswap, Contiguous, Typeconvert */
kernel = NA_IoArray(okernel, tFloat64, C_ARRAY);
...
if (!kernel...)
return PyErr_Format( _convolveError,
"Convolve1d: error converting array inputs.");

This is also ok (assuming there is nothing between this and the above
which makes a reference to objects). If NA_IoArray returns NULL,
there isn't anything to DECREF.

I'm afraid I was too selective in extracting code, making it look like
no DECREF was even wanted. Here is a more complete extract. The first
test can fail even if one or two of the arrays was successfullly
"extracted", so a DECREF is wanted then, and all the more so for the two
tests after that:

....
/* Align, Byteswap, Contiguous, Typeconvert */
kernel = NA_IoArray(okernel, tFloat64, C_ARRAY);
data = NA_IoArray(odata, tFloat64, C_ARRAY);
convolved = NA_OptionalOutputArray(oconvolved, tFloat64, C_ARRAY,
data);

if (!kernel || !data || !convolved)
return PyErr_Format( _convolveError,
"Convolve1d: error converting array inputs.");

if ((kernel->nd != 1) || (data->nd != 1))
return PyErr_Format(_convolveError,
"Convolve1d: numarray must have 1 dimensions.");

if (!NA_ShapeEqual(data, convolved))
return PyErr_Format(_convolveError,
"Convolve1d: data and output numarray need identitcal
shapes.");

Convolve1d(kernel->dimensions[0],
NA_OFFSETDATA(kernel),
data->dimensions[0],
NA_OFFSETDATA(data),
NA_OFFSETDATA(convolved));

Py_XDECREF(kernel);
Py_XDECREF(data);
....

It sounds as if I should submit a bug report on the example.

-- Russell

[Lou Pecora]

Here's a nice use of goto's in a Python C extension for those who have
been following the goto thread. I like it (see code at end and the nice
resolve for returns the goto's give). I hope Mr. Cooke does not mind me
"borrowing" his code. :-)
In article <qn*************@arbutus.physics.mcmaster.ca>,
co**********@physics.mcmaster.ca (David M. Cooke) wrote:

At some point, "Russell E. Owen" <[e-mail]> wrote:

I'm writing a C extension for numarray and am puzzled about the idiom
for error handling.

The documentation seems to say one should always decref an array after
calling NA_InputArray, etc., to convert numarray args to C arrays.

However, the example and also the numarray code suggests that it's OK to
return early via (for example) PyErr_Format without doing the DECREF.
For example, I have appended a very abbreviated version of the sample
code.

Comments? I'll plow ahead and follow the example (i.e. with early
return) but I was wondering if anyone knew why this was OK. Perhaps I
should put in a request to the numarray project for a clarification in
the documentation.

-- Russell

static PyObject * Py_Convolve1d(PyObject *obj, PyObject *args)
{
PyObject *okernel,...;
PyArrayObject *kernel...;

if (!PyArg_ParseTuple(args, "OO|O", &okernel, ...))
return PyErr_Format(_convolveError,
"Convolve1d: Invalid parameters.");

This is ok; if PyArg_ParseTuple is NULL, no objects are created (or
INCREF'd). Also, if it's not NULL, okernel is a borrowed reference;
you shouldn't DECREF it.

/* Align, Byteswap, Contiguous, Typeconvert */
kernel = NA_IoArray(okernel, tFloat64, C_ARRAY);
...
if (!kernel...)
return PyErr_Format( _convolveError,
"Convolve1d: error converting array inputs.");

This is also ok (assuming there is nothing between this and the above
which makes a reference to objects). If NA_IoArray returns NULL,
there isn't anything to DECREF.

...
Py_XDECREF(kernel);
...
}

Personally, I write it like this:

static PyObject * Py_Convolve1d(PyObject *obj, PyObject *args)
{
PyObject *okernel,...;
PyArrayObject *kernel = NULL;

if (!PyArg_ParseTuple(args, "OO|O", &okernel, ...)) {
PyErr_Format(_convolveError,
"Convolve1d: Invalid parameters.");
goto error;
}
kernel = NA_IoArray(okernel, tFloat64, C_ARRAY);
if (!kernel...) {
PyErr_Format( _convolveError,
"Convolve1d: error converting array inputs.");
goto error;
}

...more stuff...

Py_XDECREF(kernel)
return the result
error:
Py_XDECREF(kernel)
return NULL;
}

Notice that kernel was initialized to NULL, so Py_XDECREF will work in
all cases. Clean error-handling code like this is one of the few
places I use goto in C. You now have only two exits out of the
function, at the bottom, and so it's _much_ easier to keep track of
what needs to be DECREF'd.

-- Lou Pecora
My views are my own.

[David M. Cooke]

At some point, Lou Pecora <pe****@anvil.nrl.navy.mil> wrote:

Here's a nice use of goto's in a Python C extension for those who have
been following the goto thread. I like it (see code at end and the nice
resolve for returns the goto's give). I hope Mr. Cooke does not mind me
"borrowing" his code. :-)

Not a problem; I wish more people would :-)

--
|>|\/|<
/--------------------------------------------------------------------------\
|David M. Cooke
|cookedm(at)physics(dot)mcmaster(dot)ca

[Josiah Carlson]

It is arbitrarily trivial to convert it to use an error boolean.

- Josiah

static PyObject * Py_Convolve1d(PyObject *obj, PyObject *args)
{
PyObject *okernel,...;
PyArrayObject *kernel = NULL;
bool error = 0;

if (!PyArg_ParseTuple(args, "OO|O", &okernel, ...)) {
PyErr_Format(_convolveError,
"Convolve1d: Invalid parameters.");
error = 1;
}
kernel = NA_IoArray(okernel, tFloat64, C_ARRAY);
if (!kernel...) {
PyErr_Format( _convolveError,
"Convolve1d: error converting array inputs.");
error = 1;
}

...more stuff...

if (!error) {
Py_XDECREF(kernel)
return the result
} else {
Py_XDECREF(kernel)
return NULL;
}
}

[Joe Mason]

In article <c2**********@news.service.uci.edu>, Josiah Carlson wrote:

It is arbitrarily trivial to convert it to use an error boolean.
No, it isn't, as evidenced by the fact that you did it wrong.
static PyObject * Py_Convolve1d(PyObject *obj, PyObject *args)
{
PyObject *okernel,...;
PyArrayObject *kernel = NULL;
bool error = 0;

if (!PyArg_ParseTuple(args, "OO|O", &okernel, ...)) {
PyErr_Format(_convolveError,
"Convolve1d: Invalid parameters.");
error = 1;
}
if (!error) { kernel = NA_IoArray(okernel, tFloat64, C_ARRAY);
if (!kernel...) {
PyErr_Format( _convolveError,
"Convolve1d: error converting array inputs.");
error = 1;
} }

if (!error) { ...more stuff... }
if (!error) {
Py_XDECREF(kernel)
return the result
} else {
Py_XDECREF(kernel)
return NULL;
}
}

You have to remember to check the error variable at every step in the
function, or use else clauses and deal with heavy nesting. This is a
perfect situation for GOTO.

Joe

[Josiah Carlson]

> You have to remember to check the error variable at every step in the

function, or use else clauses and deal with heavy nesting. This is a
perfect situation for GOTO.

I did slip, but it is still fairly easy to do, I just wasn't paying
attention. It does show a situation where GOTO can be useful, I'll give
you that. Depending on the code, it may be a case where a macro would
be better suited.

- Josiah

[Isaac To]

>>>>> "Josiah" == Josiah Carlson <jc******@nospam.uci.edu> writes:

Josiah> I did slip, but it is still fairly easy to do

This "fairly easy" ignores the possibility that later you need to add or
modify the line that must be executed before the function returns. Macro
might be a solution, although I consider the goto solution cleaner (doesn't
need a separate language (and thus different indentation rules), won't
pollute the namespace, ease the compiler to reduce compiled code size, etc).

Regards,
Isaac.