RFR: 8283013: Simplify Arguments::parse_argument() [v2]

Ioi Lam iklam at openjdk.java.net
Mon Mar 28 21:00:57 UTC 2022


On Mon, 28 Mar 2022 11:51:52 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Ioi Lam has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Disabled test for CompileThresholdScaling due to JDK-8283807
>>  - @dholmes-ora comments
>
> 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.

Fixed.

> src/hotspot/share/runtime/arguments.cpp line 905:
> 
>> 903:     return false;
>> 904:   }
>> 905:   if (g_isnan(v) || !g_isfinite(v)) {
> 
> Surely the not-sign should not be there ???

This is actually correct. If the number is NOT FINITE, we don't accept it. There's a test case for values like `"Infinity"`:

https://github.com/openjdk/jdk/blob/8413d87666e58c18c914b7df043ba3dbc6fa9022/test/hotspot/gtest/runtime/test_arguments.cpp#L280-L287

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

Fixed.

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

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


More information about the hotspot-dev mailing list