[Python-Dev] RE: [Python-checkins] python/dist/src/Objectstypeobject.c, 2.244, 2.245

[Raymond Hettinger]

[Wojtek Walczak]
> PyInt_FromLong() returns PyObject, so you need to use
PyObject_IsTrue()
> (the way I did) or hack the code not to use PyInt_FromLong(). I used
> PyInt_FromLong() because it was there before. Original code:
> 
>    res = (*func)(self, value);
>    if (res == -1 && PyErr_Occurred())
>       return NULL;
>    return PyInt_FromLong((long)res);
> }
> 
> If you're sure it isn't needed, then of course we can use the easier
way
> changing the snippet above into:
> 
>    res = (*func)(self, value);
>    if (res == -1 && PyErr_Occurred())
>       return NULL;
>    ret = res ? Py_True : Py_False;
>    Py_INCREF(ret);
>    return ret;
> }
> 
> So, why was there PyInt_FromLong()? :>

obj.__contains__() returns a python object.  "res" is a C numeric
object.  So, PyInt_FromLong() was needed to change it from a C long into
a PyObject * to a Python integer (either 0 or 1).

Wrapping that return value in Py_ObjectIsTrue() does successfully
convert the Python integer into a Python bool.

One issue with the way you wrote it is that both PyInt_FromLong() and
Py_ObjectIsTrue() create new Python objects but only one of them is
returned.  The other needs to have its reference count lowered by one so
that obj.__contains__() won't leak.

The other issue is that it wasn't necessary to create an intermediate
PyInt value.  Instead, the PyBool can be created directly from "res"
using PyBool_FromLong() or the equivalent:  ret = res ? Py_True :
Py_False; Py_INCREF(ret); return ret;

Looking at the functions signatures may make it more clear:

  wrap_objobjproc:   argstuple --> PyObject*
  PyInt_FromLong:    long --> PyObject*
  PyObject_IsTrue:   PyObject* --> PyObject*
  PyBool_FromLong:   long --> PyObject*


Hope this helps,


Raymond Hettinger