RFR: 8365626: (fs) Improve handling of broken links in Files.isSameFile() (win) [v3]

[Alan Bateman]

On Tue, 9 Sep 2025 20:27:11 GMT, Brian Burkhalter <bpb at openjdk.org> wrote:

>> Improve handling of broken symbolic links in `Files.isSameFile` on Windows as was done on Unix by the fix of [JDK-8154364](https://bugs.openjdk.org/browse/JDK-8154364).
>
> Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8365626: Address reviewer comments since previous commit

I think the algorithm is okay but I think this patch is going to take several rounds of cleanup.

src/java.base/windows/classes/sun/nio/fs/WindowsFileSystemProvider.java line 424:

> 422:     // contains file attributes and its handle
> 423:     // the attributes' key is valid as long as the handle remains open
> 424:     private record LinkAttributes(WindowsFileAttributes attrs, long handle) {

Can you put a /** .. */ class description on this record.

src/java.base/windows/classes/sun/nio/fs/WindowsFileSystemProvider.java line 444:

> 442: 
> 443:     // find the attributes of the last accessible link in the chain
> 444:     private LinkAttributes lastFileAttrs(WindowsPath path)

Maybe rename to lastLinkAttributes ?

src/java.base/windows/classes/sun/nio/fs/WindowsFileSystemProvider.java line 444:

> 442: 
> 443:     // find the attributes of the last accessible link in the chain
> 444:     private LinkAttributes lastFileAttrs(WindowsPath path)

Can we put a more complete /** .. */ method description on this method.

src/java.base/windows/classes/sun/nio/fs/WindowsFileSystemProvider.java line 447:

> 445:         throws IOException, WindowsException
> 446:     {
> 447:         var fileAttrs = new HashSet<LinkAttributes>();

The naming here is problematic as this is a set of link attributes.

src/java.base/windows/classes/sun/nio/fs/WindowsFileSystemProvider.java line 462:

> 460: 
> 461:                 WindowsFileAttributes attrs =
> 462:                     WindowsFileAttributes.readAttributes(h);

Spurious line break here.

src/java.base/windows/classes/sun/nio/fs/WindowsFileSystemProvider.java line 490:

> 488: 
> 489:     // find the key by following links
> 490:     private LinkAttributes fileAttrs(WindowsPath file) throws WindowsException {

The comment doesn't work for this method. Can we find a better name for this method and put a more complete method description?

src/java.base/windows/classes/sun/nio/fs/WindowsFileSystemProvider.java line 526:

> 524:         if (!(obj2 instanceof WindowsPath))
> 525:             return false;
> 526:         WindowsPath file2 = (WindowsPath)obj2;

You can use instanceof WindowsPath file2 to avoid the cast.

src/java.base/windows/classes/sun/nio/fs/WindowsLinkSupport.java line 293:

> 291:      */
> 292:     static String readLink(WindowsPath path, long handle) throws IOException
> 293:     {

Minor formatting nit, the "{" can move to the previous line.

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

PR Review: https://git.openjdk.org/jdk/pull/27079#pullrequestreview-3205969709
PR Review Comment: https://git.openjdk.org/jdk/pull/27079#discussion_r2336672995
PR Review Comment: https://git.openjdk.org/jdk/pull/27079#discussion_r2336636371
PR Review Comment: https://git.openjdk.org/jdk/pull/27079#discussion_r2336674414
PR Review Comment: https://git.openjdk.org/jdk/pull/27079#discussion_r2336675152
PR Review Comment: https://git.openjdk.org/jdk/pull/27079#discussion_r2336676016
PR Review Comment: https://git.openjdk.org/jdk/pull/27079#discussion_r2336677771
PR Review Comment: https://git.openjdk.org/jdk/pull/27079#discussion_r2336632022
PR Review Comment: https://git.openjdk.org/jdk/pull/27079#discussion_r2336626428