RFR (S) round 0 for 8141068 Refactor -XXFlags= code in preparation for removal

[Ron Durbin]

Dan,

Thanks for the refresh on the history on why we do the checks the way we do.

>-----Original Message-----
>From: Daniel D. Daugherty
>Sent: Wednesday, November 11, 2015 10:35 AM
>To: Coleen Phillimore; Ron Durbin; hotspot-runtime-dev at openjdk.java.net
>Subject: Re: RFR (S) round 0 for 8141068 Refactor -XXFlags= code in preparation for removal
>
>On 11/11/15 9:43 AM, Coleen Phillimore wrote:
>>
>>
>> On 11/11/15 11:36 AM, Daniel D. Daugherty wrote:
>>> This needs a little bit of clarification:
>>>
>>> 3976 jint Arguments::match_special_option_and_act(const
>>> JavaVMInitArgs* args,
>>> 3977                                              char **
>>> vm_options_file,
>>> 3978 ScopedVMInitArgs* args_out) {
>>>
>>> The 'vm_options_file' parameter is currently used for two things:
>>>
>>> 1) when the parameter is non-NULL, that indicates the caller
>>>    supports '-XX:VMOptionsFile'; currently only the cmd line
>>>    option bucket supports '-XX:VMOptionsFile'
>>
>> The args_out != NULL parameter indicates the same thing.
>
>Yes. That's the check that used to be done. However, Ron
>previously consolidated all of the checks to use the same
>variable (vm_options_file). It seemed to confuse folks that
>one check was done with 'vm_options_file' and the other check
>was done with 'args_out'.
>
>
>>> 2) makes sure that only one -XX:VMOptionsFile=foo option is
>>>    specified on the cmd line
>>
>> You can do that with a local variable vm_options_file in
>> match_special_option_and_act.
>>
>> So it can be trivially removed.   But it's not a big deal.
>
>Yes, that's possible. And then we would be back in the situation
>where one check is done via 'args_out' and the other check is
>done via 'vm_options_file'.
>
>The other reason for having 'vm_options_file' have a more global
>scope was that we didn't know whether the restriction for having
>only one '-XX:VMOptionsFile' would apply across all buckets when
>support was added for the env vars. Again, that's something that
>will be dealt with JDK-8135198.
>
>Dan
>
>
>>
>> Coleen
>>>
>>> Restriction #1 is implemented by:
>>>
>>> 3994       if (vm_options_file != NULL) {
>>> <snip>
>>> 4022       } else {
>>> 4023         jio_fprintf(defaultStream::error_stream(),
>>> 4024                     "VM options file is only supported on the
>>> command line\n");
>>> 4025         return JNI_EINVAL;
>>> 4026       }
>>>
>>> Restriction #2 is implemented by:
>>>
>>> 3994       if (vm_options_file != NULL) {
>>> 3995         // The caller accepts -XX:VMOptionsFile
>>> 3996         if (*vm_options_file != NULL) {
>>> 3997           jio_fprintf(defaultStream::error_stream(),
>>> 3998                       "The VM Options file can only be specified
>>> once and "
>>> 3999                       "only on the command line.\n");
>>> 4000           return JNI_EINVAL;
>>> 4001         }
>>>
>>> So the 'vm_options_file' parameter cannot be removed at this
>>> time without changing the current set of restrictions on the
>>> '-XX:VMOptionsFile' option. Ron has another bug looking at
>>> lifting those restrictions:
>>>
>>> JDK-8135198 Specifying a vm options file should not be limited
>>>             to only once on the command line
>>> https://bugs.openjdk.java.net/browse/JDK-8135198
>>>
>>> JDK-8135198 will require a CCC request in order to change the
>>> semantics that were defined in the original work (JDK-8061999).
>>>
>>> Dan
>>>
>>>
>>> On 11/10/15 2:02 PM, Ron Durbin wrote:
>>>> Coleen
>>>>
>>>> This is to formally respond to your email.  Dan and I agreed the
>>>> change you are requesting should be done, but not
>>>> in this change set.  Keeping this change set to only XX:Flags
>>>> changes. A future change set that could clean up
>>>> the options file processing should address your issue.
>>>>
>>>> Ron
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Coleen Phillimore
>>>>> Sent: Monday, November 09, 2015 2:03 PM
>>>>> To: hotspot-runtime-dev at openjdk.java.net
>>>>> Subject: Re: RFR (S) round 0 for 8141068 Refactor -XXFlags= code in
>>>>> preparation for removal
>>>>>
>>>>>
>>>>> Ron,
>>>>>
>>>>> This looks really good!  Although I don't see why you need to pass
>>>>> vm_options_file argument into match_special_options_and_act now since
>>>>> it's not used afterward, so you only have to pass one NULL in the
>>>>> cases
>>>>> where vm_options_file is not allowed.
>>>>>
>>>>> thanks,
>>>>> Coleen
>>>>>
>>>>> On 11/6/15 2:05 PM, Ron Durbin wrote:
>>>>>> This bug fix consists of a refactoring/cleanup of the -XX:Flags code.
>>>>>>
>>>>>> Webrev:
>>>>>> http://cr.openjdk.java.net/~rdurbin/8141068_OCR0_JDK9_webrev
>>>>>>
>>>>>>
>>>>>> Bug report:
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8141068
>>>>>>
>>>>>> This bug fix has been tested on:
>>>>>>      OS:
>>>>>>        Solaris, MAC, Windows, Linux
>>>>>>      Tests:
>>>>>>        Manual unit tests
>>>>>>        JPRT with -testset hotspot(including the SQE current test
>>>>>> coverage for this feature.)
>>>
>>
>