Review Request JDK-8136930 Examine implications for custom launchers, equivalent of java -X options in particular

Mandy Chung mandy.chung at oracle.com
Mon Jun 6 03:51:28 UTC 2016


> On Jun 5, 2016, at 5:50 PM, David Holmes <david.holmes at oracle.com> wrote:
> 
> 
> 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
> 

-Djdk.module.patch.1junk=<arg> is expected not match and should not be ignored.  Hence it should be set in the system property.  

155         ((strncmp(prop_end, "patch.", 6) == 0) && isdigit(prop_end[6]))) {

Harold - is it intentional? I assume we want to match the suffix if it’s a number.

> 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.

I agree.  

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

Not in the hotspot open repo.

Mandy



More information about the hotspot-runtime-dev mailing list