RFR [9] 8139706: JarFile.getBytes could use InputStream.readNBytes

Claes Redestad claes.redestad at oracle.com
Sat Oct 17 13:11:36 UTC 2015


On 2015-10-17 01:37, Xueming Shen wrote:
>
> On 10/16/2015 3:20 PM, Claes Redestad wrote:
>>
>> On 2015-10-16 18:48, Xueming Shen wrote:
>>> looks fine. though it might be better to simply check len != 
>>> b.length, as it's still possible that reallAllBytes
>>> returns a byte[] with length > len, if the entry is compressed, and 
>>> the "length" in entry does not really
>>> match the length of the bytes from the inflater.
>>>
>>> If you go with len != b.length, you can pull two checks out and 
>>> consolidate them at the end before return.
>>
>> We'll also need to check that len != -1 in that extracted check, but 
>> I guess it's still cleaner:
>>
>> http://cr.openjdk.java.net/~redestad/8139706/webrev.04
>>
>> /Claes
>
> it looks good.
>
> btw,  I don't remember where did this len != -1 come from. The is an 
> meta-inf entry from the
> ZipFile, which should come from the cen table, which means it should 
> always have the correct
> size set  in. Not like the entry from zis, which is from loc table and 
> in case of a compressed
> entry, the size/csize might be -1 (as they are unknown when you write 
> out the loc entry...).
> I might be wrong without digging into the code and history a little 
> more deep, and it really
> does not matter too much. So the change is good for me.
>
> -sherman

Maybe you're right, but you're also right that getting rid of an extra 
check or two won't matter much here.

I'll go ahead and push the latest patch later, unless someone objects.

Thanks everyone for reviewing and catching issues!

/Claes



More information about the core-libs-dev mailing list