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

This is one step towards rectifying the mistake of using `FD_CLOEXEC` in
the first place. Eventually we may deprecate support for `FD_CLOEXEC`,
as the `O_*` flags better match the underlying `pipe()` API.

See discussion on
https://gitlab.gnome.org/GNOME/glib/-/merge_requests/3459#note_1779264

Signed-off-by: Philip Withnall <philip@tecnocode.co.uk>

2 files changed, 45 insertions, 8 deletions

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/tests/unix.c b/glib/tests/unix.c
index 90345eda0..12f4a609f 100644
--- a/glib/tests/unix.c
+++ b/glib/tests/unix.c
@@ -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;
@@ -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);