JDK9 RFR of JDK-8029646: [pack200] should support the new zip64 format.

Kumar Srinivasan kumar.x.srinivasan at oracle.com
Tue Jan 21 16:11:19 UTC 2014


Alex,

Thanks for adding the test, few comments:

PackTestZip64.java:

1. compareTwoFile, I would read the entire file into 
ByteArrayInputStream, compare
the total first ie. fast fail, but what you have is also ok.

2.  generateLargeJar: I would replace lines 126,127 and 128 using 
for-each loop,
by using Collections.list<Enumeration> e); will make it more compact.
Though you are usign Try-With-Resources on the input jar, how about

3. Though the test might not take too long on your system, I am 
concerned it may
take too long on our Jprt system.

Otherwise looks good.

Kumar


On 1/16/2014 8:21 AM, Alexander Zuev wrote:
> Sherman, Kumar,
>
>   i have fixed the glitches you have found and changed the test so it 
> creates a new jar
> based on the golden.jar content (to have a reasonable set of various 
> data to start with),
> then adding to it 65536 empty files to test how we cope with such a 
> huge jars.
>
>   The testing time is less than a 30 seconds on my machine (not the 
> top-of-the-line) so i
> decided not to bother with conditional testing, lets test our behavior 
> every time in full.
>
>   The updated webrev can be found at 
> http://cr.openjdk.java.net/~kizune/8029646/webrev.02
>
> /Alex
>
> On 1/15/14 21:34, Xueming Shen wrote:
>> On 1/15/14 7:01 AM, Alexander Zuev wrote:
>>> Hello,
>>>
>>>   the new webrev with all the typos and comments fixed can be found 
>>> at http://cr.openjdk.java.net/~kizune/8029646/webrev.01/
>>>
>>> /Alex
>>
>> (1)  jarmagic can be just a static constant somewhere or a stack 
>> variable. not big deal though.
>> (2)  the test only "tests" EOF for s. there is possibility that the 
>> newly created has more extra
>>       bytes at the end...though in theory this should not happen, it 
>> might be better just add an
>>       extra line to check the sizes of two first first?
>> (3)  the rest of the change looks good, but I agreed with Kumar that 
>> you may need to add a
>>       regression test for  a jar file with > 64k entries. otherwise 
>> the code for zip64 end  is not
>>       being tested. the code looks fine, but I would trust a 
>> regression test more than my eyes:-)
>>
>> Thanks!
>> -Sherman
>




More information about the core-libs-dev mailing list