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

Wed Oct 8 23:45:45 EDT 2003

> 	if (res == -1 && PyErr_Occurred())
> 		return NULL;
> ! 	return PyInt_FromLong((long)res);
> }
>> --- 3577,3583 ----
> 	if (res == -1 && PyErr_Occurred())
> 		return NULL;
> ! 	ret = PyObject_IsTrue(PyInt_FromLong((long)res)) ? Py_True :
> Py_False;
>>> The line above leaks and does unnecessary work. I believe it should
> read:
>> 	ret = res ? Py_True : Py_False;

Ai. I did the review while only half awake. :-)
But the correct thing to do is to use PyBool_FromLong(res); there's
really no need to inline what that function does.
> Also, there is another one of these in Objects/descrobject.c line 712.

I'll fix that one while I'm at it.
BTW, I notice there are a bunch of uses of PyBool_FromLong() that are
preceded by something like "if (res < 0) return NULL;" (or "!= -1").
Maybe PyBool_FromLong() itself could make this unneeded by adding
something like
 if (ok < 0 && PyErr_Occurred())
 return NULL;
to its start?
And, while we're reviewing usage patterns of PyBool_FromLong(), the
string and unicode types are full of places where it is called by a
return statement with a constant 1 or 0 as argument. This seems
wasteful to me; I imagine that
 Py_INCREF(Py_True);
 return Py_True;
takes less time than
 return PyBool_FromLong(1);
Maybe a pair of macros Py_return_True and Py_return_False would make
sense?
--Guido van Rossum (home page: http://www.python.org/~guido/)