Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(45)

Issue 8873039: Add an API to unpack Zip files directly from and to file descriptors. (Closed)

Created:
9 years ago by Jorge Lucangeli Obes
Modified:
9 years ago
Reviewers:
satorux1
CC:
chromium-reviews, Paweł Hajdan Jr., Hironori Bono
Base URL:
http://git.chromium.org/chromium/src.git@master
Visibility:
Public.

Description

Add an API to unpack Zip files directly from and to file descriptors. BUG=None TEST=zip_reader_unittest.cc (compiles into 'unit_tests' binary) Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=114684

Patch Set 1 #

Total comments: 14

Patch Set 2 : Addressed satorux1 comments. Rebased. #

Patch Set 3 : Lint fixes. #

Total comments: 6

Patch Set 4 : Added FdWrapper class for testing, removed unused code. #

Patch Set 5 : Fixed lint errors. #

Total comments: 10

Patch Set 6 : Style fixes. #

Total comments: 8

Patch Set 7 : More style fixes. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+255 lines, -8 lines) Patch
M chrome/common/zip_internal.h View 1 1 chunk +5 lines, -0 lines 0 comments Download
M chrome/common/zip_internal.cc View 1 2 3 4 5 6 3 chunks +50 lines, -0 lines 0 comments Download
M chrome/common/zip_reader.h View 1 2 3 4 chunks +16 lines, -0 lines 0 comments Download
M chrome/common/zip_reader.cc View 1 2 2 chunks +67 lines, -8 lines 0 comments Download
M chrome/common/zip_reader_unittest.cc View 1 2 3 4 5 6 5 chunks +117 lines, -0 lines 0 comments Download

Messages

Total messages: 18 (0 generated)
Jorge Lucangeli Obes
Reworked Zip file handling patch that does not need file access by path.
9 years ago (2011-12-08 23:40:28 UTC) #1
satorux1
The code looks good. Style nits only: http://codereview.chromium.org/8873039/diff/1/chrome/common/zip_internal.cc File chrome/common/zip_internal.cc (right): http://codereview.chromium.org/8873039/diff/1/chrome/common/zip_internal.cc#newcode85 chrome/common/zip_internal.cc:85: unzFile OpenFdForUnzipping(const ...
9 years ago (2011-12-12 04:54:14 UTC) #2
Jorge Lucangeli Obes
Addressed comments http://codereview.chromium.org/8873039/diff/1/chrome/common/zip_internal.cc File chrome/common/zip_internal.cc (right): http://codereview.chromium.org/8873039/diff/1/chrome/common/zip_internal.cc#newcode85 chrome/common/zip_internal.cc:85: unzFile OpenFdForUnzipping(const int fd) { On 2011/12/12 ...
9 years ago (2011-12-12 23:34:35 UTC) #3
Jorge Lucangeli Obes
Addressed satorux1 comments and fixed lint checks.
9 years ago (2011-12-12 23:40:14 UTC) #4
satorux1
Please reply to comments by hitting 'Done' button from next time. That'll make it easier ...
9 years ago (2011-12-13 05:23:08 UTC) #5
satorux1
Jorge, hbono@ pointed out there was a GYP flag named "use_system_zlib" and we use the ...
9 years ago (2011-12-13 06:06:51 UTC) #6
satorux1
Good news. Looking at the changes in third_party/zlib, the patch just adds two functions without ...
9 years ago (2011-12-13 06:09:45 UTC) #7
Jorge Lucangeli Obes
On Mon, Dec 12, 2011 at 10:09 PM, <satorux@chromium.org> wrote: > Good news. Looking at ...
9 years ago (2011-12-13 17:45:35 UTC) #8
satorux1
FWIW, I take back my comment saying "The change will make it impossible to use ...
9 years ago (2011-12-13 23:14:07 UTC) #9
Jorge Lucangeli Obes
http://codereview.chromium.org/8873039/diff/10004/chrome/common/zip_reader.h File chrome/common/zip_reader.h (right): http://codereview.chromium.org/8873039/diff/10004/chrome/common/zip_reader.h#newcode159 chrome/common/zip_reader.h:159: bool ExtractCurrentEntryToBuffer(const char* buf, const int size); On 2011/12/13 ...
9 years ago (2011-12-14 23:17:55 UTC) #10
Jorge Lucangeli Obes
On 2011/12/13 23:14:07, satorux1 wrote: > FWIW, I take back my comment saying "The change ...
9 years ago (2011-12-14 23:18:25 UTC) #11
satorux1
It's great to see changes under third_party gone! The code looks good. Only style nits. ...
9 years ago (2011-12-15 00:05:35 UTC) #12
Jorge Lucangeli Obes
http://codereview.chromium.org/8873039/diff/20001/chrome/common/zip_internal.cc File chrome/common/zip_internal.cc (right): http://codereview.chromium.org/8873039/diff/20001/chrome/common/zip_internal.cc#newcode71 chrome/common/zip_internal.cc:71: void* fdopen_file_func(void* opaque, const char* filename, int mode) { ...
9 years ago (2011-12-15 00:19:00 UTC) #13
satorux1
http://codereview.chromium.org/8873039/diff/23001/chrome/common/zip_internal.cc File chrome/common/zip_internal.cc (right): http://codereview.chromium.org/8873039/diff/23001/chrome/common/zip_internal.cc#newcode71 chrome/common/zip_internal.cc:71: void* FdOpenFileFunc(void* opaque, const char* filename, int mode) { ...
9 years ago (2011-12-15 00:40:45 UTC) #14
Jorge Lucangeli Obes
http://codereview.chromium.org/8873039/diff/23001/chrome/common/zip_internal.cc File chrome/common/zip_internal.cc (right): http://codereview.chromium.org/8873039/diff/23001/chrome/common/zip_internal.cc#newcode71 chrome/common/zip_internal.cc:71: void* FdOpenFileFunc(void* opaque, const char* filename, int mode) { ...
9 years ago (2011-12-15 01:06:19 UTC) #15
satorux1
LGTM
9 years ago (2011-12-15 01:13:13 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jorgelo@chromium.org/8873039/24007
9 years ago (2011-12-15 18:34:32 UTC) #17
commit-bot: I haz the power
9 years ago (2011-12-15 19:47:41 UTC) #18
Change committed as 114684

Powered by Google App Engine
This is Rietveld 408576698