Martin Buchholz wrote:
On Mon, Aug 24, 2009 at 11:16, Xueming Shen<xueming.s...@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
preprocessed?
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
change.
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:-)
Re:
"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.
Sherman
