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