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.

Reply via email to