RFR: 8324781: runtime/Thread/TestAlwaysPreTouchStacks.java failed with Expected a higher ratio between stack committed and reserved [v3]
Johan Sjölen
jsjolen at openjdk.org
Mon Apr 8 13:01:10 UTC 2024
On Mon, 8 Apr 2024 12:42:25 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:
>> src/hotspot/os/linux/os_linux.cpp line 387:
>>
>>> 385: } else {
>>> 386: ++walker;
>>> 387: }
>>
>> I can't tell if this code is correct or not.
>> * When is walker ever nullptr?
>> * Don't wee need error handling for strtol?
>>
>> Maybe this is all pre-existing issues, but I'd like to get someone else to review and sign off on this code.
>
> Not typical code for me either.
>
> Quoting my man pages:
>
>
> If endptr is not NULL, strtol() stores the address of the first
> invalid character in *endptr. If there were no digits at all,
> strtol() stores the original value of nptr in *endptr (and
> returns 0). In particular, if *nptr is not [aq]\0[aq] but
> **endptr is [aq]\0[aq] on return, the entire string is valid.
>
>
> Given a string such as `2.6.5`, each `strtol` will put `walker` at a new `.` and as minor is always set the condition in the loop will become false. The `walker` will never be null. Looking at the documentation, we don't need error-handling for the `strtol` call (won't over/underflow and will be a valid version revision).
Basic fix: change `walker != nullptr` to `walker[0] != '\0'` and add a test case for this.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18592#discussion_r1555802987
More information about the hotspot-runtime-dev
mailing list