[issue5915] PEP 383 implementation

[Benjamin Peterson]

Benjamin Peterson <benjamin at python.org> added the comment:

The calls to bytes2str are not checked in posixmodule.c.


http://codereview.appspot.com/52095/diff/1/5
File Include/unicodeobject.h (right):

http://codereview.appspot.com/52095/diff/1/5#newcode1254
Line 1254: The function is intended to be used for paths and file names
only
If these are only meant to be used internally during bootstrap, should
they have the _Py prefix?

http://codereview.appspot.com/52095/diff/1/10
File Lib/test/test_pep383.py (right):

http://codereview.appspot.com/52095/diff/1/10#newcode1
Line 1: from test import support
I'm not sure this deserves a whole new file. Couldn't UtfbTest go in
test_codecs and FileTests go in test_os?

http://codereview.appspot.com/52095/diff/1/10#newcode2
Line 2: import unittest, shutil, os, sys
PEP 8 says these should be on separate lines.

http://codereview.appspot.com/52095/diff/1/10#newcode33
Line 33: if os.name != 'win32':
Isn't os.name "nt" on Windows?

Also, you could skip this whole class on Windows by decorating the class
with @unittest.skipIf(sys.platform.startswith("win")).

http://codereview.appspot.com/52095/diff/1/10#newcode47
Line 47: f = open(self.bdir + b"/" + fn, "w")
Looks like a good place for os.path.join.

http://codereview.appspot.com/52095/diff/1/13
File Modules/posixmodule.c (right):

http://codereview.appspot.com/52095/diff/1/13#newcode547
Line 547: else if(PyByteArray_Check(o)) {
There's a small white space problem here.

http://codereview.appspot.com/52095/diff/1/13#newcode548
Line 548: if (lock && o->ob_type->tp_as_buffer->bf_getbuffer(o, NULL, 0)
< 0)
I believe you want PyObject_GetBuffer here.

http://codereview.appspot.com/52095/diff/1/13#newcode596
Line 596: bytes2str(name, 0));
What if byte2str returns NULL?

http://codereview.appspot.com/52095/diff/1/2
File Python/codecs.c (right):

http://codereview.appspot.com/52095/diff/1/2#newcode852
Line 852: if (!res)
This leaks "object".

http://codereview.appspot.com/52095/diff/1/2#newcode895
Line 895: return NULL;
This leaks "object".

http://codereview.appspot.com/52095

----------

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue5915>
_______________________________________