classification
Title: PyHeapTypeObject fix
Type: Stage:
Components: None Versions: Python 3.0
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: gvanrossum Nosy List: georg.brandl, gvanrossum, loewis, theller
Priority: normal Keywords: patch

Created on 2007-07-11 19:46 by theller, last changed 2008-01-06 22:29 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
typeobject.diff theller, 2007-07-11 19:46
typeobject.diff theller, 2007-07-13 09:43 Version 2
Messages (9)
msg52844 - (view) Author: Thomas Heller (theller) * (Python committer) Date: 2007-07-11 19:46
This patch makes sure that the PyHeapTypeObject's ht_name member always contains a PyUnicode_Object.  Other code relies on this.
msg52845 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-07-11 21:15
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).
msg52846 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-07-11 21:31
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.
msg52847 - (view) Author: Thomas Heller (theller) * (Python committer) Date: 2007-07-13 09:43
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
msg52848 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-07-16 21:39
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.
msg55591 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2007-09-02 19:57
The source of make_type is in Parser/asdl_c.py.
msg57070 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2007-11-02 22:29
The patch's changes to typeobject.c are already in the PEP3137 branch
(but without the "this is buggy" comment), the others are not.
msg57073 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-11-02 22:46
And in the py3k branch as well (r57504).

I'll work on the others and on a fix for the "this is buggy" thing.
msg57076 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-11-02 23:08
Fix for all issues committed as revision 58815 (in the py3k branch).

The strlen() check is replaced by something saner.

Thanks for reminding me!
History
Date User Action Args
2008-01-06 22:29:45adminsetkeywords: - py3k
versions: Python 3.0
2007-11-02 23:08:01gvanrossumsetstatus: open -> closed
resolution: fixed
messages: + msg57076
2007-11-02 22:46:15gvanrossumsetmessages: + msg57073
2007-11-02 22:29:26georg.brandlsetnosy: + georg.brandl
messages: + msg57070
2007-09-02 19:57:55loewissetnosy: + loewis
messages: + msg55591
2007-08-30 00:22:52gvanrossumsetversions: + Python 3.0
2007-07-11 19:46:28thellercreate