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

Reply via email to