[Zodb-checkins] CVS: Zope3/src/zodb/btrees - BucketTemplate.c:1.14

Tim Peters tim.one@comcast.net
Fri, 11 Apr 2003 15:10:18 -0400


Update of /cvs-repository/Zope3/src/zodb/btrees
In directory cvs.zope.org:/tmp/cvs-serv11788/src/zodb/btrees

Modified Files:
	BucketTemplate.c 
Log Message:
_bucket_set():  This could leave a mapping bucket in a variety of insane
states when the value passed in was of the wrong type (for example,
doing
    b[obj] = 3.7
when b is an OIBTree).  This manifested as a refcount leak in the test
suite, but could have been much worse (most likely in real life is that
a seemingly arbitrary existing key would "go missing").


=== Zope3/src/zodb/btrees/BucketTemplate.c 1.13 => 1.14 ===
--- Zope3/src/zodb/btrees/BucketTemplate.c:1.13	Thu Apr 10 16:21:39 2003
+++ Zope3/src/zodb/btrees/BucketTemplate.c	Fri Apr 11 15:10:17 2003
@@ -296,19 +296,27 @@
 {
     int i, cmp;
     KEY_TYPE key;
+    VALUE_TYPE value;
     int result = -1;    /* until proven innocent */
     int copied = 1;
 
     COPY_KEY_FROM_ARG(key, keyarg, copied);
     UNLESS(copied) return -1;
 
+    /* Copy the value early (if needed), so that in case of error a
+     * pile of bucket mutations don't need to be undone.
+     */
+    if (v && !noval) {
+    	COPY_VALUE_FROM_ARG(value, v, copied);
+    	UNLESS(copied) return -1;
+    }
+
     PyPersist_INCREF(self);
     if (!PyPersist_IS_STICKY(self))
         return -1;
 
     BUCKET_SEARCH(i, cmp, self, key, goto Done);
     if (cmp == 0) {
-        VALUE_TYPE value;
         /* The key exists, at index i. */
 
         if (v) {
@@ -322,8 +330,6 @@
             }
 
             /* The key exists at index i, and we need to replace the value. */
-            COPY_VALUE_FROM_ARG(value, v, copied);
-            UNLESS(copied) goto Done;
 #ifdef VALUE_SAME
             /* short-circuit if no change */
             if (VALUE_SAME(self->values[i], value)) {
@@ -396,8 +402,7 @@
     INCREF_KEY(self->keys[i]);
 
     if (! noval) {
-        COPY_VALUE_FROM_ARG(self->values[i], v, copied);
-        UNLESS(copied) return -1;
+        COPY_VALUE(self->values[i], value);
         INCREF_VALUE(self->values[i]);
     }