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

Kumar Srinivasan kumar.x.srinivasan at oracle.com
Fri Feb 25 01:17:26 UTC 2011


All the changes look good, the regression test
  jdk/test/tools/pack200 and jdk/test/tools/jar
must be run, as jprt does not run these by default,
also suggest a full control build using jdk, deploy and install.

>> 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.
>
> Now that I'm looking at this more closely, what should happen if the 
> properties file doesn't exist? This is in a static initializer and 
> it's attempting to load "intrinsic.properties" which should always be 
> present -- if it's not present, the system was constructed improperly.

Yes intrinsic.properties *must* be present in the system, I think we 
added the
logic to fail in the early development days to ensure this is always 
present,
and is specifically needed for jcov builds required by SQE.

>
> So, if intrinsic.properties isn't found, what should happen? Ignore 
> this (seems like a bad idea); issue warning message (how?); throw 
> something like FileNotFoundException? Well, static initializers can't 
> throw checked exceptions, so maybe throw a RuntimeException with a 
> suitable message?

RE will be fine here, as you have already detected this condition should 
never happen
unless there was a build issue.

Thanks
Kumar

>
> Advice appreciated.
>
> s'marks




More information about the core-libs-dev mailing list