Request for review: 8006298: Specifying malformed JFR options (--XX:+FlightRecorderOptions) outputs non-sensical error

[harold seigel]

Hi,

Please review the back port to 7 of the fix for bug 8006298.  The 
changes are similar to the original bug fix.

http://cr.openjdk.java.net/~hseigel/bug_8006298_7u/ 
<http://cr.openjdk.java.net/%7Ehseigel/bug_8006298_7u/>

Thanks, Harold

On 1/31/2013 1:40 PM, Mikael Vidstedt wrote:
>
> Thanks Harold!
>
> Cheers,
> Mikael
>
> On 2013-01-31 10:39, harold seigel wrote:
>> Good catch!
>>
>> I'll change it to strchr() before checking it in.
>>
>> Thanks, Harold
>>
>> On 1/31/2013 12:34 PM, Mikael Vidstedt wrote:
>>>
>>>
>>> Harold,
>>>
>>> Thanks for making the changes. One further question (sorry for not 
>>> asking this the first time around):
>>>
>>> Is there a fundamental reason for finding the last equals ('=') 
>>> instead of the first one - i.e. would it make more sense to use 
>>> strchr instead of strrchr? Not that I think we have any options that 
>>> accept multiple equals today, but...
>>>
>>> Cheers,
>>> Mikael
>>>
>>> On 2013-01-31 06:55, harold seigel wrote:
>>>> Hi,
>>>>
>>>> Thank you for your comments!
>>>>
>>>> Please review this updated webrev: 
>>>> http://cr.openjdk.java.net/~hseigel/bug_8006298_2/ 
>>>> <http://cr.openjdk.java.net/%7Ehseigel/bug_8006298_2/>
>>>>
>>>> It avoids the 'const' and '\0' issues by determining the length of 
>>>> the argument and passing it to find_flag().
>>>>
>>>> This changes the output messages slightly, by including the '=...' 
>>>> text.  For example:
>>>>
>>>>      % $JAVA_HOME/bin/java -XX:+UseLargePages=8 -version
>>>>     Improperly specified VM option 'UseLargePages=8'
>>>>
>>>>     % $JAVA_HOME/bin/java -XX:-ObjectAlignmentInBytes=16 -version
>>>>     Unexpected +/- setting in VM option 'ObjectAlignmentInBytes=16'
>>>>
>>>> Thanks, Harold
>>>>
>>>>
>>>> On 1/30/2013 10:39 PM, Vitaly Davidovich wrote:
>>>>>
>>>>> I'd also change "equal_sign[0] = 0;" to "equal_sign[0] = '\0';" if 
>>>>> this line is to be kept - it's more idiomatic.
>>>>>
>>>>> Thanks
>>>>>
>>>>> Sent from my phone
>>>>>
>>>>> On Jan 30, 2013 8:34 PM, "Mikael Vidstedt" 
>>>>> <mikael.vidstedt at oracle.com <mailto:mikael.vidstedt at oracle.com>> 
>>>>> wrote:
>>>>>
>>>>>
>>>>>     Harold,
>>>>>
>>>>>     Thanks for doing this, I like the improved messages a lot!
>>>>>
>>>>>     One small question:
>>>>>
>>>>>     The first argument to process_argument is a "const char* arg".
>>>>>     argname is also const, based on arg. You do this:
>>>>>
>>>>>     char* equal_sign = (char *)strrchr(argname, '=');
>>>>>     if (equal_sign > argname)
>>>>>     equal_sign[0] = 0;
>>>>>
>>>>>     Doesn't that effectively break the const'ness of the incoming
>>>>>     argument?
>>>>>
>>>>>     Cheers,
>>>>>     Mikael
>>>>>
>>>>>     On 2013-01-30 10:51, harold seigel wrote:
>>>>>>     Hi,
>>>>>>
>>>>>>     Please review the following change to fix bug 8006298.
>>>>>>
>>>>>>     Summary:
>>>>>>     This change enables hotspot to emit more useful messages when
>>>>>>     Java options are specified incorrectly.
>>>>>>
>>>>>>     This was tested using JCK, UTE, and JTREG tests, and by
>>>>>>     hand.  Below are the test cases that were run by hand. 
>>>>>>     Please let me know if you have any additional suggestions. 
>>>>>>     (The "Error:..." messages appeared in all cases but are only
>>>>>>     shown for the initial case.)
>>>>>>
>>>>>>         $JAVA_HOME/bin/java -XX:UseLargePages -version
>>>>>>         Missing +/- setting for VM option 'UseLargePages'
>>>>>>         Error: Could not create the Java Virtual Machine.
>>>>>>         Error: A fatal exception has occurred. Program will exit.
>>>>>>
>>>>>>         $JAVA_HOME/bin/java -XX:+UseLargePages=8 -version
>>>>>>         Improperly specified VM option 'UseLargePages'
>>>>>>
>>>>>>         $JAVA_HOME/bin/java -XX:ObjectAlignmentInBytes=v -version
>>>>>>         Improperly specified VM option 'ObjectAlignmentInBytes'
>>>>>>
>>>>>>         $JAVA_HOME/bin/java -XX:-ObjectAlignmentInBytes=16 -version
>>>>>>         Unexpected +/- setting in VM option
>>>>>>         'ObjectAlignmentInBytes' <-- 64 bit VM's
>>>>>>         Unrecognized VM option 'ObjectAlignmentInBytes' <-- 32
>>>>>>         bit VM's
>>>>>>
>>>>>>         $JAVA_HOME/bin/java -XX:bogus_option -version
>>>>>>         Unrecognized VM option 'bogus_option'
>>>>>>
>>>>>>     Open webrev at
>>>>>>     http://cr.openjdk.java.net/~hseigel/bug_8006298/
>>>>>>     <http://cr.openjdk.java.net/%7Ehseigel/bug_8006298/>
>>>>>>
>>>>>>     Bug link at http://bugs.sun.com/view_bug.do?bug_id=8006298
>>>>>>
>>>>>>     Thanks!  Harold
>>>>>
>>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130205/ea2c4cfb/attachment-0001.html