Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(22137)

#15955: gzip, bz2, lzma: add option to limit output size

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 3 months ago by lists
Modified:
4 years, 4 months ago
Reviewers:
nadeem.vawda, nikolaus, vadmium+py, pitrou
CC:
AntoinePitrou, haypo, christian.heimes, nadeem.vawda, eric.araujo, Arfrever, Nikratio, devnull_psf.upfronthosting.co.za, Martin Panter, storchaka
Visibility:
Public.

Patch Set 1 #

Total comments: 12

Patch Set 2 #

Total comments: 14

Patch Set 3 #

Total comments: 6

Patch Set 4 #

Patch Set 5 #

Patch Set 6 #

Total comments: 15

Patch Set 7 #

Patch Set 8 #

Total comments: 12

Patch Set 9 #

Total comments: 1

Patch Set 10 #

Total comments: 1

Patch Set 11 #

Patch Set 12 #

Total comments: 1

Patch Set 13 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/library/bz2.rst View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +30 lines, -6 lines 0 comments Download
Lib/test/test_bz2.py View 1 2 3 4 5 6 7 8 9 10 11 12 3 chunks +103 lines, -0 lines 0 comments Download
Modules/_bz2module.c View 1 2 3 4 5 6 7 8 9 10 11 12 10 chunks +203 lines, -60 lines 0 comments Download
Modules/clinic/_bz2module.c.h View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +23 lines, -14 lines 0 comments Download

Messages

Total messages: 17
nadeem.vawda
Thanks for working on this patch. It will need a fair bit of reworking before ...
5 years, 2 months ago #1
Nikratio
Hi Nadeem, On 2014/04/06 15:25:06, nadeem.vawda wrote: > Thanks for working on this patch. It ...
5 years, 2 months ago #2
Nikratio
http://bugs.python.org/review/15955/diff/11435/Modules/_lzmamodule.c File Modules/_lzmamodule.c (right): http://bugs.python.org/review/15955/diff/11435/Modules/_lzmamodule.c#newcode69 Modules/_lzmamodule.c:69: PyObject *needs_input; On 2014/04/06 15:25:06, nadeem.vawda wrote: > According ...
5 years, 2 months ago #3
nadeem.vawda
On 2014/04/08 02:54:09, Nikratio wrote: > No worries, I don't get discouraged that easily. I'll ...
5 years, 2 months ago #4
Nikratio
On 2014/04/09 23:41:09, nadeem.vawda wrote: > > I actually meant to write the patch in ...
5 years, 2 months ago #5
Martin Panter
Looks fairly sensible to me. Just a problem handling realloc error, some funny test code, ...
4 years, 5 months ago #6
Martin Panter
https://bugs.python.org/review/15955/diff/11547/Modules/_lzmamodule.c File Modules/_lzmamodule.c (right): https://bugs.python.org/review/15955/diff/11547/Modules/_lzmamodule.c#newcode987 Modules/_lzmamodule.c:987: d->input_buffer_size += len - avail_now; On 2015/01/09 04:35:20, vadmium ...
4 years, 5 months ago #7
AntoinePitrou
Thanks for the patch. Here is are some review comments. http://bugs.python.org/review/15955/diff/13599/Lib/test/test_lzma.py File Lib/test/test_lzma.py (right): http://bugs.python.org/review/15955/diff/13599/Lib/test/test_lzma.py#newcode928 ...
4 years, 5 months ago #8
Martin Panter
http://bugs.python.org/review/15955/diff/13599/Lib/test/test_lzma.py File Lib/test/test_lzma.py (right): http://bugs.python.org/review/15955/diff/13599/Lib/test/test_lzma.py#newcode928 Lib/test/test_lzma.py:928: def test_decompress_limited(self): On 2015/01/16 08:59:08, AntoinePitrou wrote: > On ...
4 years, 5 months ago #9
AntoinePitrou
http://bugs.python.org/review/15955/diff/13984/Lib/test/test_bz2.py File Lib/test/test_bz2.py (right): http://bugs.python.org/review/15955/diff/13984/Lib/test/test_bz2.py#newcode744 Lib/test/test_bz2.py:744: self.assertLessEqual(len(out[-1]), max_length) Why is it LessEqual here but Equal ...
4 years, 4 months ago #10
Martin Panter
http://bugs.python.org/review/15955/diff/13984/Modules/_bz2module.c File Modules/_bz2module.c (right): http://bugs.python.org/review/15955/diff/13984/Modules/_bz2module.c#newcode442 Modules/_bz2module.c:442: UINT_MAX); On 2015/02/21 19:18:46, AntoinePitrou wrote: > `PyBytes_GET_SIZE(result) - ...
4 years, 4 months ago #11
Nikratio
http://bugs.python.org/review/15955/diff/13984/Lib/test/test_bz2.py File Lib/test/test_bz2.py (right): http://bugs.python.org/review/15955/diff/13984/Lib/test/test_bz2.py#newcode744 Lib/test/test_bz2.py:744: self.assertLessEqual(len(out[-1]), max_length) On 2015/02/21 19:18:46, AntoinePitrou wrote: > Why ...
4 years, 4 months ago #12
Nikratio
http://bugs.python.org/review/15955/diff/13984/Modules/_bz2module.c File Modules/_bz2module.c (right): http://bugs.python.org/review/15955/diff/13984/Modules/_bz2module.c#newcode442 Modules/_bz2module.c:442: UINT_MAX); On 2015/02/22 20:58:29, Nikratio wrote: > On 2015/02/21 ...
4 years, 4 months ago #13
Martin Panter
Documentation in 2nd patch looks okay to me. https://bugs.python.org/review/15955/diff/13984/Modules/_bz2module.c File Modules/_bz2module.c (right): https://bugs.python.org/review/15955/diff/13984/Modules/_bz2module.c#newcode442 Modules/_bz2module.c:442: UINT_MAX); ...
4 years, 4 months ago #14
AntoinePitrou
https://bugs.python.org/review/15955/diff/13984/Modules/_bz2module.c File Modules/_bz2module.c (right): https://bugs.python.org/review/15955/diff/13984/Modules/_bz2module.c#newcode442 Modules/_bz2module.c:442: UINT_MAX); I agree that casting to size_t sounds safest. ...
4 years, 4 months ago #15
Martin Panter
http://bugs.python.org/review/15955/diff/13998/Modules/_bz2module.c File Modules/_bz2module.c (right): http://bugs.python.org/review/15955/diff/13998/Modules/_bz2module.c#newcode544 Modules/_bz2module.c:544: Py_CLEAR(d->unused_data); This line has too much indentation. Have a ...
4 years, 4 months ago #16
Martin Panter
4 years, 4 months ago #17
Indentation in rev6 looks good! Sorry to say I found another problem that snuck
in somehow:

https://bugs.python.org/review/15955/diff/14003/Modules/_bz2module.c
File Modules/_bz2module.c (right):

https://bugs.python.org/review/15955/diff/14003/Modules/_bz2module.c#newcode449
Modules/_bz2module.c:449: bzs->avail_in = (unsigned
int)Py_MIN(d->bzs_avail_in_real, UINT_MAX);
These two avail_in lines have been dupliated in the rev6 patch
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+