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