RFR: 8163798: Create a JarFile versionedStream method

Steve Drach steve.drach at oracle.com
Mon Sep 12 21:17:33 UTC 2016


Here’s a new webrev addressing Paul’s additional concerns

http://cr.openjdk.java.net/~sdrach/8163798/webrev.04/ <http://cr.openjdk.java.net/~sdrach/8163798/webrev.04/>


> Ok, better. Now there is no need to create an instance of VersionedStream, all methods can be static.

Done

> 
> I wonder if you can combine the checks for the index:
> 
>   64             int index = name.indexOf('/', META_INF_VERSIONS_LEN);
>   65             if (index == -1) {

Removed that test, let Integer.parseint handle it

> 
>   77             if (++index < name.length()) {
> 
> e.g. no slash, or found at end (not just malformed since we want to skip version directory entries if present?)
> 
>   if (index == -1 || index == name.length() - 1)
>    
> 
>>> 
>>> Also can getJarEntry ever return null?
>> 
>> yes, that’s why it’s filtered out
>> 
> 
> Under what circumstances in this case can it return null? since you have iterated over the existing entries and reduced to a distinct subset.

Right, that was before I put the “fix” in JarFile at line 561.  I removed the null check

> 
> Paul.
> 
>>> 
>>> Claes makes a good point regarding performance. I would suggest getting this functional and tested then tweaking for performance.
>>> 
>>> Paul.
> 



More information about the core-libs-dev mailing list