DMARC-Filter: OpenDMARC Filter v1.4.2 delorie.com 52UCodga1264069
Date: Sun, 30 Mar 2025 14:50:08 +0200
To: Paul Eggert <eggert@cs.ucla.edu>
Cc: =?utf-8?Q?P=C3=A1draig?= Brady <P@draigbrady.com>,
        Bruno Haible <bruno@clisp.org>, bug-gnulib@gnu.org, cygwin@cygwin.com,
        Coreutils <coreutils@gnu.org>
Subject: Re: symbolic link curiousity in 3.6.0
Message-ID: <Z-k-ALeeYtJx7SqL@calimero.vinschen.de>
Mail-Followup-To: Paul Eggert <eggert@cs.ucla.edu>,
 =?utf-8?Q?P=C3=A1draig?= Brady <P@draigbrady.com>,
 Bruno Haible <bruno@clisp.org>, bug-gnulib@gnu.org,
 cygwin@cygwin.com, Coreutils <coreutils@gnu.org>
References: <Pine.BSF.4.63.2503250218240.74063@m0.truegem.net>
 <Z-aGVCUo0N1VSl5s@calimero.vinschen.de>
 <Z-aP1jhjXTUVvP-E@calimero.vinschen.de> <11037686.3WhfQktd6Z@nimes>
 <91c9d441-36e3-4dd5-b2ca-3cfd498d2260@draigBrady.com>
 <Z-fLulclFs13NfAm@calimero.vinschen.de>
 <a78f800c-0463-4efb-b431-c2c244bd13c7@cs.ucla.edu>
MIME-Version: 1.0
Content-Disposition: inline
In-Reply-To: <a78f800c-0463-4efb-b431-c2c244bd13c7@cs.ucla.edu>
Precedence: list
From: Corinna Vinschen via Cygwin <cygwin@cygwin.com>
Reply-To: Corinna Vinschen <corinna-cygwin@cygwin.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: cygwin-bounces~archive-cygwin=delorie.com@cygwin.com
Sender: "Cygwin" <cygwin-bounces~archive-cygwin=delorie.com@cygwin.com>

Hi Paul,

thanks for the patch.

On Mar 29 10:31, Paul Eggert via Cygwin wrote:
> On 3/29/25 04:30, Corinna Vinschen wrote:
> > What it should do if only the POSIX.1e draft 17 functions are available
> > is something along these lines:
> 
> Yes, that sounds like a better approach. However, shouldn't it use O_PATH
> not O_RDONLY? We might lack read access.
> 
> Does the attached Gnulib patch work for you? I haven't tested or installed
> it (I don't use Cygwin).

> From e245ab6ac865c7ff723837645886eb717c53a754 Mon Sep 17 00:00:00 2001
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sat, 29 Mar 2025 10:27:01 -0600
> Subject: [PATCH] file-has-acl: port symlink code to Cygwin
> 
> Problem reported by Corinna Vinschen in:
> https://lists.gnu.org/r/bug-gnulib/2025-03/msg00112.html
> * lib/file-has-acl.c (acl_get_link_np): New static function,
> defined only if needed.
> (HAVE_ACL_GET_LINK_NP): Define this if defining acl_get_link_np.
> ---
>  ChangeLog          |  9 +++++++++
>  lib/file-has-acl.c | 21 ++++++++++++++++++++-
>  2 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/ChangeLog b/ChangeLog
> index 58195260cf..a7fa40dc33 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,12 @@
> +2025-03-29  Paul Eggert  <eggert@cs.ucla.edu>
> +
> +	file-has-acl: port symlink code to Cygwin
> +	Problem reported by Corinna Vinschen in:
> +	https://lists.gnu.org/r/bug-gnulib/2025-03/msg00112.html
> +	* lib/file-has-acl.c (acl_get_link_np): New static function,
> +	defined only if needed.
> +	(HAVE_ACL_GET_LINK_NP): Define this if defining acl_get_link_np.
> +
>  2025-03-29  Bruno Haible  <bruno@clisp.org>
>  
>  	acl-permissions: Update comments regarding NetBSD.
> diff --git a/lib/file-has-acl.c b/lib/file-has-acl.c
> index 179e805bd4..2538b61944 100644
> --- a/lib/file-has-acl.c
> +++ b/lib/file-has-acl.c
> @@ -362,6 +362,25 @@ acl_nfs4_nontrivial (uint32_t *xattr, ssize_t nbytes)
>  }
>  #endif
>  
> +#if (!USE_LINUX_XATTR && USE_ACL && HAVE_ACL_GET_FD \
> +     && !HAVE_ACL_EXTENDED_FILE && !HAVE_ACL_TYPE_EXTENDED \
> +     && !HAVE_ACL_GET_LINK_NP && defined O_PATH)

The definition of O_PATH requires an additional

  #include <fcntl.h>

Adding that to gllib/file-has-acl.c, the patch works as desired on
Cygwin.

However, assuming not only Cygwin is affected, shouldn't the patch
rather use O_PATH if it's available, O_RDONLY if not?

> -#    if HAVE_ACL_GET_LINK_NP /* FreeBSD, NetBSD >= 10 */
> +#    if HAVE_ACL_GET_LINK_NP /* FreeBSD, NetBSD >= 10, Cygwin >= 2.5 */

Changing the comment is wrong, IMHO.  We're adding a local version of
HAVE_ACL_GET_LINK_NP specificially because Cygwin (or, FWIW, any system
only providing the POSIX.1e draft 17 function) does not provide
acl_get_link_np, isn't it?

For Cygwin, I will add this function nevertheless, but it will only be
available in some upcoming version, either 3.6.1 or 3.7.0.

In terms of coreutils, I think either ls(1) gobble_file() or
file_has_aclinfo_cache() should still handle ENOENT from
file_has_aclinfo() and not print any error message.  After all, due to
the preconditions for building acl_get_link_np, we can't be sure
acl_get_link_np has really been built into file-has-acl.c, and the
problem persists.

>      if (! (flags & ACL_SYMLINK_FOLLOW))
>        acl_get_file_or_link = acl_get_link_np;
>  #    endif


Thanks,
Corinna

-- 
Problem reports:      https://cygwin.com/problems.html
FAQ:                  https://cygwin.com/faq/
Documentation:        https://cygwin.com/docs.html
Unsubscribe info:     https://cygwin.com/ml/#unsubscribe-simple