zlib1.2.3

Martin Buchholz martinrb at google.com
Mon Aug 24 18:52:45 UTC 2009


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
>>    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.

Martin

> Seems like I'm not going to catch tonight's deadline:-), maybe we can try
> some experiments.
>
>>
>> ----
>>
>> You should probably update zconf.h to include unistd.h
>> as if you had run configure, at least on Unix.
>>  318 #if 0           /* HAVE_UNISTD_H -- this line is updated by
>> ./configure */
>>
>>  319 #  include <sys/types.h> /* for off_t */
>>  320 #  include <unistd.h>    /* for SEEK_* and off_t */
>>  It looks like you won't get large file support on 32-bit systems
>> without struggling with configury stuff.  Or maybe we never use
>> zlib in the JDK to directly access files via seek and off_t?
>>
> No, the zlib in our code never accesses the file directly. File access is
> done in our own zipfile code..
>



More information about the core-libs-dev mailing list