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

Issue 13473020: Fix mangling of zlib API on 64 bit platforms. (Closed)

Created:
7 years, 8 months ago by gavinp
Modified:
7 years, 8 months ago
Reviewers:
brettw, agl, Ryan Sleevi
CC:
chromium-reviews
Visibility:
Public.

Description

Fix mangling of zlib API on 64 bit platforms. It seems https://codereview.chromium.org/8954009/ was a bit overzealous. On 64 bit platforms, zlib.h attempts to use the 64 bit API for all callers. But, we were not allowing that, making writing portable code difficult. This CL updates our manglings in zlib.h so that on a 64 bit platform the API defaults to the 64 bit version, as intended. This change is downstream of https://codereview.chromium.org/13564004/ and must land after it. R=rsleevi@chromium.org,agl@chromium.org, brettw@chromium.org BUG=None Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=193371

Patch Set 1 #

Patch Set 2 : rebase #

Patch Set 3 : rebase again #

Patch Set 4 : rebase on upstream #

Unified diffs Side-by-side diffs Delta from patch set Stats (+49 lines, -24 lines) Patch
M third_party/zlib/README.chromium View 1 chunk +1 line, -1 line 0 comments Download
M third_party/zlib/google.patch View 1 2 3 1 chunk +25 lines, -16 lines 0 comments Download
M third_party/zlib/zlib.h View 1 chunk +23 lines, -7 lines 0 comments Download

Messages

Total messages: 10 (0 generated)
gavinp
After this change, we can now use crc32_combine and adler32_combine safely.
7 years, 8 months ago (2013-04-08 11:31:21 UTC) #1
agl
lgtm If it compiles then LGTM.
7 years, 8 months ago (2013-04-08 14:32:05 UTC) #2
gavinp
brettw: can you give an OWNERS review as zlib has no OWNERS currently?
7 years, 8 months ago (2013-04-09 07:45:29 UTC) #3
gavinp
On 2013/04/09 07:45:29, gavinp wrote: > brettw: can you give an OWNERS review as zlib ...
7 years, 8 months ago (2013-04-09 07:47:08 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/gavinp@chromium.org/13473020/17001
7 years, 8 months ago (2013-04-09 17:32:34 UTC) #5
commit-bot: I haz the power
Retried try job too often on linux_chromeos for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chromeos&number=101245
7 years, 8 months ago (2013-04-09 19:27:26 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/gavinp@chromium.org/13473020/17001
7 years, 8 months ago (2013-04-10 09:32:38 UTC) #7
commit-bot: I haz the power
Retried try job too often on linux_chromeos for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chromeos&number=101688
7 years, 8 months ago (2013-04-10 11:14:10 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/gavinp@chromium.org/13473020/17001
7 years, 8 months ago (2013-04-10 11:15:06 UTC) #9
commit-bot: I haz the power
7 years, 8 months ago (2013-04-10 12:25:37 UTC) #10
Message was sent while issue was closed.
Change committed as 193371

Powered by Google App Engine
This is Rietveld 408576698