review request for 7021582, use try-with-resources in jar/zip implementation and tests
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
>> At L129 it uses getResourceAsStream and that will return null if the
>> 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
logic to fail in the early development days to ensure this is always
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
unless there was a build issue.
> Advice appreciated.
More information about the core-libs-dev