On 04/10/2013 21:58, Brian Burkhalter wrote:
:
An updated webrev which I hope adequately addresses the expressed concerns may
be found at:
http://cr.openjdk.java.net/~bpb/7179567.2/
This looks much better.
If I read the code correctly then the long standing behavior of
URLClassLoader.getPermissions(null) was to throw a NPE, now it is
changed to return the empty collection in order to match the super type
(SecureClassLoader). I can't think of anything that would break so this
is probably okay, just maybe a bit surprising.
My previous comment on @throws vs. @exception was just to keep things
consistent as this is old code and would look odd for a method to use
both. Our preference is to switch to @throws whenever the opportunity
arises. The comment was also on the alignment/style. Take
URLCLassLoader(URL[], ClassLoader) for example, the existing
SecurityException has its description aligned under SecurityException
whereas the new NullPointerException wraps around. So my overall comment
here was to avoid mixing styles (it doesn't matter too much which style
is used, just keep it consistent for future maintainers).
In the test then it might be better to change the @summary line to only
include the second paragraph (the bug summary is not interesting,
especially when it references a test that is not in OpenJDK).
-Alan.
