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