[Python-checkins] python/dist/src/Objects abstract.c,2.93.6.5,2.93.6.6

mwh@users.sourceforge.net mwh@users.sourceforge.net
Tue, 24 Sep 2002 04:08:25 -0700


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

Modified Files:
      Tag: release22-maint
	abstract.c 
Log Message:
backport bwarsaw's checkin of
    revision 2.101 of abstract.c

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.

----

Whitespace normalization made this a pest to backport...

Did a test case ever get added for this?


Index: abstract.c
===================================================================
RCS file: /cvsroot/python/python/dist/src/Objects/abstract.c,v
retrieving revision 2.93.6.5
retrieving revision 2.93.6.6
diff -C2 -d -r2.93.6.5 -r2.93.6.6
*** abstract.c	28 Jul 2002 10:21:30 -0000	2.93.6.5
--- abstract.c	24 Sep 2002 11:08:23 -0000	2.93.6.6
***************
*** 1866,1869 ****
--- 1866,1895 ----
  /* 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)
***************
*** 1877,1887 ****
  			return NULL;
  	}
- 
  	bases = PyObject_GetAttr(cls, __bases__);
! 	if (bases == NULL || !PyTuple_Check(bases)) {
! 	        Py_XDECREF(bases);
  		return NULL;
  	}
- 
  	return bases;
  }
--- 1903,1916 ----
  			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;
  }
***************
*** 1900,1906 ****
  
  	bases = abstract_get_bases(derived);
! 	if (bases == NULL)
  		return 0;
! 
  	n = PyTuple_GET_SIZE(bases);
  	for (i = 0; i < n; i++) {
--- 1929,1937 ----
  
  	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++) {
***************
*** 1947,1954 ****
  		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;
! 		} 
  		Py_DECREF(cls_bases);
  		if (__class__ == NULL) {
--- 1978,1987 ----
  		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;
! 		}
  		Py_DECREF(cls_bases);
  		if (__class__ == NULL) {
***************
*** 1982,1986 ****
  		derived_bases = abstract_get_bases(derived);
  		if (derived_bases == NULL) {
! 			PyErr_SetString(PyExc_TypeError, 
  					"issubclass() arg 1 must be a class");
  			return -1;
--- 2015,2021 ----
  		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;
***************
*** 1990,1994 ****
  		cls_bases = abstract_get_bases(cls);
  		if (cls_bases == NULL) {
! 			PyErr_SetString(PyExc_TypeError, 
  					"issubclass() arg 2 must be a class");
  			return -1;
--- 2025,2031 ----
  		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;