RFC: Junction Point/NT Symlink Support [2]

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

RFC: Junction Point/NT Symlink Support [2]

Erich E. Hoover-3
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-Junction_Points

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


Reply | Threaded
Open this post in threaded view
|

Re: RFC: Junction Point/NT Symlink Support [2]

Hans Leidekker-4
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?



Reply | Threaded
Open this post in threaded view
|

Re: RFC: Junction Point/NT Symlink Support [2]

Erich E. Hoover-3
On Fri, Apr 19, 2019 at 2:30 AM Hans Leidekker <[hidden email]> 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


Reply | Threaded
Open this post in threaded view
|

Re: RFC: Junction Point/NT Symlink Support [2]

Austin English-2
In reply to this post by Erich E. Hoover-3
On Wed, Apr 17, 2019 at 9:30 AM Erich E. Hoover <[hidden email]> 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-Junction_Points

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

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

--
-Austin
GPG: 267B CC1F 053F 0749 (expires 2021/02/18)