Hi Brian,

Aside: I'm confused about the relationship of this bug and JDK-6445180 - are they not duplicates? Seems to me this one should have been closed as a dup when it was reported.

On 5/10/2013 6:58 AM, Brian Burkhalter wrote:
On Oct 3, 2013, at 5:38 PM, Brian Burkhalter wrote:

On Oct 3, 2013, at 5:35 PM, Alan Bateman wrote:

On 03/10/2013 16:10, Brian Burkhalter wrote:
Please review and comment at your convenience.

Issue:  https://bugs.openjdk.java.net/browse/JDK-7179567
Webrev: http://cr.openjdk.java.net/~bpb/7179567/

An updated webrev which I hope adequately addresses the expressed concerns may 
be found at:

http://cr.openjdk.java.net/~bpb/7179567.2/

URLClassLoader.java: please delete the commented out line:

+         //Objects.requireNonNull(urls);

SecureClassLoader.java:

186 * @throws SecurityException if the {@code ClassLoader} instance is not
 187      * initialized.

should read "this classloader instance" as it refers to the current instance. Also this may be bringing the spec into line with the implementation but "initialization" here is purely an implementation detail not part of the specification for this class so it should not be mentioned explicitly - and this change still needs a CCC (which might help determine exactly what should be said here - I'm unclear how you can try to call getPermissions if this is uninitialized as it would need 'this' to escape from the constructor - which perhaps it can do via a third-party security manager?).

NoCallStackClassLoader.java: the comment is far too verbose, we don't write such explanatory kinds of comments for this kind of thing (else the code would be overrun with commentary!).

Aside: if you are a JDK Author you don't need a Contributed-by line in the commit comments.

David
-----


Will you be adding tests for these cases to the webrev?

As needed once the concept in general is accepted.

The foregoing webrev includes a test of the affected public methods.

Thanks,

Brian

Reply via email to