RFR: 8338851: Hoist os::Posix::realpath() to os::realpath() and implement on Windows [v2]

[Simon Tooke]

On Thu, 29 Aug 2024 05:45:35 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Simon Tooke has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - simplify windwos realpath() implementation
>>  - get rid of os::posix::realpath() and os::win32::realpath()
>
> This is okay in principle but a few changes can be made.
> 
> Also it seems that none of the callers of `realpath` ever check `errno` so I think that can be removed.
> 
> Thanks

Hello, @dholmes-ora , and thank you for your review!  I am in the process of attempting to address your concerns.  Your review has resulted in simpler code for my implementation - so thanks for that too.  Please let me know if you have more concerns or questions.

> src/hotspot/os/windows/os_windows.cpp line 5344:
> 
>> 5342:     // In this case, use the user provided buffer but at least check whether _fullpath caused
>> 5343:     // a memory overwrite.
>> 5344:     if (errno == EINVAL) {
> 
> There is nothing to indicate that `_fullpath` can ever set `EINVAL` it is only specified to return null on error. This code should not check errno but can just re-try with the user-supplied buffer.

According to [the documentation](https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/fullpath-wfullpath?view=msvc-170), EINVAL can be set in some circumstances.  Those circumstances are checked earlier in the function, so I have simplified this code.

Setting EINVAL (or ENAMETOOLONG) is part of the existing documentation for os::realpath(); I am loathe to change it.

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

PR Comment: https://git.openjdk.org/jdk/pull/20683#issuecomment-2329878249
PR Review Comment: https://git.openjdk.org/jdk/pull/20683#discussion_r1744350217