Xueming.Shen at Sun.COM
Mon Aug 24 14:21:25 PDT 2009
Martin Buchholz wrote:
> On Mon, Aug 24, 2009 at 11:16, Xueming Shen<Xueming.Shen at sun.com> wrote:
>> Martin Buchholz wrote:
>>> mit if you insist!
>>> Oh, I see your point. Are you saying this "local change" is not
>>> necessary since it never gets
>>> No, more than that - the code is defining a derived type u4 from
>>> primitive types - it should not be using other derived types.
>>> BTW, u4 is of size *exactly* 4 bytes, while uLong I think is of size
>>> *at least* 4 bytes, so these are slightly semantically different.
>> If you consider we have "already" made the change to make sure uLong is
>> 32-bit, then
>> this should no longer be a problem, yes, semantically we should simply use
>> "unsigned long"
>> here, but it does not make any difference. I made this change with the
>> assumption of that LP64
>>> Looking again at zlib's types, I see that
>>> uLong is unconditionally typedef'ed to "unsigned long"
>>> so in an unmodified distribution they can be used interchangeably
>>> (although a little unclean - why have a typedef in the first place?)
>>> zlib definitely claims to support 64-bit platforms - see FAQ. It should
>>> Just Work to let uLong be always unsigned long,
>>> even though that might be "too large"
>>> for the data being provided on LP64 systems.
>> The SCCS history shows there were some "check in, check out then check in
>> again" history regarding
>> this "32-bit or 64-bit" modification, due to some hotspot failure bug. This
>> is the most reasons I decided
>> to go with the "working version".
> Working is always better than Not Working.
> You have my blessing to commit your current changes.
> But if I was RE, I would probably do more work on this in the
> directions I outlined
> - reduce JDK-specific diffs - try to use unmodified zlib
> - #include <unistd.h> in zconf.h
> - resolve lingering doubts about 64-bit file offset support on 32-bit
> Unix systems.
I believe 64-bit file offset support has been done in zip_util.c/h
already in 7 and the latest 6 (don't be
surprised if there is yet another corner case, but I'm pretty much sure
we are safe on this one).
The current implementation interfaces with the zlib via in/out byte
buffer. I don't there is anything 64-bit file
offset issue to "resolve" inside zlib1.2.3.
I was reminded that the b72 (tonight deadline) is the last build of m5,
so to play safe I will hold the change
for m6's first build. Will look into your "reduce JDK-specific diffs",
please commit yourself for the review:-)
"I don't see why the rest of the jdk has to be changed,
or why a completely unmodified zlib cannot be used"
Current CRC32 and pack/zip.c expect a 32bit unsigned uint from zlib
crc32(). If we keep the new crc32 as
"unsigned long", which is 64-bit on 64-bit platform, we have to do
something such as cast and testing.
More information about the core-libs-dev