Review Request: 7193339 Prepare system classes be defined by a non-null module loader

[Paul Sandoz]

Hi Mandy,

--- old/src/share/classes/com/sun/jmx/remote/internal/IIOPHelper.java	Thu Aug 23 12:29:01 2012
+++ new/src/share/classes/com/sun/jmx/remote/internal/IIOPHelper.java	Thu Aug 23 12:29:00 2012
@@ -52,7 +52,7 @@
         AccessController.doPrivileged(new PrivilegedAction<IIOPProxy>() {
             public IIOPProxy run() {
                 try {
-                    Class<?> c = Class.forName(IMPL_CLASS, true, null);
+                    Class<?> c = Class.forName(IMPL_CLASS);


Why not:

  Class<?> c = Class.forName(IMPL_CLASS, true, IIOPHelper.class.getClassLoader());

to avoid traversing up the call stack to obtain the calling class.

Paul.

On Aug 23, 2012, at 9:33 PM, Mandy Chung <mandy.chung at oracle.com> wrote:

> On 8/23/2012 11:58 AM, Alan Bateman wrote:
>> On 23/08/2012 18:43, Mandy Chung wrote:
>>> This change is to bring the jdk modularization changes from jigsaw repo [1]
>>> to jdk8.  This allows the jdk modularization changes to be exposed for
>>> testing as early as possible and minimize the amount of changes carried
>>> in the jigsaw repo that helps sync up jigsaw with jdk8/jdk9.
>>> 
>>> Webrev at:
>>> http://cr.openjdk.java.net/~mchung/jdk8/webrevs/7193339/webrev.00/
>>> 
>> This looks good to me and it's good to have these changes in jdk8.
>> 
>> One suggestion for ReflectUtil is to add a private static boolean isAncestor method as I think that would make the checks in needsPackageAccessCheck easier to read. Also in ClassLoader you could use just use needsClassLoaderPermissionCheck(from,to).
>> 
> 
> Done.  This is a good cleanup I considered to do too but missed in the previous webrev.
> 
> http://cr.openjdk.java.net/~mchung/jdk8/webrevs/7193339/webrev.01/
> 
> Thanks for the review.
> Mandy