[Patches] [ python-Patches-1752184 ] PyHeapTypeObject fix

SourceForge.net noreply at sourceforge.net
Mon Jul 16 23:39:08 CEST 2007


Patches item #1752184, was opened at 2007-07-11 15:46
Message generated for change (Comment added) made by gvanrossum
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1752184&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 3000
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Thomas Heller (theller)
Assigned to: Guido van Rossum (gvanrossum)
Summary: PyHeapTypeObject fix

Initial Comment:
This patch makes sure that the PyHeapTypeObject's ht_name member always contains a PyUnicode_Object.  Other code relies on this.

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

>Comment By: Guido van Rossum (gvanrossum)
Date: 2007-07-16 17:39

Message:
Logged In: YES 
user_id=6380
Originator: NO

Hm...  Python-ast.c is the *output* of a generating step, according to the
top comment.  So that fix ought to be applied somewhere else.

Also, please add XXX in front of the comment "This test is buggy".

Otherwise, looks fine.

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

Comment By: Thomas Heller (theller)
Date: 2007-07-13 05:43

Message:
Logged In: YES 
user_id=11105
Originator: YES

I did not understand the SystemError in test_ast, but the second version
of the patch does not trigger it any longer.  It also does no longer accept
PyString in type_set_name, and removes the refcount leak.

The test for null bytes I did not change, a generic api would be great to
replace it.
File Added: typeobject.diff

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

Comment By: Guido van Rossum (gvanrossum)
Date: 2007-07-11 17:31

Message:
Logged In: YES 
user_id=6380
Originator: NO

Some test results: with this patch, test_ast fails weirdly:
test test_ast crashed -- <type 'SystemError'>: bad format char passed to
Py_BuildValue

I added assert(PyUnicode_Check(value)); to the top of the function and it
never triggered during any of the unit tests (I am in a debug build).  So
that's a safe assumption.

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

Comment By: Guido van Rossum (gvanrossum)
Date: 2007-07-11 17:15

Message:
Logged In: YES 
user_id=6380
Originator: NO

You're right there's a refcount leak.  Can you rework the patch to avoid
this?  One way would be to remove the later INCREF(value) and move that
into the code for when value is already a unicode. Then all the error
returns from them on must DECREF(value).

Also, the test for null bytes is not correct if there can be non-ASCII
characters in the name (as Martin will eventually implement), since it is
comparing the strlen() of the 8-bit string (which is UTF-8) with the length
of the Unicode string.  I wonder if we shouldn't add a generic API that
checks for null characters in a string, as I expect we'll need this in
other places too and the correct idiom is much more complicated now.

I also think that you might be able to simply insist on unicode, rather
than accepting both string and unicode.  That way we'll find any remaining
offenders more quickly (probably there aren't many).

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

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


More information about the Patches mailing list