| msg70625 - (view) |
Author: Anand B Pillai (pythonhacker) * |
Date: 2008-08-02 10:15 |
| >>> import zlib >>> s='This is a string' >>> sc=zlib.compress(s) >>> sc bytearray(b'x\x9c\x0b\xc9\xc8,V\x00\xa2D\x85\xe2\x92\xa2\xcc\xbct\x00/\xc2\x05\xcd') >>> zlib.decompress(sc) bytearray(b'This is a string') >>> This is wrong behavior as compress functions should return byte ,not a bytearray. Same for decompress. |
|
|
| msg70659 - (view) |
Author: Anand B Pillai (pythonhacker) * |
Date: 2008-08-03 19:48 |
| Hi, I have a patch ready for this to be applied to zlibmodule.c. The patch is attached. I have tested it and it is working fine. |
|
|
| msg70669 - (view) |
Author: Alexandre Vassalotti (alexandre.vassalotti) *  |
Date: 2008-08-04 00:13 |
| Could you submit unified diff--i.e., with 'diff -u' or 'svn diff'? Also, could you add tests for this fix? |
|
|
| msg70683 - (view) |
Author: Anand B Pillai (pythonhacker) * |
Date: 2008-08-04 04:04 |
| Uploading svn diff for zlibmodule.c. Btw, how do I add unit tests for a fix ? You want me to create a simple test file and upload it here, or is there a standard procedure for this ? Please advise. |
|
|
| msg70685 - (view) |
Author: Anand B Pillai (pythonhacker) * |
Date: 2008-08-04 04:24 |
| Ok. I added two tests for checking the type of the returned object for zlib.compress and zlib.decompress in test_zlib.py in the py3k branch. Btw, my code uses assertEqual(type(...), bytes). Is this the proper way of type checking in py3k ? I could not see any formal type for "bytes" type in the types module. Attaching the svn diff for test_zlib.py . |
|
|
| msg70781 - (view) |
Author: Anand B Pillai (pythonhacker) * |
Date: 2008-08-06 11:16 |
| Any updates ? The py3k list is also very silent since the week-end...Thanks! |
|
|
| msg70933 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2008-08-09 16:30 |
| > Any updates ? The py3k list is also very silent since the > week-end...Thanks! Your two patches look good, I suppose either Alexandre or I will commit them soon. You shouldn't to worry when you don't get a reply immediately, people react simply when they have time to do so. And as for the mailing-list activity, we are in the beginning of August which I guess implies many people are on holidays. |
|
|
| msg71078 - (view) |
Author: Anand B Pillai (pythonhacker) * |
Date: 2008-08-13 09:33 |
| Thanks. Will this make into beta3 ? |
|
|
| msg72483 - (view) |
Author: Gregory P. Smith (gregory.p.smith) * |
Date: 2008-09-04 05:15 |
| this is a very simple patch and makes sense to me. marking it a release blocker and asking about it on the mailing list. if anyone disagrees with this one please speak up. in general IO input functions elsewhere return bytes(). zlib should as well. this fixes that. |
|
|
| msg72491 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2008-09-04 11:13 |
| +1 for committing. |
|
|
| msg72494 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * |
Date: 2008-09-04 11:29 |
| Two remarks: 1. Some functions in Modules/zipimport.c (get_data) will now receive PyBytes, but zipimporter_get_source handle them as PyByteArray. Does zipimport still work at all with this patch? 2. There are other places where PyString_FromString was (incorrectly IMO) replaced with PyByteArray_FromString: - Modules/_dbmmodule.c - Modules/mmapmodule.c - Modules/ossaudiodev.c - Modules/zipimport.c (as noted above) - PC/winreg.c - Python/marshal.c |
|
|
| msg72559 - (view) |
Author: Gregory P. Smith (gregory.p.smith) * |
Date: 2008-09-05 00:07 |
| Correct, zipimport required fixing in order for this to work. The newly attached zlib-and-zipimport-gps01 patch. review at http://codereview.appspot.com/4454 I haven't had a chance to look at the other modules Amaury mentioned but on general principal I agree, they should return bytes. |
|
|
| msg72571 - (view) |
Author: Anand B Pillai (pythonhacker) * |
Date: 2008-09-05 05:50 |
| Does py3k list/barry have this bug in their radar for rc2 ? |
|
|
| msg72572 - (view) |
Author: Anand B Pillai (pythonhacker) * |
Date: 2008-09-05 05:56 |
| On Thu, Sep 4, 2008 at 4:59 PM, Amaury Forgeot d'Arc <report@bugs.python.org> wrote: > > Amaury Forgeot d'Arc <amauryfa@gmail.com> added the comment: > > Two remarks: > 1. Some functions in Modules/zipimport.c (get_data) will now receive > PyBytes, but zipimporter_get_source handle them as PyByteArray. Does > zipimport still work at all with this patch? > > 2. There are other places where PyString_FromString was (incorrectly > IMO) replaced with PyByteArray_FromString: > - Modules/_dbmmodule.c > - Modules/mmapmodule.c > - Modules/ossaudiodev.c > - Modules/zipimport.c (as noted above) > - PC/winreg.c > - Python/marshal.c Hmmm...but AFAIK zlib changes only affect zipimport directly. I wonder whether these other instances have bugs reported and are being tracked for the release. |
|
|
| msg72580 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2008-09-05 10:25 |
| Patch looks good to me (I've only looked at the patch - not the other possible misuses of PyByteArray_ that Amaury noted) |
|
|
| msg72653 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2008-09-06 00:19 |
| We must definitely clean up other uses of bytearray in the extension modules - see #3790 for an example. |
|
|
| msg72686 - (view) |
Author: Gregory P. Smith (gregory.p.smith) * |
Date: 2008-09-06 20:15 |
| fixed in r66266 along with #3790. leaving this open and assigned to me while i investigate the other uses of ByteArray in the Modules/ directory. IMHO, its fine if we fix any remaining bytearray uses up for rc2. |
|
|
| msg72694 - (view) |
Author: Gregory P. Smith (gregory.p.smith) * |
Date: 2008-09-06 20:50 |
| issue #3797 has been opened to track the other files mentioned. |
|
|
| msg72764 - (view) |
Author: Anand B Pillai (pythonhacker) * |
Date: 2008-09-08 05:43 |
| Hi Gregory, Let me know if I can help out some way in testing the #3797 patches on various platforms. Regards, --Anand |
|
|