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

[harold seigel]

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/20130131/af0a732a/attachment.html