Merge branch 'wip/smcv/autofd' into 'main'

gstdio: Preserve errno in g_autofd, document async-signal safety

See merge request GNOME/glib!3027

glib/gstdio.c

@@ -1845,6 +1845,13 @@ g_close (gint       fd,
  * anything.
  * In both cases, set @fd_ptr to `-1` before returning.
  *
+ * Like g_close(), if closing the file descriptor fails, the error is
+ * stored in both %errno and @error. If this function succeeds,
+ * %errno is undefined.
+ *
+ * This function is async-signal-safe if @error is %NULL and @fd_ptr
+ * points to either a negative number or a valid file descriptor.
+ *
  * It is a programming error for @fd_ptr to point to a non-negative
  * number that is not a valid file descriptor.
  *
@@ -1898,6 +1905,9 @@ g_close (gint       fd,
  * only supported on GCC and clang, and the variable must be initialized
  * (to either a valid file descriptor or a negative number).
  *
+ * Using this macro is async-signal-safe if the constraints described above
+ * are met, so it can be used in a signal handler or after `fork()`.
+ *
  * Any error from closing the file descriptor when it goes out of scope
  * is ignored. Use g_clear_fd() if error-checking is required.
  *

glib/gstdio.h

@@ -23,6 +23,7 @@
 
 #include <glib/gprintf.h>
 
+#include <errno.h>
 #include <sys/stat.h>
 
 G_BEGIN_DECLS
@@ -199,19 +200,28 @@ g_clear_fd (int     *fd_ptr,
 /* g_autofd should be defined on the same compilers where g_autofree is
  * This avoids duplicating the feature-detection here. */
 #ifdef g_autofree
+#ifndef __GTK_DOC_IGNORE__
 /* Not public API */
 static inline void
 _g_clear_fd_ignore_error (int *fd_ptr)
 {
+  /* Don't overwrite thread-local errno if closing the fd fails */
+  int errsv = errno;
+
   /* Suppress "Not available before" warning */
   G_GNUC_BEGIN_IGNORE_DEPRECATIONS
+
   if (!g_clear_fd (fd_ptr, NULL))
     {
       /* Do nothing: we ignore all errors, except for EBADF which
        * is a programming error, checked for by g_close(). */
     }
+
   G_GNUC_END_IGNORE_DEPRECATIONS
+
+  errno = errsv;
 }
+#endif
 
 #define g_autofd _GLIB_CLEANUP(_g_clear_fd_ignore_error) GLIB_AVAILABLE_MACRO_IN_2_76
 #endif

glib/tests/fileutils.c

@@ -2470,6 +2470,64 @@ assert_fd_was_closed (int fd)
     }
 }
 
+static void
+test_clear_fd_ebadf (void)
+{
+  char *name = NULL;
+  GError *error = NULL;
+  int fd;
+  int copy_of_fd;
+  int errsv;
+  gboolean ret;
+
+  /* We're going to trigger a programming error: attmpting to close a
+   * fd that was already closed. Make criticals non-fatal. */
+  g_assert_true (g_test_undefined ());
+  g_log_set_always_fatal (G_LOG_FATAL_MASK);
+  g_log_set_fatal_mask ("GLib", G_LOG_FATAL_MASK);
+
+  fd = g_file_open_tmp (NULL, &name, &error);
+  g_assert_cmpint (fd, !=, -1);
+  g_assert_no_error (error);
+  g_assert_nonnull (name);
+  ret = g_close (fd, &error);
+  g_assert_no_error (error);
+  assert_fd_was_closed (fd);
+  g_assert_true (ret);
+  g_unlink (name);
+  g_free (name);
+
+  /* Try to close it again with g_close() */
+  ret = g_close (fd, NULL);
+  errsv = errno;
+  g_assert_cmpint (errsv, ==, EBADF);
+  assert_fd_was_closed (fd);
+  g_assert_false (ret);
+
+  /* Try to close it again with g_clear_fd() */
+  copy_of_fd = fd;
+  errno = EILSEQ;
+  ret = g_clear_fd (&copy_of_fd, NULL);
+  errsv = errno;
+  g_assert_cmpint (errsv, ==, EBADF);
+  assert_fd_was_closed (fd);
+  g_assert_false (ret);
+
+#ifdef g_autofree
+    {
+      g_autofd int close_me = fd;
+
+      /* This avoids clang warnings about the variables being unused */
+      g_test_message ("Invalid fd will be closed by autocleanup: %d",
+                      close_me);
+      errno = EILSEQ;
+    }
+
+  errsv = errno;
+  g_assert_cmpint (errsv, ==, EILSEQ);
+#endif
+}
+
 static void
 test_clear_fd (void)
 {
@@ -2477,6 +2535,7 @@ test_clear_fd (void)
   GError *error = NULL;
   int fd;
   int copy_of_fd;
+  int errsv;
 
 #ifdef g_autofree
   g_test_summary ("Test g_clear_fd() and g_autofd");
@@ -2522,12 +2581,31 @@ test_clear_fd (void)
       /* This avoids clang warnings about the variables being unused */
       g_test_message ("Will be closed by autocleanup: %d, %d",
                       close_me, was_never_set);
+      /* This is one of the few errno values guaranteed by Standard C.
+       * We set it here to check that a successful g_autofd close doesn't
+       * alter errno. */
+      errno = EILSEQ;
     }
 
+  errsv = errno;
+  g_assert_cmpint (errsv, ==, EILSEQ);
   assert_fd_was_closed (fd);
   g_unlink (name);
   g_free (name);
 #endif
+
+  if (g_test_undefined ())
+    {
+      g_test_message ("Testing error handling");
+      g_test_trap_subprocess ("/fileutils/clear-fd/subprocess/ebadf",
+                              0, G_TEST_SUBPROCESS_DEFAULT);
+#ifdef g_autofree
+      g_test_trap_assert_stderr ("*failed with EBADF*failed with EBADF*failed with EBADF*");
+#else
+      g_test_trap_assert_stderr ("*failed with EBADF*failed with EBADF*");
+#endif
+      g_test_trap_assert_passed ();
+    }
 }
 
 int
@@ -2567,6 +2645,7 @@ main (int   argc,
   g_test_add_func ("/fileutils/stdio-wrappers", test_stdio_wrappers);
   g_test_add_func ("/fileutils/fopen-modes", test_fopen_modes);
   g_test_add_func ("/fileutils/clear-fd", test_clear_fd);
+  g_test_add_func ("/fileutils/clear-fd/subprocess/ebadf", test_clear_fd_ebadf);
 
   return g_test_run ();
 }