RFR: 8364816: GetLastError() in os_windows.cpp should not store value to errno
Joel Sikström
jsikstro at openjdk.org
Tue Aug 26 16:44:34 UTC 2025
On Fri, 22 Aug 2025 12:34:41 GMT, Anton Artemov <duke at openjdk.org> wrote:
> Hi, please consider the following changes:
>
> In `os_windows.cpp` in a few places results returned by `GetLastError()` are stored to `errno`. However, `errno` has no relation to `GetLastError()`, and their ranges are not the same.
>
> Results of `GetLastError()` should be stored into variables of type `DWORD`.
>
> The removed section in `src/hotspot/share/cds/aotClassLocation.cpp` was relying on values returned by `GetLastError()` and stored to `errno` in `os::stat()`. Though the logic was correct, these values should not be stored to `errno`. The functionality is preserved by storing a **valid** value `ENOENT` to `errno` in `os::stat()`.
>
> Tested in tiers 1 - 3.
I have one comment but otherwise this looks reasonable!
src/hotspot/os/windows/os_windows.cpp line 4700:
> 4698: if (hFile == INVALID_HANDLE_VALUE) {
> 4699: DWORD errcode = ::GetLastError();
> 4700: log_debug(os)("get_path_to_target() failed to CreateFileW: GetLastError->%lu.", errcode);
The callers of `get_path_to_target` expects errno to be set, which it isn't anymore. We should probably set errno in this case and update the comments.
See: https://github.com/openjdk/jdk/blob/master/src/hotspot/os/windows/os_windows.cpp#L4804-L4813 and https://github.com/openjdk/jdk/blob/master/src/hotspot/os/windows/os_windows.cpp#L5018-L5027
-------------
PR Review: https://git.openjdk.org/jdk/pull/26901#pullrequestreview-3156432237
PR Review Comment: https://git.openjdk.org/jdk/pull/26901#discussion_r2301524241
More information about the hotspot-runtime-dev
mailing list