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

[Max Ockner]

Dan,
I have reformatted  the  "){" fragment on line 336 as you recommended. 
Thanks for catching that.

For your second recommendation, I think I have a use case where the 
recommended code would not function properly:

Let's say there is a boolean flag SomeFlag, and let's say that the user 
tries to type  "-XX:SomeFlagg".

The first if statement passes because strlen("SomeFlagg") = 
strlen("SomeFlag")+1.
The second conditional checks if (strncmp(flag_status.name, s, f_len) == 
0).  But f_len, the length of "SomeFlag" is 8. The result is that the 
9th character of the user's input, which is where s differs from 
flag_status.name, is not checked,so this condition is passed as well.

Thanks,
Max



On 11/12/2014 1:41 PM, Daniel D. Daugherty wrote:
> On 11/7/14 12:13 PM, Max Ockner wrote:
>> ID: 8060449
>> webrev: http://cr.openjdk.java.net/~coleenp/8060449/
>
> src/share/vm/runtime/arguments.cpp
>
>     line 336: ) {
>         This fragment is on a line by itself and far left.
>         Minimally, it should align like this:
>
>         line 331:     if (...
>         line 336:        ) {
>
>     However, I recommend a slightly different structure to
>     this logic:
>
>     size_t f_len = strlen(flag_status.name);
>     size_t s_len = strlen(s);
>     if (f_len == s_len || (f_len + 1) == s_len) {
>       // this flag is the right length for a possible match
>       if (strncmp(flag_status.name, s, f_len) == 0) ||
>           ((s[0] == '+' || s[0] == '-') &&
>            strncmp(flag_status.name, &s[1], f_len) == 0)) {
>         // this flag is an exact match
>         if (JDK_Version::current().compare(flag_status.accept_until) 
> == -1) {
>         ...
>         }
>       }
>     }
>     i++;
>
> I have no idea if the above formatting is going to be
> preserved by e-mail clients...
>
> Dan
>
>
>>
>> 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
>>
>>
>