RFR: 8357086: os::xxx functions returning memory size should return size_t [v25]
Anton Artemov
duke at openjdk.org
Wed Aug 6 09:27:30 UTC 2025
On Wed, 6 Aug 2025 03:42:19 GMT, David Holmes <dholmes at openjdk.org> wrote:
>> Anton Artemov has updated the pull request incrementally with one additional commit since the last revision:
>>
>> 8357086: Addressed reviewer's comments
>
> src/hotspot/os/linux/os_linux.cpp line 299:
>
>> 297: jlong memory_and_swap_limit_in_bytes = OSContainer::memory_and_swap_limit_in_bytes();
>> 298: jlong memory_limit_in_bytes = OSContainer::memory_limit_in_bytes();
>> 299: value = static_cast<size_t>(memory_and_swap_limit_in_bytes - memory_limit_in_bytes);
>
> This isn't what I meant. You want to save into the locals before each of the `if` statements, else you are still calling these functions twice.
Right, I changed the way it is done as suggested.
> src/hotspot/os/windows/os_windows.cpp line 869:
>
>> 867: return true;
>> 868: } else {
>> 869: errno = ::GetLastError();
>
> Why are you setting `errno` here and elsewhere? I see a couple of places where we do do that though I do not know why, but generally we just want to save the `::GetlastError()` value (here we do need a local) and print it.
You are correct, there is no relationship between `errno` and `GetLastError()`. Addressed.
I created a separate issue to fix this inconsistency in other places.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2256538537
PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2256536814
More information about the hotspot-dev
mailing list