RFR: 8364816: GetLastError() in os_windows.cpp should not store value to errno [v4]

[Anton Artemov]

On Thu, 28 Aug 2025 02:15:29 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Anton Artemov has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8364816: Modified case when ENOENT value is stored to errno.
>
> src/hotspot/os/windows/os_windows.cpp line 5035:
> 
>> 5033:   if (fd == -1) {
>> 5034:     DWORD errcode = ::GetLastError();
>> 5035:     log_debug(os)("os::open() failed to _wopen: GetLastError->%lu.", errcode);
> 
> This is not actually needed. `_wopen` sets `errno` not `GetlastError`, so the original code is just wrong.
> 
> However, (and this is a general problem with errno and cleanup actions) if we log the error, we will modify errno so even if a caller is checking errno they won't get what they expect. Even if not logging the `os::free` can also potentially modify errno so this use of errno is inherently flawed!
> 
> A proper fix (and likely applies to non-Windows code too) would be to save errno and restore only upon returning.

Thanks, I addressed this local problem. It looks like Windows os-related functions do not have consistency. Some, like this one, report the error type via `errno`, some can report via a separate call to `GetLastError`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26901#discussion_r2306791897