RFR: 8350869: os::stat doesn't follow symlinks on Windows [v5]

David Holmes dholmes at openjdk.org
Tue May 20 22:23:54 UTC 2025


On Tue, 20 May 2025 12:22:36 GMT, Anton Artemov <duke at openjdk.org> wrote:

>> This PR addresses an issue on Windows, where a class file could not be loaded by a symbolic link. 
>> 
>> I added a check if a wide path is a symbolic link (soft one) and a method for dereferencing. Both os::stat() and os::open() on Windows now can handle such links.
>> 
>> Tested in tiers 1-3. A separate test added.
>
> Anton Artemov has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8350869: Clean up of the test, error handling in get_path_to_target

There is generally no need to include quotes from API documentation.

Thanks

src/hotspot/os/windows/os_windows.cpp line 4643:

> 4641:   // One does not need extra character in the end, from documentation:
> 4642:   // "If the function fails because lpszFilePath is too small to hold the string plus the terminating null character,
> 4643:   // the return value is the required buffer size, in TCHARs. This value INCLUDES the size of the terminating null character."

A simple:

// Returned value includes the terminating null character.

will suffice.

src/hotspot/os/windows/os_windows.cpp line 4657:

> 4655:   // "If the function succeeds, the return value is the length of the string received by lpszFilePath,
> 4656:   // in TCHARs.This value DOES NOT INCLUDE the size of the terminating null character."
> 4657:   // So the return value is ONE LESS than target_path_size if everything is ok

Again please simplify:

// The returned size is the length excluding the terminating null character.

src/hotspot/os/windows/os_windows.cpp line 4672:

> 4670:   }
> 4671: 
> 4672:   path_to_target[target_path_size - 1] = '\0';

Why are we explicitly adding the null character ourselves?

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

Changes requested by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/25233#pullrequestreview-2855715910
PR Review Comment: https://git.openjdk.org/jdk/pull/25233#discussion_r2098959245
PR Review Comment: https://git.openjdk.org/jdk/pull/25233#discussion_r2098960494
PR Review Comment: https://git.openjdk.org/jdk/pull/25233#discussion_r2098961439


More information about the hotspot-runtime-dev mailing list