Re: [PATCH 5/9] save-cwd: don’t depend on fd-safer-flag, unistd-safer

[Paul Eggert]

On 2024-11-11 02:06, Bruno Haible wrote:
The test, intest-openat.c:94, is reasonable IMO: It verifies that the
openat() replacement for native Windows behaves the same as a POSIX-compliant
openat()....
What do you think? Should some module dependency be added to the 'openat'
module? Or to the 'save-cwd' module?

It's probably simpler to have the openat replacement return the lowest 
available fd as per POSIX. I installed the attached untested patch to 
try to do that, and we can see whether CI likes it.From 22f3766d77fef397e9b99b944afe773de413b9b3 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Mon, 11 Nov 2024 16:25:50 -0800
Subject: [PATCH] openat: port lowest-fd to native MS-Windows

Problem reported by Bruno Haible in:
https://lists.gnu.org/r/bug-gnulib/2024-11/msg00081.html
* lib/openat.c (openat_permissive): When save_cwd allocates an FD,
allocate another one DFD and then close FD so that the later
open returns FD (the lowest available fd), as POSIX requires.
---
 ChangeLog    |  9 +++++++++
 lib/openat.c | 33 ++++++++++++++++++++++-----------
 2 files changed, 31 insertions(+), 11 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 29e63351b5..348fd1d27d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2024-11-11  Paul Eggert  <egg...@cs.ucla.edu>
+
+	openat: port lowest-fd to native MS-Windows
+	Problem reported by Bruno Haible in:
+	https://lists.gnu.org/r/bug-gnulib/2024-11/msg00081.html
+	* lib/openat.c (openat_permissive): When save_cwd allocates an FD,
+	allocate another one DFD and then close FD so that the later
+	open returns FD (the lowest available fd), as POSIX requires.
+
 2024-11-11  Bruno Haible  <br...@clisp.org>
 
 	fts: Fix dependencies (regression 2024-11-06).
diff --git a/lib/openat.c b/lib/openat.c
index 65cfc5ede4..698874427b 100644
--- a/lib/openat.c
+++ b/lib/openat.c
@@ -220,7 +220,6 @@ openat_permissive (int fd, char const *file, int flags, mode_t mode,
   struct saved_cwd saved_cwd;
   int saved_errno;
   int err;
-  bool save_ok;
 
   if (fd == AT_FDCWD || IS_ABSOLUTE_FILE_NAME (file))
     return open (file, flags, mode);
@@ -245,23 +244,35 @@ openat_permissive (int fd, char const *file, int flags, mode_t mode,
       }
   }
 
-  save_ok = (save_cwd (&saved_cwd) == 0);
-  if (! save_ok)
+  bool save_failed = save_cwd (&saved_cwd) < 0;
+
+  /* If save_cwd allocated a descriptor DFD other than FD, do another
+     save_cwd and then close DFD, so that the later open (if successful)
+     returns DFD (the lowest-numbered descriptor) as POSIX requires.  */
+  int dfd = saved_cwd.desc;
+  if (0 <= dfd && dfd != fd)
     {
-      if (! cwd_errno)
-        openat_save_fail (errno);
-      *cwd_errno = errno;
+      save_failed = save_cwd (&saved_cwd) < 0;
+      close (dfd);
+      dfd = saved_cwd.desc;
     }
-  if (0 <= fd && fd == saved_cwd.desc)
+
+  /* If saving the working directory collides with the user's requested fd,
+     then the user's fd must have been closed to begin with.  */
+  if (0 <= dfd && dfd == fd)
     {
-      /* If saving the working directory collides with the user's
-         requested fd, then the user's fd must have been closed to
-         begin with.  */
       free_cwd (&saved_cwd);
       errno = EBADF;
       return -1;
     }
 
+  if (save_failed)
+    {
+      if (! cwd_errno)
+        openat_save_fail (errno);
+      *cwd_errno = errno;
+    }
+
   err = fchdir (fd);
   saved_errno = errno;
 
@@ -269,7 +280,7 @@ openat_permissive (int fd, char const *file, int flags, mode_t mode,
     {
       err = open (file, flags, mode);
       saved_errno = errno;
-      if (save_ok && restore_cwd (&saved_cwd) != 0)
+      if (!save_failed && restore_cwd (&saved_cwd) != 0)
         {
           if (! cwd_errno)
             {
-- 
2.43.0