RFR: 8364816: GetLastError() in os_windows.cpp should not store value to errno [v2]
Anton Artemov
duke at openjdk.org
Wed Aug 27 07:37:28 UTC 2025
On Tue, 26 Aug 2025 16:25:58 GMT, Joel Sikström <jsikstro at openjdk.org> wrote:
>> Anton Artemov has updated the pull request incrementally with one additional commit since the last revision:
>>
>> 8364816: Fixed misleading comments.
>
> 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
Thanks for spotting. Actually, this comment, as well as the code around it, is mine as well. When writing it, I was storing results of `GetLastError()` in `errno`, which as we know is wrong and is addressed in this PR.
The callers of `get_path_to_target()` do not expect any value in `errno` actually. The only thing they do when failing (this is where the `errno` value would be potentially useful) is that freeing some char array and returning -1. The error, which caused it, has been already reported. The comment is misleading. I fixed it.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26901#discussion_r2303129187
More information about the hotspot-runtime-dev
mailing list