[Python-Dev] [Python 2.4] PyInt_FromLong returning NULL

[Tim Peters]

>>>                   if (! PyInt_Check(p))
>>>                     {
>>>                       if (PyDict_Check(p))
>>>                         {
>>>                           if (PyString_Check(name) || PyUnicode_Check(name))
>>>                             {
>>>                               ASSIGN(p, PyObject_GetItem(p, name));
>>>                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>
>>>                               if (p == NULL) {
>>>                                   puts("PyObject returned NULL");
>>>                                 PyErr_Clear();
>>>                               }
>>>                             }
>>>                           else
>>>                             p = PyInt_FromLong((long)1);
>>>

[Tim]
>> I note that all of this is nested inside another "if (p) {...}" block.
>> That implies the "p = PyInt_FromLong((long)1);" line is at least a
>> memory leak:  it overwrites p without decref'ing p first.

[Jim]
> The ASSIGN macro DECREFs it's first argument if it is non-NULL.
> 
> It loosly models a Python assignment, assuming that it owns the
> reference to the second argument.

ASSIGN isn't executed on the path in question.  I really can't follow
nesting with that indentation style.  Reformatting in Python style
makes it obvious to me:

if (p) {
    if (! PyInt_Check(p)) {
        if (PyDict_Check(p)) {
            if (PyString_Check(name) || PyUnicode_Check(name)) {
                ASSIGN(p, PyObject_GetItem(p, name));
                if (p == NULL)
                    PyErr_Clear();
            }
            else
                p = PyInt_FromLong(1);
        }
        else {
            ASSIGN(p, callfunction2(p, name, value));
            if (p == NULL)
                goto err;
        }
    }
}

"p = PyInt_FromLong(1)" is in an ``else`` block.  The only ASSIGN
before it is in that ``else`` block's disjoint ``if`` block.