review request for 7021582, use try-with-resources in jar/zip implementation and tests

Alan Bateman Alan.Bateman at oracle.com
Thu Feb 24 10:20:48 UTC 2011


Stuart Marks wrote:
> Hi Sherman, all,
>
> Here's a webrev to convert code in the jar/zip implementation files 
> and tests to use the new Java 7 try-with-resources construct.
>
> http://cr.openjdk.java.net/~smarks/reviews/7021582/webrev.0/
I looked through the src/** changes (I don't have time to go through the 
test changes just now).

All looks okay to me except for 
src/share/classes/com/sun/java/util/jar/pack/PropMap.java.

At L129 it uses getResourceAsStream and that will return null if the 
properties file doesn't exist causing props.load to throw NPE. I realize 
the original code will NPE too but maybe this should be fixed while you 
are in the area.

Also in this code it's not clear to me that propsLoaded is needed. I 
realize you're keeping existing behavior but it means that we get NPE if 
the properties file doesn't exist, throw RuntimeException if there is a 
problem reading it, and do nothing if close fails. I can't think of a 
case where close could fail here and maybe it would be better to just 
handle it in the same way as an error reading the properties.

-Alan.



More information about the core-libs-dev mailing list