I didn't have the heart to keep pestering Moshe, so I delayed a bit on the
latest round (and then left town for a few days). As a result, I didn't
get a chance to comment on this last patch.
There is one problem, and some formatting nits...
On Mon, 28 Feb 2000, Guido van Rossum wrote:
>...
> + static int instance_contains(PyInstanceObject *inst, PyObject *member)
> + {
> + static PyObject *__contains__;
> + PyObject *func, *arg, *res;
> + int ret;
> +
> + if(__contains__ == NULL) {
"Standard Guido Formatting" requires a space between the "if" and the "(".
if, for, while, etc are not functions... they are language constructs.
>...
> + if(PyObject_Cmp(obj, member, &cmp_res) == -1)
> + ret = -1;
> + if(cmp_res == 0)
> + ret = 1;
> + Py_DECREF(obj);
> + if(ret)
> + return ret;
I had suggested to Moshe to follow the logic in PySequence_Contains(), but
he wanted to use PyObject_Cmp() instead. No biggy, but the above code has
a bug:
PyObject_Cmp() does *not* guarantee a value for cmp_res if it returns -1.
Therefore, it is possible for cmp_res to be zero despite an error being
returned from PyObject_Cmp. Thus, you get a false-positive hit.
IMO, this section of code should read:
cmp_res = PyObject_Compare(obj, member);
Py_XDECREF(obj);
if (cmp_res == 0)
return 1;
if (PyErr_Occurred())
return -1;
The "ret" variable becomes unused and can be deleted. Oh! Just noted that
"ret" is declared twice; one hiding the declaration of the other. With the
above change and deletion of the inner "ret", then the hiding declaration
problem is also fixed.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
RetroSearch is an open source project built by @garambo | Open a GitHub Issue
Search and Browse the WWW like it's 1997 | Search results from DuckDuckGo
HTML:
3.2
| Encoding:
UTF-8
| Version:
0.7.4