C API PyErr_Clear vs PyObject_HasAttrString

[Robin Becker]

I'm trying to understand why this code causes a seg fault second time
through; the code is part of a setattr and is testing if the value is an
integer or has red, green and blue attributes. This code later goes on
to raise a ValueError if that's not the case.

if((i = PyArg_Parse(value,"i",&cv))){
c->value = cv;
c->valid = 1;
return 1;
}

rgb = PyObject_HasAttrString(value,"red")
&& PyObject_HasAttrString(value,"green")
&& PyObject_HasAttrString(value,"blue");
PyErr_Clear();


........
PyErr_SetString(PyExc_ValueError, "bad color value");


Both calls should fail and raise ValueError, but the second call causes
a seg fault in Python 2.3.4 (not in 2.2 and earlier).

In 2.3.4 I found that moving the PyErr_Clear() before the calculation of
rgb fixes the problem.

I assume the problem is that something in the rgb calculation is failing
because the python error flag is set. Is that a bug in the
implementation of PyObject_HasAttrString or is it my fault for not
knowing which API calls need a cleared error flag?

 

[Andrew MacIntyre]

I'm trying to understand why this code causes a seg fault second time
through; the code is part of a setattr and is testing if the value is an
integer or has red, green and blue attributes. This code later goes on
to raise a ValueError if that's not the case.

if((i = PyArg_Parse(value,"i",&cv))){
c->value = cv;
c->valid = 1;
return 1;
}

rgb = PyObject_HasAttrString(value,"red")
&& PyObject_HasAttrString(value,"green")
&& PyObject_HasAttrString(value,"blue");
PyErr_Clear();


......
PyErr_SetString(PyExc_ValueError, "bad color value");


Both calls should fail and raise ValueError, but the second call causes
a seg fault in Python 2.3.4 (not in 2.2 and earlier).

In 2.3.4 I found that moving the PyErr_Clear() before the calculation of
rgb fixes the problem.

I assume the problem is that something in the rgb calculation is failing
because the python error flag is set. Is that a bug in the
implementation of PyObject_HasAttrString or is it my fault for not
knowing which API calls need a cleared error flag?

Rule: if you call a Python API function, _always_ check its return
status (unless documented as void). if it fails, check for an
exception. if you want to ignore the exception, clear the
exception, otherwise cleanup and bailout.

This is a paraphrase (from memory) of a previous reply from Tim Peters.

Notwithstanding this, segfaulting in a Python API function is not welcome
(especially one documented as always succeeding!).

I had a look at the source (from a CVS checkout this morning), and the
only thing that looks like a source of such trouble would be an extension
class with a tp_getattr implementation.

If you are using such a beast, you might want to look at the tp_getattr
implementation first. Otherwise I'd suggest filing a bug report on SF.
BTW, have you tried this with 2.4c1?

 

[Robin Becker]

Andrew MacIntyre wrote:
......

Rule: if you call a Python API function, _always_ check its return
status (unless documented as void). if it fails, check for an
exception. if you want to ignore the exception, clear the
exception, otherwise cleanup and bailout.

This is a paraphrase (from memory) of a previous reply from Tim Peters.

Notwithstanding this, segfaulting in a Python API function is not welcome
(especially one documented as always succeeding!).

I had a look at the source (from a CVS checkout this morning), and the
only thing that looks like a source of such trouble would be an extension
class with a tp_getattr implementation.

It is such a beast, but the exciting test case looks like

#####################
import _test
g=_test.gstate()

for a in ('strokeColor','strokeColor'):
try:
setattr(g,a,(1,2,3))
print 'Wrong handling of bad '+a
except ValueError:
pass
#####################

so the tp_getattr slot isn't being used.

If you are using such a beast, you might want to look at the tp_getattr
implementation first. Otherwise I'd suggest filing a bug report on SF.
BTW, have you tried this with 2.4c1?

Can't today, but I'll try and build the cut down extension with 2.4c1
tomorrow.

 

[Andrew MacIntyre]

It is such a beast, but the exciting test case looks like

#####################
import _test
g=_test.gstate()

for a in ('strokeColor','strokeColor'):
try:
setattr(g,a,(1,2,3))
print 'Wrong handling of bad '+a
except ValueError:
pass
#####################

so the tp_getattr slot isn't being used.

Referring to the original C code you posted, PyObject_HasAttrString() will
call the tp_getattr routine, clearing any exceptions raised as it returns.