RFC: Junction Point/NT Symlink Support [2]

[Erich E. Hoover]

I would love any and all feedback on updated versions of my "Junction
Point" patches, which are now available in staging:
https://github.com/wine-staging/wine-staging/tree/master/patches/ntdll-Junct...

Major changes since the last publicly available version are:
1) Use of renameat2(..., RENAME_EXCHANGE), when possible
2) You can now tell the difference between Junction Points and NT
Symlinks (the reparse tag is stored in the Unix Symlink as a magic
string)
3) Support has been added for absolute symlinks
4) Support has been added for relative symlinks
5) Support has been added for file symlinks
6) Significantly more tests have been added and quirky delete
behaviors identified and fixed
7) CreateSymbolicLink[A|W] is now implemented with
DeviceIoControl(...,FSCTL_SET_REPARSE_POINT,...)

Thank you in advance to those who look these patches over, your
feedback is greatly appreciated.

Best,
Erich

[Hans Leidekker]

On Wed, 2019-04-17 at 08:29 -0600, Erich E. Hoover wrote:
> Thank you in advance to those who look these patches over, your
> feedback is greatly appreciated.
> +NTSTATUS FILE_RemoveSymlink(HANDLE handle, REPARSE_GUID_DATA_BUFFER *buffer)
> +{
> - int dest_fd, needs_close;
> - ANSI_STRING unix_name;
> - NTSTATUS status;
> -
> - if ((status = server_get_unix_fd( handle, FILE_SPECIAL_ACCESS, &dest_fd, &needs_close, NULL, NULL )))
> -
>    return status;
> -
> - if ((status = server_get_unix_name( handle, &unix_name )))
> -
>    goto cleanup;
> -
> - TRACE("Deleting symlink %s\n", unix_name.Buffer);
> - if (unlink( unix_name.Buffer ) < 0)
> - {
> -
>    status = FILE_GetNtStatus();
> -
>    goto cleanup;
> - }
> - if (mkdir( unix_name.Buffer, 0775 ) < 0)
> - {
> -
>    status = FILE_GetNtStatus();
> -
>    goto cleanup;
> - }

Shouldn't this also be a single operation? What about ownership and
permissions on the directory? Should they be preserved?

[Erich E. Hoover]

On Fri, Apr 19, 2019 at 2:30 AM Hans Leidekker  wrote:
> On Wed, 2019-04-17 at 08:29 -0600, Erich E. Hoover wrote:
> > ...
> > - TRACE("Deleting symlink %s\n", unix_name.Buffer);
> > - if (unlink( unix_name.Buffer ) < 0)
> > - {
> > -
> >    status = FILE_GetNtStatus();
> > -
> >    goto cleanup;
> > - }
> > - if (mkdir( unix_name.Buffer, 0775 ) < 0)
> > - {
> > -
> >    status = FILE_GetNtStatus();
> > -
> >    goto cleanup;
> > - }
>
> Shouldn't this also be a single operation?

Indeed it should, I could have sworn that I updated that a long time
ago - probably lost it in a rebase at some point.
> What about ownership and
> permissions on the directory? Should they be preserved?

Yes, though I don't see a great way to do that with permissions
(ownership is fine). At least on Linux symlinks don't preserve
permission data, so about the best that can be done is to copy the
permissions of the symlink's destination (if available).  Unless you
are aware of some way to do this that I'm not?  I explored this a long
time ago and I still have a little tidbit of test code from that:
===
    int ret = fchmodat(-1, "testlink", 0775, AT_SYMLINK_NOFOLLOW);
    fprintf(stderr, "ret? %d %m\n", ret);
===
and it (unfortunately) returns:
ret? -1 Operation not supported

Thank you so much for taking a look at these patches for me, hopefully
it won't be too much longer before they're ready for upstreaming :)

Best,
Erich

[Austin English]

On Wed, Apr 17, 2019 at 9:30 AM Erich E. Hoover 
wrote:
> I would love any and all feedback on updated versions of my "Junction
> Point" patches, which are now available in staging:
>
> https://github.com/wine-staging/wine-staging/tree/master/patches/ntdll-Junct...
>
> Major changes since the last publicly available version are:
> - Use of renameat2(..., RENAME_EXCHANGE), when possible
> - You can now tell the difference between Junction Points and NT
>
> Symlinks (the reparse tag is stored in the Unix Symlink as a magic
> string)
> 3) Support has been added for absolute symlinks
> 4) Support has been added for relative symlinks
> 5) Support has been added for file symlinks
> 6) Significantly more tests have been added and quirky delete
> behaviors identified and fixed
> 7) CreateSymbolicLink[A|W] is now implemented with
> DeviceIoControl(...,FSCTL_SET_REPARSE_POINT,...)
>
> Thank you in advance to those who look these patches over, your
> feedback is greatly appreciated.
>
> Best,
> Erich

FWIW, I ran this against winetricks-test (after rebasing the patches as I
mentioned on IRC); everything there still installs.