Took a crack at figuring out how the set iteration protocol really works.

Added lots of comments as a result.  Redid parts of multiunion() because
it was clearly leaking references tucked away inside its SetIteration
struct.  Added XXX comments in another place I'm pretty sure is leaking
similar references in error cases (but am too tired to fix that now, if
so).

src/BTrees/BTreeModuleTemplate.c

@@ -173,14 +173,49 @@ staticforward PyExtensionClass BTreeType;
     (RESULT) = _i;                                          \
 }
 
+/* SetIteration structs are used in the internal set iteration protocol.
+ * When you want to iterate over a set or bucket or BTree (even an
+ * individual key!),
+ * 1. Declare a new iterator and a "merge" int:
+ *        SetIteration si = {0,0,0};
+ *        int merge = 0;
+ *    XXX Using "{0,0,0}" or "{0,0}" appear most common, but I don't
+ *    XXX think it makes any difference; looks like "{0}" would work too;
+ *    XXX looks like not initializing it at all would also work.
+ * 2. Initialize it via
+ *        initSetIteration(&si, PyObject *s, int weight, &merge)
+ *    There's an error if that returns an int < 0.  Note that si.set must
+ *        be Py_XDECREF'ed in this case.
+ *    If it's successful, si.hasValue is set to true iff s has values (as
+ *    well as keys).
+ * 3. Get the first element:
+ *        if (si.next(&si) < 0) { there was an error }
+ *    If the set isn't empty, this sets si.position to an int >= 0,
+ *    si.key to the element's key (of type KEY_TYPE), and si.value to
+ *    the element's value (of type VALUE_TYPE).  si.value is defined
+ *    iff merge was set to true by the initSetIteration() call.  If
+ *    there was an error, the caller is responsible for Py_XDECREF'ing
+ *    si.set.
+ * 4. Process all the elements:
+ *        while (si.position >= 0) {
+ *            do something si.key and/or si.value;
+ *            if (si.next(&si) < 0) {
+ *                there was an error;
+ *                Py_XDECREF(si.set);
+ *                do whatever else is appropriate for the caller;
+ *            }
+ *        }
+ * 5. Decref the SetIteration's set:
+ *        Py_XDECREF(si.set);
+ */
 typedef struct SetIteration_s
 {
-  PyObject *set;
-  int position;
-  int hasValue;
-  KEY_TYPE key;
-  VALUE_TYPE value;
-  int (*next)(struct SetIteration_s*);
+  PyObject *set;    /* the set, bucket, BTree, ..., being iterated */
+  int position;     /* initialized to 0; set to -1 by next() when done */
+  int hasValue;     /* true iff 'set' has values (as well as keys) */
+  KEY_TYPE key;     /* next() sets to next key */
+  VALUE_TYPE value; /* next() sets to next value, iff hasValue is true */
+  int (*next)(struct SetIteration_s*);  /* function to get next element */
 } SetIteration;
 
 static PyObject *
@@ -349,7 +384,7 @@ static char BTree_module_documentation[] =
 "\n"
 MASTER_ID
 BTREEITEMSTEMPLATE_C
-"$Id: BTreeModuleTemplate.c,v 1.27 2002/05/31 20:01:16 tim_one Exp $\n"
+"$Id: BTreeModuleTemplate.c,v 1.28 2002/06/02 07:40:20 tim_one Exp $\n"
 BTREETEMPLATE_C
 BUCKETTEMPLATE_C
 KEYMACROS_H

src/BTrees/MergeTemplate.c

@@ -12,7 +12,7 @@
   
  ****************************************************************************/
 
-#define MERGETEMPLATE_C "$Id: MergeTemplate.c,v 1.11 2002/05/31 14:58:29 tim_one Exp $\n"
+#define MERGETEMPLATE_C "$Id: MergeTemplate.c,v 1.12 2002/06/02 07:40:20 tim_one Exp $\n"
 
 /****************************************************************************
  Set operations
@@ -60,6 +60,9 @@ bucket_merge(Bucket *s1, Bucket *s2, Bucket *s3)
   SetIteration i1 = {0,0,0}, i2 = {0,0,0}, i3 = {0,0,0};
   int cmp12, cmp13, cmp23, mapping=0, set;
 
+  /* XXX Looks like the various "return NULL;" exits from here on can
+   * XXX leak references saved away in i1.set, i2.set, i3.set.
+   */
   if (initSetIteration(&i1, OBJECT(s1), 0, &mapping) < 0) return NULL;
   if (initSetIteration(&i2, OBJECT(s2), 0, &mapping) < 0) return NULL;
   if (initSetIteration(&i3, OBJECT(s3), 0, &mapping) < 0) return NULL;

