[Patches] [ python-Patches-1444030 ] Several minor fixes for NULL checks, etc.

[SourceForge.net]

Patches item #1444030, was opened at 2006-03-06 03:03
Message generated for change (Comment added) made by nnorwitz
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1444030&group_id=5470

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: None
Group: Python 2.5
Status: Open
Resolution: None
Priority: 4
Submitted By: Hye-Shik Chang (perky)
>Assigned to: Hye-Shik Chang (perky)
Summary: Several minor fixes for NULL checks, etc.

Initial Comment:
Attached patch includes fixes for missing NULL checks,
reference leaks on minor cases found by a static
analysis tool.

It touches:
Python/ceval.c
Python/traceback.c
Python/ast.c
Python/modsupport.c
Objects/object.c
Objects/weakrefobject.c
Objects/unicodeobject.c
Objects/longobject.c
Objects/stringobject.c
Parser/firstsets.c
Modules/arraymodule.c
Modules/zipimport.c
Modules/cStringIO.c


----------------------------------------------------------------------

>Comment By: Neal Norwitz (nnorwitz)
Date: 2006-03-06 15:20

Message:
Logged In: YES 
user_id=33168

Thanks Perky!

I haven't gotten access to the report yet.  All these look
fine, with the following exceptions:

Python/modsupport.c:  This looks like it's a false positive,
presumably because of if ((v = PyList_New(n)) == NULL).  I
changed that code and cleaned up the conditions below.  I
think the original code was more complex than it needs to
be.  I checked in this change to modsupport.c.

Objects/object.c:  To be consistent with PyObject_Str() this
should return u"<NULL>".  The current code is wrong, but the
patch just returns NULL.  I *think* the correct thing to do
is set v instead of res, then if that result is NULL, return
NULL, ie:

  v = PyString_FromString("<NULL>");
  if (v == NULL)
    return NULL;

Objects/weakrefobject.c:  Your addition is correct, but I
wonder if you also need to restore the error before
returning like the code below?

        if (restore_error)
            PyErr_Restore(err_type, err_value, err_tb);

Objects/unicodeobject.c: [block 1945]  the current code is
correct, but perhaps a bit confusing.  goto ucnhashError is
called in 3 places I see.  The first 2 places, v is not set
yet.  In the third place, v was already DECREF'd.  Perhaps
after the DECREF, we should do: v = NULL; ?

All other unicode changes are correct.

Objects/longobject.c:  I disagree with these changes, but
the code is broken.  I don't believe we should be checking
for a or b being NULL.  I think the code should be this (the
first line is just for context):

        z = _PyLong_New(size_z);
        if (z == NULL)
                return NULL;

The callers take care of DECREF'ing a and b and we didn't
allocate either of them in this function.

Objects/stringobject.c:  Since list is always NULL if going
to onError, the DECREF is not needed.  I would get rid of
the whole onError label and just return NULL if the list
alloc fails.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1444030&group_id=5470