RFR: 8366272: The os::xxx APIs do not manage errno correctly [v3]

[Anton Artemov]

On Mon, 8 Dec 2025 05:58:56 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Anton Artemov has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8366272: Addressed reviewer's comments.
>
> src/hotspot/os/aix/os_aix.cpp line 2334:
> 
>> 2332:         ErrnoPreserver ep;
>> 2333:         ::close(fd);
>> 2334:         errno = ep.saved_errno();
> 
> I suspect the intent is that we set EISDIR assuming the close succeeds, but if the close fails then errno reflects that error. Otherwise the trivial change here would be to reorder the two statements.

Right, a bit over-engineered. Addressed in the latest commit.

> src/hotspot/os/windows/os_windows.cpp line 4785:
> 
>> 4783:       ErrnoPreserver ep;
>> 4784:       os::free(wide_path);
>> 4785:       errno = ep.saved_errno();
> 
> Just swap the statements

Addressed in the latest commit.

> src/hotspot/os/windows/os_windows.cpp line 4805:
> 
>> 4803:     os::free(wide_path);
>> 4804:     os::free(path_to_target);
>> 4805:     errno = ep.saved_errno();
> 
> I would just move the free calls higher, then set errno after that.

Addressed in the latest commit.

> src/hotspot/os/windows/os_windows.cpp line 5007:
> 
>> 5005:       ErrnoPreserver ep;
>> 5006:       os::free(wide_path);
>> 5007:       errno = ep.saved_errno();
> 
> Again in this simple case we can just switch the order and set errno after the free.

Addressed in the latest commit.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28086#discussion_r2597743098
PR Review Comment: https://git.openjdk.org/jdk/pull/28086#discussion_r2597743986
PR Review Comment: https://git.openjdk.org/jdk/pull/28086#discussion_r2597743604
PR Review Comment: https://git.openjdk.org/jdk/pull/28086#discussion_r2597743312