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