src/BTrees/SetOpTemplate.c

@@ -16,7 +16,7 @@
  Set operations
  ****************************************************************************/
 
-#define SETOPTEMPLATE_C "$Id: SetOpTemplate.c,v 1.16 2002/06/01 00:49:19 tim_one Exp $\n"
+#define SETOPTEMPLATE_C "$Id: SetOpTemplate.c,v 1.17 2002/06/02 07:40:20 tim_one Exp $\n"
 
 #ifdef INTSET_H
 static int 
@@ -49,6 +49,9 @@ nextIntSet(SetIteration *i)
 static int 
 nextKeyAsSet(SetIteration *i)
 {
+  /* XXX Looks like this block could be replaced by
+   * XXX   i->position = i->position == 0 ? 1 : -1;
+   */
   if (i->position >= 0)
     {
       if (i->position < 1)
@@ -62,10 +65,36 @@ nextKeyAsSet(SetIteration *i)
 }
 #endif
 
+/* initSetIteration
+ *
+ * Start the set iteration protocol.  See the comments at struct SetIteration.
+ *
+ * Arguments
+ *      i       The address of a SetIteration control struct.
+ *      s       The address of the set, bucket, BTree, ..., to be iterated.
+ *      w       If w < 0, ignore values when iterating.
+ *      merge   Is set to 1 if s has values (as well as keys) and w >= 0.
+ *              The value on input is ignored.
+ *
+ * Return
+ *      0 on success; -1 and an exception set if error.
+ *      merge is set to 1 if s has values and w >= 0, else merge is left
+ *          alone.
+ *      i.set gets a new reference to s, or to some other object used to
+ *          iterate over s.  The caller must Py_XDECREF i.set when it's
+ *          done with i, and regardless of whether the call to
+ *          initSetIteration succeeds or fails (a failing call may or not
+ *          set i.set to NULL).
+ *      i.position is set to 0.
+ *      i.hasValue is set to true if s has values, and to false otherwise.
+ *          Note that this is done independent of w's value.
+ *      i.next is set to an appropriate iteration function.
+ *      i.key and i.value are left alone.
+ */
 static int
 initSetIteration(SetIteration *i, PyObject *s, int w, int *merge)
 {
-  i->position=0;
+  i->position = 0;
 
   if (ExtensionClassSubclassInstance_Check(s, &BucketType))
     {
@@ -416,6 +445,7 @@ multiunion_m(PyObject *ignored, PyObject *args)
     int n;                  /* length of input sequence */
     PyObject *set = NULL;   /* an element of the input sequence */
     Bucket *result;         /* result set */
+    SetIteration setiter = {0, 0, 0};
     int i;
 
     UNLESS(PyArg_ParseTuple(args, "O", &seq))
@@ -440,6 +470,7 @@ multiunion_m(PyObject *ignored, PyObject *args)
         /* If set is a bucket, do a straight resize + memcpy. */
         if (set->ob_type == (PyTypeObject*)&SetType ||
             set->ob_type == (PyTypeObject*)&BucketType) {
+
             const int setsize = SIZED(set)->len;
             int size_desired = result->len + setsize;
             /* If there are more to come, overallocate by 25% (arbitrary). */
@@ -456,7 +487,6 @@ multiunion_m(PyObject *ignored, PyObject *args)
         }
         else {
             /* No cheap way:  iterate over set's elements one at a time. */
-            SetIteration setiter = {0, 0, 0};
             int merge;  /* dummy needed for initSetIteration */
 
             if (initSetIteration(&setiter, set, 1, &merge) < 0)
@@ -472,6 +502,8 @@ multiunion_m(PyObject *ignored, PyObject *args)
                 if (setiter.next(&setiter) < 0)
                     goto Error;
             }
+            Py_XDECREF(setiter.set);
+            setiter.set = NULL;
         }
         Py_DECREF(set);
         set = NULL;
@@ -492,6 +524,7 @@ multiunion_m(PyObject *ignored, PyObject *args)
 Error:
     Py_DECREF(result);
     Py_XDECREF(set);
+    Py_XDECREF(setiter.set);
     return NULL;
 }