Wrote down the invariants of some common objects whose structure is

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.

12 files changed, 98 insertions, 25 deletions

diff --git a/Include/cellobject.h b/Include/cellobject.h
index 2a6dcaa..fd186e2 100644
--- 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
index 758c76d..59c19bb 100644
--- 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
index 61ef0f0..1bbd59c 100644
--- 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
index e4867a5..0999a82 100644
--- 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
index 0f60549..9736dc3 100644
--- 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
index b6f571b..50aa061 100644
--- 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
index abbd847..cc2d683 100644
--- 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
index 1fb123a..fc80254 100644
--- 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
index 3deea8f..0c7e5b6 100644
--- 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
index f1839fe..6b60d62 100644
--- 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
index 381412f..1acfeb5 100644
--- a/Lib/test/test_funcattrs.py
+++ b/Lib/test/test_funcattrs.py
@@ -218,11 +218,11 @@ d[foo]
 
 # 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 test_func_name():
 
 
 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
index c46887c..c7f7c9d 100644
--- 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;