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

[David Holmes]

On Sun, 13 Oct 2024 15:38:49 GMT, Simon Tooke <stooke at openjdk.org> wrote:

>> This PR changes the status of realpath() from a Posix-specific API to a globally available API, i.e. adding it to the "Hotspot Porting API".  Code would refer to os::realpath() instead of os::Posix::realpath().
>> 
>> This requires a Windows implementation of realpath(), using Windows _fullpath(), and renaming os::Posix::realpath() to os::realpath().
>> 
>> The main difference between POSIX and Windows behaviour is that POSIX actually requires an existing accessible file, while Windows will happily work with made-up filenames.
>> 
>> Please note that guidelines for doing this appear in src/hotspot/share/runtime/os.hpp
>
> Simon Tooke has updated the pull request incrementally with one additional commit since the last revision:
> 
>   clean up test code

Hi @stooke , I was away on vacation when you made the updates.

I'm afraid I still have some issues about expected behaviour on Windows with regard to the tests.

test/hotspot/gtest/runtime/test_os.cpp line 413:

> 411: 
> 412: TEST_VM(os, realpath) {
> 413:   /* POSIX requires that the file exists, Windows doesn't */

I'm not sure what this comment means. I can't fully discern from

https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/fullpath-wfullpath?view=msvc-170

what Windows does if the path does not exist.

test/hotspot/gtest/runtime/test_os.cpp line 422:

> 420:   errno = 0;
> 421:   const char* returnedBuffer = os::realpath(nosuchpath, buffer, sizeof(nosuchpath) - 2);
> 422:   /* Returns ENOENT on Linux, ENAMETOOLONG on Windows */

Suggestion:

  /* Reports ENOENT on Linux, ENAMETOOLONG on Windows */

and similarly below.

test/hotspot/gtest/runtime/test_os.cpp line 425:

> 423:   EXPECT_TRUE(returnedBuffer == nullptr);
> 424: #if defined(_WINDOWS)
> 425:   EXPECT_TRUE(errno == ENAMETOOLONG);

Why is this the case? Our implementation does not set it and `_fullpath` makes no mention of it.

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

PR Review: https://git.openjdk.org/jdk/pull/20683#pullrequestreview-2370878680
PR Review Comment: https://git.openjdk.org/jdk/pull/20683#discussion_r1802144245
PR Review Comment: https://git.openjdk.org/jdk/pull/20683#discussion_r1802139054
PR Review Comment: https://git.openjdk.org/jdk/pull/20683#discussion_r1802142390