Mailing List Archive

Update of /cvsroot/python/python/dist/src/Objects
In directory usw-pr-cvs1:/tmp/cvs-serv19866

Modified Files:
abstract.c
Log Message:
abstract_get_bases(): Clarify exactly what the return values and
states can be for this function, and ensure that only AttributeErrors
are masked. Any other exception raised via the equivalent of
getattr(cls, '__bases__') should be propagated up.

abstract_issubclass(): If abstract_get_bases() returns NULL, we must
call PyErr_Occurred() to see if an exception is being propagated, and
return -1 or 0 as appropriate. This is the specific fix for a problem
whereby if getattr(derived, '__bases__') raised an exception, an
"undetected error" would occur (under a debug build). This nasty
situation was uncovered when writing a security proxy extension type
for the Zope3 project, where the security proxy raised a Forbidden
exception on getattr of __bases__.

PyObject_IsInstance(), PyObject_IsSubclass(): After both calls to
abstract_get_bases(), where we're setting the TypeError if the return
value is NULL, we must first check to see if an exception occurred,
and /not/ mask an existing exception.

Neil Schemenauer should double check that these changes don't break
his ExtensionClass examples (there aren't any test cases for those
examples and abstract_get_bases() was added by him in response to
problems with ExtensionClass). Neil, please add test cases if
possible!

I belive this is a bug fix candidate for Python 2.2.2.


Index: abstract.c
===================================================================
RCS file: /cvsroot/python/python/dist/src/Objects/abstract.c,v
retrieving revision 2.100
retrieving revision 2.101
diff -C2 -d -r2.100 -r2.101
*** abstract.c 16 Apr 2002 16:44:51 -0000 2.100
--- abstract.c 23 Apr 2002 22:45:44 -0000 2.101
***************
*** 1862,1865 ****
--- 1862,1891 ----
/* isinstance(), issubclass() */

+ /* abstract_get_bases() has logically 4 return states, with a sort of 0th
+ * state that will almost never happen.
+ *
+ * 0. creating the __bases__ static string could get a MemoryError
+ * 1. getattr(cls, '__bases__') could raise an AttributeError
+ * 2. getattr(cls, '__bases__') could raise some other exception
+ * 3. getattr(cls, '__bases__') could return a tuple
+ * 4. getattr(cls, '__bases__') could return something other than a tuple
+ *
+ * Only state #3 is a non-error state and only it returns a non-NULL object
+ * (it returns the retrieved tuple).
+ *
+ * Any raised AttributeErrors are masked by clearing the exception and
+ * returning NULL. If an object other than a tuple comes out of __bases__,
+ * then again, the return value is NULL. So yes, these two situations
+ * produce exactly the same results: NULL is returned and no error is set.
+ *
+ * If some exception other than AttributeError is raised, then NULL is also
+ * returned, but the exception is not cleared. That's because we want the
+ * exception to be propagated along.
+ *
+ * Callers are expected to test for PyErr_Occurred() when the return value
+ * is NULL to decide whether a valid exception should be propagated or not.
+ * When there's no exception to propagate, it's customary for the caller to
+ * set a TypeError.
+ */
static PyObject *
abstract_get_bases(PyObject *cls)
***************
*** 1873,1883 ****
return NULL;
}
-
bases = PyObject_GetAttr(cls, __bases__);
! if (bases == NULL || !PyTuple_Check(bases)) {
! Py_XDECREF(bases);
return NULL;
}
-
return bases;
}
--- 1899,1912 ----
return NULL;
}
bases = PyObject_GetAttr(cls, __bases__);
! if (bases == NULL) {
! if (PyErr_ExceptionMatches(PyExc_AttributeError))
! PyErr_Clear();
! return NULL;
! }
! if (!PyTuple_Check(bases)) {
! Py_DECREF(bases);
return NULL;
}
return bases;
}
***************
*** 1896,1902 ****

bases = abstract_get_bases(derived);
! if (bases == NULL)
return 0;
!
n = PyTuple_GET_SIZE(bases);
for (i = 0; i < n; i++) {
--- 1925,1933 ----

bases = abstract_get_bases(derived);
! if (bases == NULL) {
! if (PyErr_Occurred())
! return -1;
return 0;
! }
n = PyTuple_GET_SIZE(bases);
for (i = 0; i < n; i++) {
***************
*** 1943,1947 ****
PyObject *cls_bases = abstract_get_bases(cls);
if (cls_bases == NULL) {
! PyErr_SetString(PyExc_TypeError,
"isinstance() arg 2 must be a class or type");
return -1;
--- 1974,1980 ----
PyObject *cls_bases = abstract_get_bases(cls);
if (cls_bases == NULL) {
! /* Do not mask errors. */
! if (!PyErr_Occurred())
! PyErr_SetString(PyExc_TypeError,
"isinstance() arg 2 must be a class or type");
return -1;
***************
*** 1978,1982 ****
derived_bases = abstract_get_bases(derived);
if (derived_bases == NULL) {
! PyErr_SetString(PyExc_TypeError,
"issubclass() arg 1 must be a class");
return -1;
--- 2011,2017 ----
derived_bases = abstract_get_bases(derived);
if (derived_bases == NULL) {
! /* Do not mask errors */
! if (!PyErr_Occurred())
! PyErr_SetString(PyExc_TypeError,
"issubclass() arg 1 must be a class");
return -1;
***************
*** 1986,1990 ****
cls_bases = abstract_get_bases(cls);
if (cls_bases == NULL) {
! PyErr_SetString(PyExc_TypeError,
"issubclass() arg 2 must be a class");
return -1;
--- 2021,2027 ----
cls_bases = abstract_get_bases(cls);
if (cls_bases == NULL) {
! /* Do not mask errors */
! if (!PyErr_Occurred())
! PyErr_SetString(PyExc_TypeError,
"issubclass() arg 2 must be a class");
return -1;