On 30/03/2025 13:50, Corinna Vinschen wrote:
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 <egg...@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  <egg...@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  <br...@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.

I tend to agree. I'll apply this later:


commit 88385a0d6d56197d3c180432c8a4bca07241e90b (HEAD -> master)
Author: Pádraig Brady <p...@draigbrady.com>
Date:   Sun Mar 30 15:16:54 2025 +0100

    ls: suppress ENOENT errors when reading ACL info

    * src/ls.c (gobble_file): Indicating unknown ACL info with '?'
    suffices for the edge case of a file being removed while reading,
    or older cygwin when reading through dangling symlinks.
    Reported by Corinna Vinschen.

diff --git a/src/ls.c b/src/ls.c
index 46ec42037..2cf4ee444 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -3538,8 +3538,14 @@ gobble_file (char const *name, enum filetype type, ino_t 
inode,

       /* Let the user know via '?' if errno is EACCES, which can
          happen with Linux kernel 6.12 on an NFS file system.
-         That's better than a longwinded diagnostic.  */
-      bool cannot_access_acl = n < 0 && errno == EACCES;
+         That's better than a longwinded diagnostic.
+
+         Similarly, ignore ENOENT which may happen on some versions
+         of cygwin when processing dangling symlinks for example.
+         Also if a file is removed while we're reading ACL info,
+         ACL_T_UNKNOWN is sufficient indication for that edge case.  */
+      bool cannot_access_acl = n < 0
+           && (errno == EACCES || errno == ENOENT);

       f->acl_type = (!have_scontext && !have_acl
                      ? (cannot_access_acl ? ACL_T_UNKNOWN : ACL_T_NONE)

cheers,
Pádraig.

--
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

Reply via email to