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

[David Holmes]

Hi Harold,

On 7/06/2016 6:19 AM, harold seigel wrote:
> Hi David,
>
> On 6/5/2016 8:50 PM, David Holmes wrote:
>> 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.
>
> Would you prefer an implementation that looks like this?  (The isdigit()
> issue will be addressed separately.)
>
>    #define MODULE_PROPERTY_PREFIX "jdk.module"
>    #define ADDEXPORTS "addexports"
>    #define ADDREADS "addreads"
>    #define PATCH "patch"
>    #define ADDMODS "addmods"
>    #define LIMITMODS "limitmods"
>
>    // Return true if the option is one of the module-related java
>    properties
>    // that can only be set using the proper module-related option and
>    cannot
>    // be read by jvmti.
>    // It's expected that the caller removed the leading "-D" from 'option'.
>    bool Arguments::is_internal_module_property(const char* option) {
>       assert((strncmp(option, "-D", 2) != 0), "Unexpected leading -D");
>       if (strncmp(option, MODULE_PROPERTY_PREFIX ".",
>    strlen(MODULE_PROPERTY_PREFIX) + 1) == 0) {
>         const char* prop_end = option + 11; // 11 is strlen("jdk.module.")
>         // For the repeating properties such as (-Djdk.module.patch.0
>         // -Djdk.module.patch.1, etc) return true for
>    "-D<property_name>.<digit>...".
>         if (((strncmp(prop_end, ADDEXPORTS ".", strlen(ADDEXPORTS) + 1)
>    == 0) && isdigit(prop_end[11])) ||
>             ((strncmp(prop_end, ADDREADS ".", strlen(ADDREADS) + 1) ==
>    0) && isdigit(prop_end[9])) ||
>             ((strncmp(prop_end, PATCH ".", strlen(PATCH) + 1) == 0) &&
>    isdigit(prop_end[6]))) {
>           return true;
>         }
>         return Arguments::is_matching_property(prop_end, ADDMODS,
>    strlen(ADDMODS)) ||
>                Arguments::is_matching_property(prop_end, LIMITMODS,
>    strlen(LIMITMODS));
>       }
>       return false;
>    }
>                   ...

This is better. Though I don't object to using eg

#define ADDMODS_LEN 7

to avoid strlen calls. Or if you want to be more pure something like:

static const char * ADDMODS = "addmods";
static int ADDMODS_LEN  = strlen(ADDMODS);

etc. We really shouldn't calculate any of those string lengths more than 
once.

Thanks,
David

> Thanks, Harold
>