[issue5734] Fix BufferedRWPair

Antoine Pitrou report at bugs.python.org
Fri Apr 17 23:11:18 CEST 2009


Antoine Pitrou <pitrou at free.fr> added the comment:

Reviewers: report_bugs.python.org,

Message:
Here are some comments. In general, I think the changes to _pyio.py are
unwarranted (even though I dislike the naming of _checkReadable and
_checkWritable).

http://codereview.appspot.com/40126/diff/1/2
File Lib/_pyio.py (left):

http://codereview.appspot.com/40126/diff/1/2#oldcode370
Line 370: def _checkReadable(self, msg=None):
Not sure why you're removing it. Currently it's used in Lib/socket.py.

http://codereview.appspot.com/40126/diff/1/2#oldcode384
Line 384: def _checkWritable(self, msg=None):
Same question as for _checkReadable().

http://codereview.appspot.com/40126/diff/1/3
File Lib/test/test_io.py (right):

http://codereview.appspot.com/40126/diff/1/3#newcode1121
Line 1121: self.assertTrue(pair.readable)
This is probably `pair.readable()` and not `pair.readable`.

http://codereview.appspot.com/40126/diff/1/3#newcode1125
Line 1125: self.assertTrue(pair.writable)
Same comment as for readable above.

http://codereview.appspot.com/40126/diff/1/3#newcode1126
Line 1126:
There should probably be a test for seekable() as well.

http://codereview.appspot.com/40126/diff/1/4
File Modules/_io/bufferedio.c (right):

http://codereview.appspot.com/40126/diff/1/4#newcode1876
Line 1876: Py_DECREF(self->reader);
You must use Py_CLEAR so that there is no double free when calling
BufferedRWPair_dealloc().

Please review this at http://codereview.appspot.com/40126

Affected files:
   M     Lib/_pyio.py
   M     Lib/test/test_io.py
   M     Modules/_io/bufferedio.c

----------
title: BufferedRWPair broken -> Fix BufferedRWPair

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


More information about the Python-bugs-list mailing list