RFR:8060449:Proper error messages for newly obsolete command line flags.

[Lois Foltan]

On 11/12/2014 11:24 AM, Lois Foltan wrote:
> Hi Max,
>
> Overall, this looks good.  A couple of comments:
>
> src/share/vm/runtime/arguments.cpp
>
>   - Within the altered if statement, shouldn't the second comparison 
> of &s[1] to len actually be (strlen(&s[1] == (len-1)), since the 
> original len was calculated to take into account the character in 
> s[0]?  This comment also applies to the second strncmp, shouldn't it 
> be (len-1)?

Scratch this first comment all together.  I misread and thought the 
strlen was calculated differently against s and not flag_status.name at 
first.  Sorry about that!
Lois

>
>   - Minor coding style, if the first part of an if statement logically 
> does a "strncmp" and then a "strlen" it would be nice if follow on 
> conditions in that if statement did the same for consistency.  Please 
> consider reversing the second "strlen" and "strncmp".  I find it a 
> little bit more readable that way.
>
> test/runtime/CommandLine/ObsoleteFlagErrorMessage.java
>
>   - looks good, please update copyright before pushing.
>
> Thanks,
> Lois
>
> On 11/7/2014 2:13 PM, Max Ockner wrote:
>> ID: 8060449
>> webrev: http://cr.openjdk.java.net/~coleenp/8060449/
>>
>> Summary: A "newly obsolete" command line option is one which is no 
>> longer supported, but still is acknowledged. There is a list of these 
>> in arguments.cpp.
>> It used to be that only a fixed number of characters were checked 
>> when comparing a given command line option to the list of obsolete 
>> flags (strncmp was used, where the number of characters to check is 
>> equal to the length of the flag name from the table.)
>> As a result, an arbitrary string appended to the end of an obsolete 
>> argument goes unnoticed.
>> This issue is fixed by comparing the lengths of the given flag and 
>> the flags from the obsolete flags table.
>> When a misspelled flag is fuzzy-matched to an obsolete flag, an 
>> appropriate warning is given to save the user a few key strokes: (1) 
>> unrecognized option [bad option]. (2) Did you mean [option]? (3) 
>> [option] is obsolete as of [version])
>>
>> A new test for this feature checks for the presence of all three 
>> components of the above error message.
>>
>> Tested with: vm.quick.testlist
>>              hotspot jtreg tests
>>              jprt
>>
>> Thanks for your help!
>> Max Ockner
>