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>
This commit is contained in:
Philip Withnall 2023-08-16 14:26:14 +01:00
parent de6cebb5f6
commit d0dc7171d6
2 changed files with 45 additions and 8 deletions

View File

@ -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);

View File

@ -57,6 +57,34 @@ test_pipe (void)
g_assert (g_str_has_prefix (buf, "hello"));
}
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)
{
@ -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);