RFR (M) round 2 8061999 Enhance VM option parsing to allow options to be specified

Coleen Phillimore coleen.phillimore at oracle.com
Mon Aug 31 20:07:13 UTC 2015


Ron,

I believe that the code in the insert function could be simpified as 
below.  It doesn't need the count in the loop counter.

3451     for (int count = 0, i = 0; count < length; i++) {

simpler loop just counts the the incoming arguments and pushes 
args_to_insert at position.

     for (int i = 0; i < args->nOptions; i++) {
       if (i == vm_args_position) {
         // insert the new options at position
         for (int j = 0; j < args_to_insert->nOptions; j++) {
            options->push(args_to_insert->options[j]);
         }
       } else {
         options->push(args->options[i]);
       }
     }

I don't see when this gets freed and you use the value before it gets 
back to the caller, so I don't see why it needs to be strdup'ed.

3775         // Save a copy of vm_options_file since we don't know
3776         // when args will get freed by the caller.
3777         *vm_options_file = os::strdup((char *)tail);


Also with -XX:Flags=

3755       // Save a copy of flags_file since we don't know
3756       // when args will get freed by the caller.
3757       *flags_file = strdup(tail);

These options are strdup-ed in ScopedVMInitArgs::set_args() and 
deallocated in the ~ScopedVMInitArgs destructor so should not be freed 
during the lifetime of the Arguments::parse() function.    I think you 
said you found problems in testing with flags_file being deallocated but 
I don't see where that would have been.

thanks,
Coleen

On 8/28/15 9:56 AM, Ron Durbin wrote:
> Here is the round 2 webrev for 8061999.
>
> Due to the large number of conflicts with other bug fixes in the cmd options
> area and the resulting refactoring of this fix, a delta webrev is not provided
> relative to round 1 because it wouldn't make any sense.
>
> Webrev link: http://cr.openjdk.java.net/~rdurbin/8061999_OCR2_JDK9_webrev
>
> RFE link: https://bugs.openjdk.java.net/browse/JDK-8061999
>
> This RFE allows a file to be specified that holds VM Options that
> would otherwise be specified on the command line or in an environment variable.
> Only one options file may be specified on the command line and no options file
> may be specified in either of the following environment variables
> "JAVA_TOOL_OPTIONS" or "_JAVA_OPTIONS".
>
> The options file feature supports all VM options currently supported on
> the command line, except the options file option. The option to specify an
> options file is "-XX:VMOptionsFile=<Filename>".
> The options file feature supports an options file up to 1024 bytes in size,
>
> This feature has been tested on:
>    OS:
>      Solaris, MAC, Windows, Linux
>    Tests:
>      Manual unit tests
>      JPRT with -testset hotspot (including the SQE proposed test coverage for this feature.)
>      Aurora,(Big Apps, JTREG,Tonga), Runtime SVC Nightly



More information about the hotspot-runtime-dev mailing list