* [PATCH] Cygwin: fchmodat: add limited support for AT_SYMLINK_NOFOLLOW
@ 2021-01-26 21:30 Ken Brown
2021-01-27 12:16 ` Ken Brown
2021-01-27 12:40 ` Corinna Vinschen
0 siblings, 2 replies; 6+ messages in thread
From: Ken Brown @ 2021-01-26 21:30 UTC (permalink / raw)
To: cygwin-patches
Allow fchmodat with the AT_SYMLINK_NOFOLLOW flag to succeed on
non-symlinks. Previously it always failed, as it does on Linux. But
POSIX permits it to succeed on non-symlinks even if it fails on
symlinks.
The reason for following POSIX rather than Linux is to make gnulib
report that fchmodat works on Cygwin. This improves the efficiency of
packages like GNU tar that use gnulib's fchmodat module. Previously
such packages would use a gnulib replacement for fchmodat on Cygwin.
---
winsup/cygwin/syscalls.cc | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 4cc8d07f5..0983cc76a 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -4787,17 +4787,27 @@ fchmodat (int dirfd, const char *pathname, mode_t mode, int flags)
tmp_pathbuf tp;
__try
{
- if (flags)
+ if (flags & ~AT_SYMLINK_NOFOLLOW)
{
- /* BSD has lchmod, but Linux does not. POSIX says
- AT_SYMLINK_NOFOLLOW is allowed to fail on symlinks; but Linux
- blindly fails even for non-symlinks. */
- set_errno ((flags & ~AT_SYMLINK_NOFOLLOW) ? EINVAL : EOPNOTSUPP);
+ set_errno (EINVAL);
__leave;
}
char *path = tp.c_get ();
if (gen_full_path_at (path, dirfd, pathname))
__leave;
+ if (flags)
+ {
+ /* BSD has lchmod, but Linux does not. POSIX says
+ AT_SYMLINK_NOFOLLOW is allowed to fail on symlinks.
+ Linux blindly fails even for non-symlinks, but we allow
+ it to succeed. */
+ path_conv pc (path);
+ if (pc.issymlink ())
+ {
+ set_errno (EOPNOTSUPP);
+ __leave;
+ }
+ }
return chmod (path, mode);
}
__except (EFAULT) {}
--
2.30.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Cygwin: fchmodat: add limited support for AT_SYMLINK_NOFOLLOW
2021-01-26 21:30 [PATCH] Cygwin: fchmodat: add limited support for AT_SYMLINK_NOFOLLOW Ken Brown
@ 2021-01-27 12:16 ` Ken Brown
2021-01-27 12:40 ` Corinna Vinschen
1 sibling, 0 replies; 6+ messages in thread
From: Ken Brown @ 2021-01-27 12:16 UTC (permalink / raw)
To: cygwin-patches
On 1/26/2021 4:30 PM, Ken Brown via Cygwin-patches wrote:
> Allow fchmodat with the AT_SYMLINK_NOFOLLOW flag to succeed on
> non-symlinks. Previously it always failed, as it does on Linux. But
> POSIX permits it to succeed on non-symlinks even if it fails on
> symlinks.
>
> The reason for following POSIX rather than Linux is to make gnulib
> report that fchmodat works on Cygwin. This improves the efficiency of
> packages like GNU tar that use gnulib's fchmodat module. Previously
> such packages would use a gnulib replacement for fchmodat on Cygwin.
> ---
> winsup/cygwin/syscalls.cc | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
> index 4cc8d07f5..0983cc76a 100644
> --- a/winsup/cygwin/syscalls.cc
> +++ b/winsup/cygwin/syscalls.cc
> @@ -4787,17 +4787,27 @@ fchmodat (int dirfd, const char *pathname, mode_t mode, int flags)
> tmp_pathbuf tp;
> __try
> {
> - if (flags)
> + if (flags & ~AT_SYMLINK_NOFOLLOW)
> {
> - /* BSD has lchmod, but Linux does not. POSIX says
> - AT_SYMLINK_NOFOLLOW is allowed to fail on symlinks; but Linux
> - blindly fails even for non-symlinks. */
> - set_errno ((flags & ~AT_SYMLINK_NOFOLLOW) ? EINVAL : EOPNOTSUPP);
> + set_errno (EINVAL);
> __leave;
> }
> char *path = tp.c_get ();
> if (gen_full_path_at (path, dirfd, pathname))
> __leave;
> + if (flags)
> + {
> + /* BSD has lchmod, but Linux does not. POSIX says
> + AT_SYMLINK_NOFOLLOW is allowed to fail on symlinks.
> + Linux blindly fails even for non-symlinks, but we allow
> + it to succeed. */
> + path_conv pc (path);
> + if (pc.issymlink ())
> + {
> + set_errno (EOPNOTSUPP);
> + __leave;
> + }
> + }
> return chmod (path, mode);
> }
> __except (EFAULT) {}
This is wrong. Please ignore. I'll send a revised patch.
Ken
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Cygwin: fchmodat: add limited support for AT_SYMLINK_NOFOLLOW
2021-01-26 21:30 [PATCH] Cygwin: fchmodat: add limited support for AT_SYMLINK_NOFOLLOW Ken Brown
2021-01-27 12:16 ` Ken Brown
@ 2021-01-27 12:40 ` Corinna Vinschen
2021-01-27 13:22 ` Ken Brown
1 sibling, 1 reply; 6+ messages in thread
From: Corinna Vinschen @ 2021-01-27 12:40 UTC (permalink / raw)
To: cygwin-patches
On Jan 26 16:30, Ken Brown via Cygwin-patches wrote:
> Allow fchmodat with the AT_SYMLINK_NOFOLLOW flag to succeed on
> non-symlinks. Previously it always failed, as it does on Linux. But
> POSIX permits it to succeed on non-symlinks even if it fails on
> symlinks.
>
> The reason for following POSIX rather than Linux is to make gnulib
> report that fchmodat works on Cygwin. This improves the efficiency of
> packages like GNU tar that use gnulib's fchmodat module. Previously
> such packages would use a gnulib replacement for fchmodat on Cygwin.
Wait, what? So if Cygwin behaves like Linux, gnulib treats fchmodat
as non-working? So what does gnulib do on a Linux system? Does it
use its own fchmodat there, too?
Puzzled,
Corinna
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Cygwin: fchmodat: add limited support for AT_SYMLINK_NOFOLLOW
2021-01-27 12:40 ` Corinna Vinschen
@ 2021-01-27 13:22 ` Ken Brown
2021-01-27 13:27 ` Corinna Vinschen
0 siblings, 1 reply; 6+ messages in thread
From: Ken Brown @ 2021-01-27 13:22 UTC (permalink / raw)
To: cygwin-patches
On 1/27/2021 7:40 AM, Corinna Vinschen via Cygwin-patches wrote:
> On Jan 26 16:30, Ken Brown via Cygwin-patches wrote:
>> Allow fchmodat with the AT_SYMLINK_NOFOLLOW flag to succeed on
>> non-symlinks. Previously it always failed, as it does on Linux. But
>> POSIX permits it to succeed on non-symlinks even if it fails on
>> symlinks.
>>
>> The reason for following POSIX rather than Linux is to make gnulib
>> report that fchmodat works on Cygwin. This improves the efficiency of
>> packages like GNU tar that use gnulib's fchmodat module. Previously
>> such packages would use a gnulib replacement for fchmodat on Cygwin.
>
> Wait, what? So if Cygwin behaves like Linux, gnulib treats fchmodat
> as non-working? So what does gnulib do on a Linux system? Does it
> use its own fchmodat there, too?
Apparently so. Here's a comment from gnulib's test program for fchmodat:
/* Test whether fchmodat+AT_SYMLINK_NOFOLLOW works on non-symlinks.
This test fails on GNU/Linux with glibc 2.31 (but not on
GNU/kFreeBSD nor GNU/Hurd) and Cygwin 2.9. */
I agree that it's strange.
Ken
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Cygwin: fchmodat: add limited support for AT_SYMLINK_NOFOLLOW
2021-01-27 13:22 ` Ken Brown
@ 2021-01-27 13:27 ` Corinna Vinschen
2021-01-27 17:26 ` Ken Brown
0 siblings, 1 reply; 6+ messages in thread
From: Corinna Vinschen @ 2021-01-27 13:27 UTC (permalink / raw)
To: cygwin-patches
On Jan 27 08:22, Ken Brown via Cygwin-patches wrote:
> On 1/27/2021 7:40 AM, Corinna Vinschen via Cygwin-patches wrote:
> > On Jan 26 16:30, Ken Brown via Cygwin-patches wrote:
> > > Allow fchmodat with the AT_SYMLINK_NOFOLLOW flag to succeed on
> > > non-symlinks. Previously it always failed, as it does on Linux. But
> > > POSIX permits it to succeed on non-symlinks even if it fails on
> > > symlinks.
> > >
> > > The reason for following POSIX rather than Linux is to make gnulib
> > > report that fchmodat works on Cygwin. This improves the efficiency of
> > > packages like GNU tar that use gnulib's fchmodat module. Previously
> > > such packages would use a gnulib replacement for fchmodat on Cygwin.
> >
> > Wait, what? So if Cygwin behaves like Linux, gnulib treats fchmodat
> > as non-working? So what does gnulib do on a Linux system? Does it
> > use its own fchmodat there, too?
>
> Apparently so. Here's a comment from gnulib's test program for fchmodat:
>
> /* Test whether fchmodat+AT_SYMLINK_NOFOLLOW works on non-symlinks.
> This test fails on GNU/Linux with glibc 2.31 (but not on
> GNU/kFreeBSD nor GNU/Hurd) and Cygwin 2.9. */
>
> I agree that it's strange.
¯\_(ツ)_/¯
Corinna
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Cygwin: fchmodat: add limited support for AT_SYMLINK_NOFOLLOW
2021-01-27 13:27 ` Corinna Vinschen
@ 2021-01-27 17:26 ` Ken Brown
0 siblings, 0 replies; 6+ messages in thread
From: Ken Brown @ 2021-01-27 17:26 UTC (permalink / raw)
To: cygwin-patches
On 1/27/2021 8:27 AM, Corinna Vinschen via Cygwin-patches wrote:
> On Jan 27 08:22, Ken Brown via Cygwin-patches wrote:
>> On 1/27/2021 7:40 AM, Corinna Vinschen via Cygwin-patches wrote:
>>> On Jan 26 16:30, Ken Brown via Cygwin-patches wrote:
>>>> Allow fchmodat with the AT_SYMLINK_NOFOLLOW flag to succeed on
>>>> non-symlinks. Previously it always failed, as it does on Linux. But
>>>> POSIX permits it to succeed on non-symlinks even if it fails on
>>>> symlinks.
>>>>
>>>> The reason for following POSIX rather than Linux is to make gnulib
>>>> report that fchmodat works on Cygwin. This improves the efficiency of
>>>> packages like GNU tar that use gnulib's fchmodat module. Previously
>>>> such packages would use a gnulib replacement for fchmodat on Cygwin.
>>>
>>> Wait, what? So if Cygwin behaves like Linux, gnulib treats fchmodat
>>> as non-working? So what does gnulib do on a Linux system? Does it
>>> use its own fchmodat there, too?
>>
>> Apparently so. Here's a comment from gnulib's test program for fchmodat:
>>
>> /* Test whether fchmodat+AT_SYMLINK_NOFOLLOW works on non-symlinks.
>> This test fails on GNU/Linux with glibc 2.31 (but not on
>> GNU/kFreeBSD nor GNU/Hurd) and Cygwin 2.9. */
>>
>> I agree that it's strange.
>
> ¯\_(ツ)_/¯
I'll go ahead and submit a revised patch for the record, and you can decide
whether you want to deviate from Linux. My own opinion is that it can't be bad
to support a flag that Linux doesn't support, but I don't feel strongly about it.
BTW, the mistake in the first version of the patch is that I forgot to specify
PC_SYM_NOFOLLOW in the path_conv constructor.
Ken
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-01-27 17:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-26 21:30 [PATCH] Cygwin: fchmodat: add limited support for AT_SYMLINK_NOFOLLOW Ken Brown
2021-01-27 12:16 ` Ken Brown
2021-01-27 12:40 ` Corinna Vinschen
2021-01-27 13:22 ` Ken Brown
2021-01-27 13:27 ` Corinna Vinschen
2021-01-27 17:26 ` Ken Brown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).