[SciPy-dev] RE:[PATCH] zeros.c

[Chuck Harris]

This patch does the following:

1) If the passed function raises an exception, that is the exception reported.
2) If the passed function does not return a valid value, the report is made clearer.
3) Small formatting cleanup
4) Documentation strings are added for the module functions.
5) A reference counting bug is fixed

The reference counting bug is easy to trigger by passing the extra argument. Say

def f(x): return x - .5

foo = 'fubar'
for i in range(1000) : bisect(f,0,1,args=foo)

at some point foo will be garbage collected and disappears. I must admit that
I haven't tried precisely this example and no longer have the original installed
to check, but it should work. Oddly enough, Idle also seems to suffer from a 
reference counting bug, so using sys.getrefcount may return unexpected results.

The reason this bug shows up is that PyTuple_GetItem borrows a reference and
PyTuple_SET_ITEM steals it, so when the tuple is destroyed the reference count
of the item is decremented instead of being left in its original state.

The lines all seem to end in ^M, as reported by emacs. It is not visible here,
but there may be some sort of line end problem. I got the source from CVS using 
http to download the files. Is this as it should be?

Chuck
==================PATCH==================
--- zeros-old.c	Wed May 29 13:40:50 2002
+++ zeros.c	Wed May 29 11:39:09 2002
@@ -40,11 +40,18 @@
     args = myparams->args;
     f = myparams->function;
     PyTuple_SetItem(args,0,Py_BuildValue("d",x));
-    val = PyFloat_AsDouble(retval=PyObject_CallObject(f,args));
-    Py_XDECREF(retval);
+    retval = PyObject_CallObject(f,args); 
+    if (PyErr_Occurred()) {
+        Py_XDECREF(retval);
+        longjmp(myparams->env, 1);
+    }
+    val = PyFloat_AsDouble(retval);
     if (PyErr_Occurred()) {
+        PyErr_SetString(PyExc_RuntimeError,"Could not convert function value to double");
+        Py_XDECREF(retval);
         longjmp(myparams->env, 1);
     }
+    Py_XDECREF(retval);
     return val;    
 }
 
@@ -62,11 +69,10 @@
     PyObject *f,*xargs,*item,*fargs=NULL;
 
     
-    if (!PyArg_ParseTuple(args,"OdddiOi|i",&f,&a,&b,&xtol,&iter,&xargs,&fulloutput,&disp)) 
-        {
-            PyErr_SetString(PyExc_RuntimeError,"Unable to parse arguments");
-            return NULL;
-        }
+    if (!PyArg_ParseTuple(args,"OdddiOi|i",&f,&a,&b,&xtol,&iter,&xargs,&fulloutput,&disp)) {
+        PyErr_SetString(PyExc_RuntimeError,"Unable to parse arguments");
+        return NULL;
+    }
     if (xtol < 0) {
         PyErr_SetString(PyExc_ValueError,"xtol must be >= 0");
         return NULL;
@@ -87,6 +93,7 @@
     for (i = 0; i < len; i++) {
         item = PyTuple_GetItem(xargs, i);
         if (item == NULL) { Py_DECREF(fargs); return NULL;}
+	Py_INCREF(item);
         PyTuple_SET_ITEM(fargs,i+1,item);
     }
 
@@ -154,10 +161,10 @@
 
 static PyMethodDef 
 Zerosmethods[] = {
-	{"_bisect", _bisect, METH_VARARGS, "a"},
-	{"_ridder", _ridder, METH_VARARGS, "a"},
-	{"_brenth", _brenth, METH_VARARGS, "a"},
-	{"_brentq", _brentq, METH_VARARGS, "a"},
+	{"_bisect", _bisect, METH_VARARGS, "bisect(f,a,b)"},
+	{"_ridder", _ridder, METH_VARARGS, "ridder(f,a,b)"},
+	{"_brenth", _brenth, METH_VARARGS, "brenth(f,a,b)"},
+	{"_brentq", _brentq, METH_VARARGS, "brentq(f,a,b)"},
 	{NULL, NULL}
 };