RFR: 8357086: os::xxx functions returning memory size should return size_t [v28]

Anton Artemov duke at openjdk.org
Wed Aug 20 07:19:39 UTC 2025


On Wed, 20 Aug 2025 01:06:34 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 306:
> 
>> 304:   int ret = sysinfo(&si);
>> 305:   if (ret != 0) {
>> 306:     assert(false, "sysinfo failed in total_swap_space()?");
> 
> We should report the value of errno in these cases.
> Suggestion:
> 
>     assert(false, "sysinfo failed in total_swap_space(): %s", os::strerror(errno));

Added `errno `reporting as suggested.

> src/hotspot/os/windows/os_windows.cpp line 869:
> 
>> 867:     return true;
>> 868:   } else {
>> 869:     assert(false, "GlobalMemoryStatusEx failed in win32::available_memory().");
> 
> These should report the value of `::GetLastError()`

Added error reporting as suggested.

> src/hotspot/os/windows/os_windows.cpp line 870:
> 
>> 868:   } else {
>> 869:     assert(false, "GlobalMemoryStatusEx failed in win32::available_memory().");
>> 870:     return res == TRUE;
> 
> Suggestion:
> 
>     return false;
> 
> Please restore all these to "return false" as we know that `res == false` here.

Addressed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2287217685
PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2287217572
PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2287217452


More information about the hotspot-dev mailing list