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