Merge branch 'o-nonblock-more' into 'main'

glib-unix: Accept O_CLOEXEC as well as FD_CLOEXEC in g_unix_open_pipe()

See merge request GNOME/glib!3538

10 files changed, 60 insertions, 23 deletions

diff --git a/gio/gfile.c b/gio/gfile.c
index 0e6d4c819..16ff09fe3 100644
--- a/gio/gfile.c
+++ b/gio/gfile.c
@@ -3188,7 +3188,7 @@ splice_stream_with_progress (GInputStream           *in,
   fd_in = g_file_descriptor_based_get_fd (G_FILE_DESCRIPTOR_BASED (in));
   fd_out = g_file_descriptor_based_get_fd (G_FILE_DESCRIPTOR_BASED (out));
 
-  if (!g_unix_open_pipe (buffer, FD_CLOEXEC, error))
+  if (!g_unix_open_pipe (buffer, O_CLOEXEC, error))
     return FALSE;
 
   /* Try a 1MiB buffer for improved throughput. If that fails, use the default
diff --git a/gio/gtestdbus.c b/gio/gtestdbus.c
index 9ff74e653..d75b4395a 100644
--- a/gio/gtestdbus.c
+++ b/gio/gtestdbus.c
@@ -604,7 +604,7 @@ make_pipe (gint     pipe_fds[2],
            GError **error)
 {
 #if defined(G_OS_UNIX)
-  return g_unix_open_pipe (pipe_fds, FD_CLOEXEC, error);
+  return g_unix_open_pipe (pipe_fds, O_CLOEXEC, error);
 #elif defined(G_OS_WIN32)
   if (_pipe (pipe_fds, 4096, _O_BINARY) < 0)
     {
diff --git a/gio/tests/gdbus-close-pending.c b/gio/tests/gdbus-close-pending.c
index 7d4d6c9ab..61f1faa79 100644
--- a/gio/tests/gdbus-close-pending.c
+++ b/gio/tests/gdbus-close-pending.c
@@ -306,9 +306,9 @@ test_once (Fixture       *f,
   GDBusMessage *message;
   gboolean pipe_res;
 
-  pipe_res = g_unix_open_pipe (f->server_to_client, FD_CLOEXEC, &f->error);
+  pipe_res = g_unix_open_pipe (f->server_to_client, O_CLOEXEC, &f->error);
   g_assert (pipe_res);
-  pipe_res = g_unix_open_pipe (f->client_to_server, FD_CLOEXEC, &f->error);
+  pipe_res = g_unix_open_pipe (f->client_to_server, O_CLOEXEC, &f->error);
   g_assert (pipe_res);
 
   f->server_iostream = my_io_stream_new_for_fds (f->client_to_server[0],
diff --git a/gio/tests/gsubprocess.c b/gio/tests/gsubprocess.c
index 695012c7d..45fdf93b2 100644
--- a/gio/tests/gsubprocess.c
+++ b/gio/tests/gsubprocess.c
@@ -1750,9 +1750,9 @@ do_test_pass_fd (GSubprocessFlags     flags,
   char *basic_fd_str;
   char *needdup_fd_str;
 
-  g_unix_open_pipe (basic_pipefds, FD_CLOEXEC, error);
+  g_unix_open_pipe (basic_pipefds, O_CLOEXEC, error);
   g_assert_no_error (local_error);
-  g_unix_open_pipe (needdup_pipefds, FD_CLOEXEC, error);
+  g_unix_open_pipe (needdup_pipefds, O_CLOEXEC, error);
   g_assert_no_error (local_error);
 
   basic_fd_str = g_strdup_printf ("%d", basic_pipefds[1]);
@@ -1854,10 +1854,10 @@ do_test_fd_conflation (GSubprocessFlags     flags,
       return;
     }
 
-  g_unix_open_pipe (unused_pipefds, FD_CLOEXEC, &error);
+  g_unix_open_pipe (unused_pipefds, O_CLOEXEC, &error);
   g_assert_no_error (error);
 
-  g_unix_open_pipe (pipefds, FD_CLOEXEC, &error);
+  g_unix_open_pipe (pipefds, O_CLOEXEC, &error);
   g_assert_no_error (error);
 
   /* The fds should be sequential since we are in a new process. */
diff --git a/gio/tests/unix-fd.c b/gio/tests/unix-fd.c
index ecf1aea37..62c17df14 100644
--- a/gio/tests/unix-fd.c
+++ b/gio/tests/unix-fd.c
@@ -63,7 +63,7 @@ test_fd_list (void)
   s = _pipe (sv, 4096, _O_NOINHERIT | _O_BINARY);
   g_assert_cmpint (s, ==, 0);
 #else
-  g_unix_open_pipe (sv, FD_CLOEXEC, &err);
+  g_unix_open_pipe (sv, O_CLOEXEC, &err);
   g_assert_no_error (err);
 #endif
   list = g_unix_fd_list_new_from_array (sv, -1);
diff --git a/glib/glib-unix.c b/glib/glib-unix.c
index 0532a500c..df5726cf0 100644
--- a/glib/glib-unix.c
+++ b/glib/glib-unix.c
@@ -82,15 +82,18 @@ g_unix_set_error_from_errno (GError **error,
  * uses the pipe2() system call, which atomically creates a pipe with
  * the configured flags.
  *
- * As of GLib 2.78, the supported flags are `FD_CLOEXEC` and `O_NONBLOCK`. Prior
- * to GLib 2.78, only `FD_CLOEXEC` was supported — if you wanted to configure
- * `O_NONBLOCK` then that had to be done separately with `fcntl()`.
+ * As of GLib 2.78, the supported flags are `O_CLOEXEC`/`FD_CLOEXEC` (see below)
+ * and `O_NONBLOCK`. Prior to GLib 2.78, only `FD_CLOEXEC` was supported — if
+ * you wanted to configure `O_NONBLOCK` then that had to be done separately with
+ * `fcntl()`.
  *
  * It is a programmer error to call this function with unsupported flags, and a
  * critical warning will be raised.
  *
- * This function does not take `O_CLOEXEC`, it takes `FD_CLOEXEC` as if
- * for fcntl(); these are different on Linux/glibc.
+ * As of GLib 2.78, it is preferred to pass `O_CLOEXEC` in, rather than
+ * `FD_CLOEXEC`, as that matches the underlying `pipe()` API more closely. Prior
+ * to 2.78, only `FD_CLOEXEC` was supported. Support for `FD_CLOEXEC` may be
+ * deprecated and removed in future.
  *
  * Returns: %TRUE on success, %FALSE if not (and errno will be set).
  *
@@ -101,11 +104,16 @@ g_unix_open_pipe (int     *fds,
                   int      flags,
                   GError **error)
 {
-  /* We only support FD_CLOEXEC and O_NONBLOCK */
-  g_return_val_if_fail ((flags & (FD_CLOEXEC | O_NONBLOCK)) == flags, FALSE);
+  /* We only support O_CLOEXEC/FD_CLOEXEC and O_NONBLOCK */
+  g_return_val_if_fail ((flags & (O_CLOEXEC | FD_CLOEXEC | O_NONBLOCK)) == flags, FALSE);
+
+#if O_CLOEXEC != FD_CLOEXEC && !defined(G_DISABLE_CHECKS)
+  if (flags & FD_CLOEXEC)
+    g_debug ("g_unix_open_pipe() called with FD_CLOEXEC; please migrate to using O_CLOEXEC instead");
+#endif
 
   if (!g_unix_open_pipe_internal (fds,
-                                  (flags & FD_CLOEXEC) != 0,
+                                  (flags & (O_CLOEXEC | FD_CLOEXEC)) != 0,
                                   (flags & O_NONBLOCK) != 0))
     return g_unix_set_error_from_errno (error, errno);
 
diff --git a/glib/gtester.c b/glib/gtester.c
index af1f29164..24f84f6c9 100644
--- a/glib/gtester.c
+++ b/glib/gtester.c
@@ -305,7 +305,7 @@ launch_test_binary (const char *binary,
   gboolean loop_pending;
   gint i = 0;
 
-  if (!g_unix_open_pipe (report_pipe, FD_CLOEXEC, &error))
+  if (!g_unix_open_pipe (report_pipe, O_CLOEXEC, &error))
     {
       if (subtest_mode_fatal)
         g_error ("Failed to open pipe for test binary: %s: %s", binary, error->message);
diff --git a/glib/gwakeup.c b/glib/gwakeup.c
index a3283a3ff..1674304f0 100644
--- a/glib/gwakeup.c
+++ b/glib/gwakeup.c
@@ -160,7 +160,7 @@ g_wakeup_new (void)
   /* for any failure, try a pipe instead */
 #endif
 
-  if (!g_unix_open_pipe (wakeup->fds, FD_CLOEXEC | O_NONBLOCK, &error))
+  if (!g_unix_open_pipe (wakeup->fds, O_CLOEXEC | O_NONBLOCK, &error))
     g_error ("Creating pipes for GWakeup: %s", error->message);
 
   if (!g_unix_set_fd_nonblocking (wakeup->fds[0], TRUE, &error) ||
diff --git a/glib/tests/spawn-singlethread.c b/glib/tests/spawn-singlethread.c
index 8d9ddef5f..33b44d650 100644
--- a/glib/tests/spawn-singlethread.c
+++ b/glib/tests/spawn-singlethread.c
@@ -237,7 +237,7 @@ test_spawn_async_with_fds (void)
               break;
             case PIPE:
 #ifdef G_OS_UNIX
-              g_unix_open_pipe (test_pipe[j], FD_CLOEXEC, &error);
+              g_unix_open_pipe (test_pipe[j], O_CLOEXEC, &error);
               g_assert_no_error (error);
 #else
               g_assert_cmpint (_pipe (test_pipe[j], 4096, _O_BINARY), >=, 0);
diff --git a/glib/tests/unix.c b/glib/tests/unix.c
index 90345eda0..ab6efaa76 100644
--- a/glib/tests/unix.c
+++ b/glib/tests/unix.c
@@ -41,7 +41,7 @@ test_pipe (void)
   gssize bytes_read;
   gboolean res;
 
-  res = g_unix_open_pipe (pipefd, FD_CLOEXEC, &error);
+  res = g_unix_open_pipe (pipefd, O_CLOEXEC, &error);
   g_assert (res);
   g_assert_no_error (error);
 
@@ -58,6 +58,34 @@ test_pipe (void)
 }
 
 static void
+test_pipe_fd_cloexec (void)
+{
+  GError *error = NULL;
+  int pipefd[2];
+  char buf[1024];
+  gssize bytes_read;
+  gboolean res;
+
+  g_test_summary ("Test that FD_CLOEXEC is still accepted as an argument to g_unix_open_pipe()");
+  g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/merge_requests/3459");
+
+  res = g_unix_open_pipe (pipefd, FD_CLOEXEC, &error);
+  g_assert (res);
+  g_assert_no_error (error);
+
+  g_assert_cmpint (write (pipefd[1], "hello", sizeof ("hello")), ==, sizeof ("hello"));
+  memset (buf, 0, sizeof (buf));
+  bytes_read = read (pipefd[0], buf, sizeof(buf) - 1);
+  g_assert_cmpint (bytes_read, >, 0);
+  buf[bytes_read] = '\0';
+
+  close (pipefd[0]);
+  close (pipefd[1]);
+
+  g_assert_true (g_str_has_prefix (buf, "hello"));
+}
+
+static void
 test_pipe_stdio_overwrite (void)
 {
   GError *error = NULL;
@@ -75,7 +103,7 @@ test_pipe_stdio_overwrite (void)
   g_close (STDIN_FILENO, &error);
   g_assert_no_error (error);
 
-  res = g_unix_open_pipe (pipefd, FD_CLOEXEC, &error);
+  res = g_unix_open_pipe (pipefd, O_CLOEXEC, &error);
   g_assert_no_error (error);
   g_assert_true (res);
 
@@ -115,7 +143,7 @@ test_nonblocking (void)
   gboolean res;
   int flags;
 
-  res = g_unix_open_pipe (pipefd, FD_CLOEXEC, &error);
+  res = g_unix_open_pipe (pipefd, O_CLOEXEC, &error);
   g_assert (res);
   g_assert_no_error (error);
 
@@ -472,6 +500,7 @@ main (int   argc,
   g_test_init (&argc, &argv, NULL);
 
   g_test_add_func ("/glib-unix/pipe", test_pipe);
+  g_test_add_func ("/glib-unix/pipe/fd-cloexec", test_pipe_fd_cloexec);
   g_test_add_func ("/glib-unix/pipe-stdio-overwrite", test_pipe_stdio_overwrite);
   g_test_add_func ("/glib-unix/error", test_error);
   g_test_add_func ("/glib-unix/nonblocking", test_nonblocking);