RFR: 8283013: Simplify Arguments::parse_argument()

David Holmes dholmes at openjdk.java.net
Mon Mar 28 12:02:48 UTC 2022


On Wed, 23 Mar 2022 06:19:38 GMT, Ioi Lam <iklam at openjdk.org> wrote:

> - Remove all the complex `sscanf()` calls in `Arguments::parse_argument()`
> - Call the appropriate parsing function according to the type of the flag
> - Added more test cases for flags of the `double` type.
> 
> As a result of this change, `double` flags can now be specified in more ways, as long as the input is accepted by `strtod()`. However, `NaN` and `INFINITY` values are not allowed because the VM probably cannot handle them. Please see the test case for details.
> 
> Tested with tiers 1-5.

Hi Ioi,

This certainly seems a lot clearer and cleaner! I have a few initial comments below.

Thanks,
David

src/hotspot/share/runtime/arguments.cpp line 896:

> 894: 
> 895: static bool set_fp_numeric_flag(JVMFlag* flag, const char* value, JVMFlagOrigin origin) {
> 896:   if (*value == '\0' || isspace(*value)) {

Please preceded with comment

// strtod allows leading whitespace, but our flag format does not.

src/hotspot/share/runtime/arguments.cpp line 1062:

> 1060:     if (('a' <= c && c <= 'z' ) ||
> 1061:         ('A' <= c && c <= 'Z' ) ||
> 1062:         ('0' <= c && c <= '9' ) ||

can't you use `isalnum(c)` ?

src/hotspot/share/runtime/arguments.cpp line 1101:

> 1099:   if (arg[0] == ':' && arg[1] == '=') {
> 1100:     // -XX:Foo:=xxx will reset the string flag to the given value.
> 1101:     const char* value = arg + 2;

I wasn't aware of this syntax and I can't see what the difference is between = and := in our code (this seems to be something from makefile variable setting logic!). ??

-------------

PR: https://git.openjdk.java.net/jdk/pull/7916


More information about the hotspot-dev mailing list