Wrote down the invariants of some common objects whose structure is

arigo · arigo · commit 89a39461bff0 · 2004-10-28T16:32:00.000Z
exposed in header files.  Fixed a few comments in these headers.

As we might have expected, writing down invariants systematically exposed a
(minor) bug.  In this case, function objects have a writeable func_code
attribute, which could be set to code objects with the wrong number of
free variables.  Calling the resulting function segfaulted the interpreter.
Added a corresponding test.
diff --git a/Include/cellobject.h b/Include/cellobject.h
@@ -8,7 +8,7 @@ extern "C" {
 
 typedef struct {
 	PyObject_HEAD
-	PyObject *ob_ref;
+	PyObject *ob_ref;	/* Content of the cell or NULL when empty */
 } PyCellObject;
 
 PyAPI_DATA(PyTypeObject) PyCell_Type;
diff --git a/Include/funcobject.h b/Include/funcobject.h
@@ -7,17 +7,34 @@
 extern "C" {
 #endif
 
+/* Function objects and code objects should not be confused with each other:
+ *
+ * Function objects are created by the execution of the 'def' statement.
+ * They reference a code object in their func_code attribute, which is a
+ * purely syntactic object, i.e. nothing more than a compiled version of some
+ * source code lines.  There is one code object per source code "fragment",
+ * but each code object can be referenced by zero or many function objects
+ * depending only on how many times the 'def' statement in the source was
+ * executed so far.
+ */
+
 typedef struct {
     PyObject_HEAD
-    PyObject *func_code;
-    PyObject *func_globals;
-    PyObject *func_defaults;
-    PyObject *func_closure;
-    PyObject *func_doc;
-    PyObject *func_name;
-    PyObject *func_dict;
-    PyObject *func_weakreflist;
-    PyObject *func_module;
+    PyObject *func_code;	/* A code object */
+    PyObject *func_globals;	/* A dictionary (other mappings won't do) */
+    PyObject *func_defaults;	/* NULL or a tuple */
+    PyObject *func_closure;	/* NULL or a tuple of cell objects */
+    PyObject *func_doc;		/* The __doc__ attribute, can be anything */
+    PyObject *func_name;	/* The __name__ attribute, a string object */
+    PyObject *func_dict;	/* The __dict__ attribute, a dict or NULL */
+    PyObject *func_weakreflist;	/* List of weak references */
+    PyObject *func_module;	/* The __module__ attribute, can be anything */
+
+    /* Invariant:
+     *     func_closure contains the bindings for func_code->co_freevars, so
+     *     PyTuple_Size(func_closure) == PyCode_GetNumFree(func_code)
+     *     (func_closure may be NULL if PyCode_GetNumFree(func_code) == 0).
+     */
 } PyFunctionObject;
 
 PyAPI_DATA(PyTypeObject) PyFunction_Type;
diff --git a/Include/intobject.h b/Include/intobject.h
@@ -11,7 +11,7 @@ returns -1 and sets errno to EBADF if the object is not an PyIntObject.
 None of the functions should be applied to nil objects.
 
 The type PyIntObject is (unfortunately) exposed here so we can declare
-_Py_TrueStruct and _Py_ZeroStruct below; don't use this.
+_Py_TrueStruct and _Py_ZeroStruct in boolobject.h; don't use this.
 */
 
 #ifndef Py_INTOBJECT_H
diff --git a/Include/listobject.h b/Include/listobject.h
@@ -31,6 +31,9 @@ typedef struct {
      *     len(list) == ob_size
      *     ob_item == NULL implies ob_size == allocated == 0
      * list.sort() temporarily sets allocated to -1 to detect mutations.
+     *
+     * Items must normally not be NULL, except during construction when
+     * the list is not yet visible outside the function that builds it.
      */
     int allocated;
 } PyListObject;
diff --git a/Include/methodobject.h b/Include/methodobject.h
@@ -7,6 +7,10 @@
 extern "C" {
 #endif
 
+/* This is about the type 'builtin_function_or_method',
+   not Python methods in user-defined classes.  See classobject.h
+   for the latter. */
+
 PyAPI_DATA(PyTypeObject) PyCFunction_Type;
 
 #define PyCFunction_Check(op) ((op)->ob_type == &PyCFunction_Type)
@@ -31,10 +35,11 @@ PyAPI_FUNC(int) PyCFunction_GetFlags(PyObject *);
 PyAPI_FUNC(PyObject *) PyCFunction_Call(PyObject *, PyObject *, PyObject *);
 
 struct PyMethodDef {
-    char	*ml_name;
-    PyCFunction  ml_meth;
-    int		 ml_flags;
-    char	*ml_doc;
+    char	*ml_name;	/* The name of the built-in function/method */
+    PyCFunction  ml_meth;	/* The C function that implements it */
+    int		 ml_flags;	/* Combination of METH_xxx flags, which mostly
+				   describe the args expected by the C func */
+    char	*ml_doc;	/* The __doc__ attribute, or NULL */
 };
 typedef struct PyMethodDef PyMethodDef;
 
@@ -75,9 +80,9 @@ PyAPI_FUNC(PyObject *) Py_FindMethodInChain(PyMethodChain *, PyObject *,
 
 typedef struct {
     PyObject_HEAD
-    PyMethodDef *m_ml;
-    PyObject    *m_self;
-    PyObject    *m_module;
+    PyMethodDef *m_ml; /* Description of the C function to call */
+    PyObject    *m_self; /* Passed as 'self' arg to the C func, can be NULL */
+    PyObject    *m_module; /* The __module__ attribute, can be anything */
 } PyCFunctionObject;
 
 #ifdef __cplusplus
diff --git a/Include/rangeobject.h b/Include/rangeobject.h
@@ -7,6 +7,9 @@
 extern "C" {
 #endif
 
+/* This is about the type 'xrange', not the built-in function range(), which
+   returns regular lists. */
+
 /*
 A range object represents an integer range.  This is an immutable object;
 a range cannot change its value after creation.
diff --git a/Include/setobject.h b/Include/setobject.h
@@ -16,6 +16,14 @@ typedef struct {
 	PyObject *data;
 	long hash;	/* only used by frozenset objects */
 	PyObject *weakreflist; /* List of weak references */
+
+	/* Invariants:
+	 *     data is a dictionary whose values are all True.
+	 *     data points to the same dict for the whole life of the set.
+	 * For frozensets only:
+	 *     data is immutable.
+	 *     hash is the hash of the frozenset or -1 if not computed yet.
+	 */
 } PySetObject;
 
 PyAPI_DATA(PyTypeObject) PySet_Type;
diff --git a/Include/sliceobject.h b/Include/sliceobject.h
@@ -16,12 +16,12 @@ PyAPI_DATA(PyObject) _Py_EllipsisObject; /* Don't use this directly */
 
 A slice object containing start, stop, and step data members (the
 names are from range).  After much talk with Guido, it was decided to
-let these be any arbitrary python type. 
+let these be any arbitrary python type.  Py_None stands for omitted values.
 */
 
 typedef struct {
     PyObject_HEAD
-    PyObject *start, *stop, *step;
+    PyObject *start, *stop, *step;	/* not NULL */
 } PySliceObject;
 
 PyAPI_DATA(PyTypeObject) PySlice_Type;
diff --git a/Include/stringobject.h b/Include/stringobject.h
@@ -37,6 +37,15 @@ typedef struct {
     long ob_shash;
     int ob_sstate;
     char ob_sval[1];
+
+    /* Invariants:
+     *     ob_sval contains space for 'ob_size+1' elements.
+     *     ob_sval[ob_size] == 0.
+     *     ob_shash is the hash of the string or -1 if not computed yet.
+     *     ob_sstate != 0 iff the string object is in stringobject.c's
+     *       'interned' dictionary; in this case the two references
+     *       from 'interned' to this object are *not counted* in ob_refcnt.
+     */
 } PyStringObject;
 
 #define SSTATE_NOT_INTERNED 0
diff --git a/Include/tupleobject.h b/Include/tupleobject.h
@@ -8,9 +8,11 @@ extern "C" {
 #endif
 
 /*
-Another generally useful object type is an tuple of object pointers.
-This is a mutable type: the tuple items can be changed (but not their
-number).  Out-of-range indices or non-tuple objects are ignored.
+Another generally useful object type is a tuple of object pointers.
+For Python, this is an immutable type.  C code can change the tuple items
+(but not their number), and even use tuples are general-purpose arrays of
+object references, but in general only brand new tuples should be mutated,
+not ones that might already have been exposed to Python code.
 
 *** WARNING *** PyTuple_SetItem does not increment the new item's reference
 count, but does decrement the reference count of the item it replaces,
@@ -22,6 +24,11 @@ returned item's reference count.
 typedef struct {
     PyObject_VAR_HEAD
     PyObject *ob_item[1];
+
+    /* ob_item contains space for 'ob_size' elements.
+     * Items must normally not be NULL, except during construction when
+     * the tuple is not yet visible outside the function that builds it.
+     */
 } PyTupleObject;
 
 PyAPI_DATA(PyTypeObject) PyTuple_Type;
diff --git a/Lib/test/test_funcattrs.py b/Lib/test/test_funcattrs.py
@@ -218,11 +218,11 @@ def temp():
 
 # Test all predefined function attributes systematically
 
-def cantset(obj, name, value):
+def cantset(obj, name, value, exception=(AttributeError, TypeError)):
     verify(hasattr(obj, name)) # Otherwise it's probably a typo
     try:
         setattr(obj, name, value)
-    except (AttributeError, TypeError):
+    except exception:
         pass
     else:
         raise TestFailed, "shouldn't be able to set %s to %r" % (name, value)
@@ -279,11 +279,20 @@ def f(): pass
 
 
 def test_func_code():
+    a = b = 24
     def f(): pass
     def g(): print 12
+    def f1(): print a
+    def g1(): print b
+    def f2(): print a, b
     verify(type(f.func_code) is types.CodeType)
     f.func_code = g.func_code
     cantset(f, "func_code", None)
+    # can't change the number of free vars
+    cantset(f,  "func_code", f1.func_code, exception=ValueError)
+    cantset(f1, "func_code",  f.func_code, exception=ValueError)
+    cantset(f1, "func_code", f2.func_code, exception=ValueError)
+    f1.func_code = g1.func_code
 
 def test_func_defaults():
     def f(a, b): return (a, b)
diff --git a/Objects/funcobject.c b/Objects/funcobject.c
@@ -230,6 +230,7 @@ static int
 func_set_code(PyFunctionObject *op, PyObject *value)
 {
 	PyObject *tmp;
+	int nfree, nclosure;
 
 	if (restricted())
 		return -1;
@@ -240,6 +241,17 @@ func_set_code(PyFunctionObject *op, PyObject *value)
 				"func_code must be set to a code object");
 		return -1;
 	}
+	nfree = PyCode_GetNumFree((PyCodeObject *)value);
+	nclosure = (op->func_closure == NULL ? 0 :
+		    PyTuple_GET_SIZE(op->func_closure));
+	if (nclosure != nfree) {
+		PyErr_Format(PyExc_ValueError,
+			     "%s() requires a code object with %d free vars,"
+			     " not %d",
+			     PyString_AsString(op->func_name),
+			     nclosure, nfree);
+		return -1;
+	}
 	tmp = op->func_code;
 	Py_INCREF(value);
 	op->func_code = value;
