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

Lois Foltan lois.foltan at oracle.com
Wed Nov 12 16:24:37 UTC 2014


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)?

   - 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



More information about the hotspot-dev mailing list