[Python-Dev] Big trouble in CVS Python

Guido van Rossum guido@python.org
Mon, 14 Apr 2003 16:13:31 -0400


> On Sun, 2003-04-13 at 13:28, Tim Peters wrote:
> 
> > It appears to be a refcount error in recently-added C code that tries to
> > generalize the builtin range() function, specifically here:
> > 
> >   Fail:
> > 	Py_XDECREF(curnum);
> > 	Py_XDECREF(istep);  <- here
> > 	Py_XDECREF(zero);
> > 
> > Word to the wise:  don't ever try to reuse a variable whose address is
> > passed to PyArg_ParseTuple for anything other than holding what
> > PyArg_ParseTuple does or doesn't store into it.
> 
> Hmm, then this is my fault.  I did exactly that.  My approach was to
> Py_INCREF an optional argument it if it was given (ie. not NULL),
> otherwise to create it from scratch, and Py_DECREF when I was done.  I
> believe this was a not uncommon idiom (I can't recal the specifics, but
> being my first submitted patch, I made sure to try to look for existing
> idioms for argument and error handling).  I apologize if I erred.
> 
>   I assume a better approach, then is to get the optional istep
> argument, and copy it into a variable for your own use (or create it if
> it didn't exist)?  ie. Never increment or decrement the optional
> argument object, returned from PyArg_ParseTuple, at all?
> 
> >   You'll never get the
> > decrefs straight (and even if you manage to at first, the next person to
> > modify your code will break it).
> 
> Bingo!   Guido took a slightly different approach (and ultimately a
> better one, I think), in adapting my patch.  Perhaps I unknowingly left
> a time bomb for him.

Sort of.  Your code didn't have the refcount bug; I moved the
initialization of 'zero' up, and changed a few 'return NULL' lines
into 'goto Fail', but I didn't move the 'INCREF(istep)' up.

> I'll submit a patch to fix this all up tonight, if it hasn't already
> been addressed by then.

Tim fixed it already.

--Guido van Rossum (home page: http://www.python.org/~guido/)