On Wed, 27 Mar 2024 16:03:09 GMT, Liam Miller-Cushon <cus...@openjdk.org> wrote:
>> src/java.base/share/native/libjli/manifest_info.h line 59: >> >>> 57: #define ZIP64_EXTID 1 // Extra field Zip64 header ID >>> 58: >>> 59: #define ZIP64_EXTMAXLEN 36 // Maximum Zip64 extra field length >> >> The fields described in APPNOTE-6.3.9.TXT 4.5.3 are total 32 bytes. Any >> other additional fields in the Zip64 extended information? >> >> >> Value Size Description >> ----- ---- ----------- >> (ZIP64) 0x0001 2 bytes Tag for this "extra" block type >> Size 2 bytes Size of this "extra" block >> Original >> Size 8 bytes Original uncompressed file size >> Compressed >> Size 8 bytes Size of compressed data >> Relative Header >> Offset 8 bytes Offset of local header record >> Disk Start >> Number 4 bytes Number of the disk on which >> this file starts > > Thanks for the catch, I had missed that the disk start number is 4 bytes and > not 8. I pushed a commit. I also removed some unused references to the disk > number, which is only being used to validate the size of the zip64 extended > info. Ok. Could you please also fix the above `36`? Please change the start of `//` (comment) to be aligned with the earlier comment lines. >> src/java.base/share/native/libjli/manifest_info.h line 146: >> >>> 144: * Macros for getting Extensible Data Fields >>> 145: */ >>> 146: #define ZIPEXT_HDR(b) SH(b, 0) /* Header ID */ >> >> How about naming the macros as ZIP64EXT_HDR and ZIP64EXT_SIZ? > > My thinking was that all extensible data fields start with a header with an > id and a length, and these macros are used to iterate through the extra > fields to find the zip64 extended information extra field, so these macros > aren't zip64-specific. Seem reasonable, thanks. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18479#discussion_r1544719690 PR Review Comment: https://git.openjdk.org/jdk/pull/18479#discussion_r1544718064