This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author alexandre.vassalotti
Recipients alexandre.vassalotti, christian.heimes, gvanrossum
Date 2007-10-13.22:10:05
SpamBayes Score 0.0016969516
Marked as misclassified No
Message-id <1192313406.27.0.0583048162244.issue1272@psf.upfronthosting.co.za>
In-reply-to
Content
I found a few problems in your patch. In PyCode_New() the type check
for the filename argument is incorrect:

--- Objects/codeobject.c	(revision 58412)
+++ Objects/codeobject.c	(working copy)
@@ -59,7 +59,7 @@
 	    freevars == NULL || !PyTuple_Check(freevars) ||
 	    cellvars == NULL || !PyTuple_Check(cellvars) ||
 	    name == NULL || (!PyString_Check(name) && !PyUnicode_Check(name)) ||
-	    filename == NULL || !PyString_Check(filename) ||
+	    filename == NULL || (!PyString_Check(name) &&
!PyUnicode_Check(name)) ||
 	    lnotab == NULL || !PyString_Check(lnotab) ||
 	    !PyObject_CheckReadBuffer(code)) {
 		PyErr_BadInternalCall();

@@ -260,6 +267,8 @@
 		ourcellvars = PyTuple_New(0);
 	if (ourcellvars == NULL)
 		goto cleanup;
+    filename = PyUnicode_DecodeFSDefault(PyString_AS_STRING(filename),
+                                         0, NULL);


The following is unnecessary and will cause a reference leak:


@@ -260,6 +267,8 @@
 		ourcellvars = PyTuple_New(0);
 	if (ourcellvars == NULL)
 		goto cleanup;
+    filename = PyUnicode_DecodeFSDefault(PyString_AS_STRING(filename),
+                                         0, NULL);
 
 	co = (PyObject *)PyCode_New(argcount, kwonlyargcount,
 				    nlocals, stacksize, flags,


I think the interface of PyUnicode_DecodeFSDefault() could be improved
a bit. The function doesn't use the last argument 'errors', so why is
there? I am not sure if it is useful to keep second argument,
'length', either. So, I believe the function prototype should be
changed to:

    PyObject *PyUnicode_Decode_FSDefault(const char *s);

Another thing that I am not sure about is whether it is correct to
consider ISO-8859-15 the same thing as Latin-1.

Overall, the patch looks good to me and doesn't cause any test to
fail. I attached an updated patch with the above issues fixed.

Thank you, Christian, for the patch. :)
History
Date User Action Args
2007-10-13 22:10:06alexandre.vassalottisetspambayes_score: 0.00169695 -> 0.0016969516
recipients: + alexandre.vassalotti, gvanrossum, christian.heimes
2007-10-13 22:10:06alexandre.vassalottisetspambayes_score: 0.00169695 -> 0.00169695
messageid: <1192313406.27.0.0583048162244.issue1272@psf.upfronthosting.co.za>
2007-10-13 22:10:06alexandre.vassalottilinkissue1272 messages
2007-10-13 22:10:05alexandre.vassalotticreate