Hi Mandy,

On 4/06/2016 4:47 PM, Mandy Chung wrote:
Webrev:
  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8136930/webrev.00/

-modulepath, -addmods, -limitmods, -XaddExports, -XaddReads, -Xpatch are java 
launcher options in the current implementation.  Custom launchers will have to 
use -D to set some system properties to configure module system.  Different 
ways to configure module system is confusing and not friendly for environments 
using both java launcher and custom launchers.

This patch pushes the handling of the module options into the VM.  That will 
avoid the confusion between launcher and VM options and avoids needing to use 
system properties.  All launcher implementations can configure the module 
system via JNI Invocation API setting these options in a unified way.  The 
options and syntax remain the same as specified in JEP 261.

For the non-repeating options, like the other VM options, the last one wins.  
The current implementation communicates the options to the module system 
through system properties, as a private interface, and these system properties 
will be removed once they are read during the module system initialization.  
These system properties are reserved as private interface and they will be 
ignored if they are set via -D in the command line. Harold implements the 
hotspot change and can explain further details.

This patch will impact existing tests and scripts that set the system properties for 
example to break encapsulation in the command line e.g. 
-Djdk.launcher.addexports.<N>.  They will need to be updated to replace the use 
of -D with the appropriate module option e.g. -XaddExports.  Since they are new 
options in JDK 9, use -XX:+IgnoreUnrecognizedVMOptions if they need to be ignored by 
earlier releases.

Taking an initial look at the VM changes ...

I may be missing the full context but you seem to be checking only that some prefix matches the expected property format, not that the entire value is valid eg. if passed jdk.module.patch.1junk it will match

You use NEW_C_HEAP_ARRAY in places but that will abort the VM on failure, whereas you return error codes for other failure modes. For consistency use NEW_C_HEAP_ARRAY_RETURN_NULL and return an error on NULL. (This seems a pre-existing flaw.)

There seem to be far too many string literals for the various jdk.module.* forms and lots of hard-coded string lengths. It would be nice if that could be cleaned up using #defines, or string constant variables, so that all the potential property names can be located in one place.

Are there any tests in hotspot/test/* that will require changes for this?

Thanks,
David

Mandy


Reply via email to