From 1c391d734c2ed554dca2b66358ddeceb60149663 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 5 Mar 2026 13:20:41 +0000 Subject: [PATCH 1/3] backports, local-alloc: Provide a backport of g_autofd The functionality that was prototyped in libglnx as glnx_fd_close and then glnx_autofd was later added to GLib as g_autofd. glnx_close_fd() doesn't have a direct equivalent in GLib, so keep it intact, but using the backported _glnx_clear_fd_ignore_error as its implementation. g_clear_fd() is the closest thing in GLib, but g_clear_fd() guarantees to set errno on failure (making it useful for error-checking), whereas glnx_close_fd() guarantees to leave errno untouched, making it more useful for cleanup code paths that recover from a syscall or similar function that sets errno: int fd = ...; success = (fsync (fd) == 0); glnx_close_fd (&fd); return success; /* if false, errno indicates why fsync failed */ (Of course in many cases, including this simple example, it would have been easier to use g_autofd.) glnx_autofd and glnx_fd_close are now equivalent to the backport of g_autofd, so document them as deprecated. There's essentially no cost to retaining them, so don't apply deprecation attributes. Signed-off-by: Simon McVittie --- glnx-backports.h | 21 +++++++++++++++++++++ glnx-local-alloc.h | 31 ++++++++----------------------- 2 files changed, 29 insertions(+), 23 deletions(-) diff --git a/glnx-backports.h b/glnx-backports.h index 9ec11e4cf..a0177ad99 100644 --- a/glnx-backports.h +++ b/glnx-backports.h @@ -82,6 +82,27 @@ g_clear_fd (int *fd_ptr, } #endif +/* This is part of the backport of g_autofd, but we define it + * unconditionally because it's also used to implement glnx_close_fd() */ +static inline void +_glnx_clear_fd_ignore_error (int *fd_ptr) +{ + /* Don't overwrite thread-local errno if closing the fd fails */ + int errsv = errno; + + 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(). */ + } + + errno = errsv; +} + +#if !GLIB_CHECK_VERSION(2, 76, 0) +#define g_autofd __attribute__((cleanup(_glnx_clear_fd_ignore_error))) +#endif + #if !GLIB_CHECK_VERSION(2, 40, 0) #define g_info(...) g_log (G_LOG_DOMAIN, G_LOG_LEVEL_INFO, __VA_ARGS__) #endif diff --git a/glnx-local-alloc.h b/glnx-local-alloc.h index a6e0e9b4f..9ac8a027d 100644 --- a/glnx-local-alloc.h +++ b/glnx-local-alloc.h @@ -51,38 +51,23 @@ glnx_local_obj_unref (void *v) * glnx_close_fd: * @fdp: Pointer to fd * - * Effectively `close (g_steal_fd (&fd))`. Also - * asserts that `close()` did not raise `EBADF` - encountering - * that error is usually a critical bug in the program. + * Same as `g_clear_fd()`, but ignoring the error (if any) and making sure + * not to alter `errno`. As a result, this function can be used for cleanup + * in contexts where `errno` needs to be preserved. */ -static inline void -glnx_close_fd (int *fdp) -{ - int errsv; - - g_assert (fdp); - - int fd = g_steal_fd (fdp); - if (fd >= 0) - { - errsv = errno; - if (close (fd) < 0) - g_assert (errno != EBADF); - errno = errsv; - } -} +#define glnx_close_fd _glnx_clear_fd_ignore_error /** * glnx_fd_close: * - * Deprecated in favor of `glnx_autofd`. + * Deprecated in favor of `g_autofd`. */ -#define glnx_fd_close __attribute__((cleanup(glnx_close_fd))) +#define glnx_fd_close g_autofd /** * glnx_autofd: * - * Call close() on a variable location when it goes out of scope. + * Deprecated in favor of `g_autofd`. */ -#define glnx_autofd __attribute__((cleanup(glnx_close_fd))) +#define glnx_autofd g_autofd G_END_DECLS From 606dbdbc200bcc88d66edf82b25322bb53949549 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 5 Mar 2026 16:01:43 +0000 Subject: [PATCH 2/3] testlib: Add an assertion that a fd has really been closed We can't easily assert this without triggering warnings from tools like valgrind by doing an invalid operation on a closed fd, so we only check this when under `-m undefined`. Originally contributed to GLib 2.76 in GNOME/glib@b3934133 "gstdio: Add g_clear_fd() and g_autofd". The implementation in GLib used g_fsync() as a portable thing that we can do with a fd, but that function is newer than our minimum GLib version, and libglnx isn't portable to non-Unix anyway, so use fnctl() instead. Signed-off-by: Simon McVittie --- tests/libglnx-testlib.c | 19 +++++++++++++++++++ tests/libglnx-testlib.h | 5 ++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/tests/libglnx-testlib.c b/tests/libglnx-testlib.c index 3eb2ba143..22c38981d 100644 --- a/tests/libglnx-testlib.c +++ b/tests/libglnx-testlib.c @@ -23,6 +23,7 @@ #include "libglnx-testlib.h" #include +#include #include @@ -72,3 +73,21 @@ _glnx_test_auto_temp_dir_leave (_GLnxTestAutoTempDir *dir) g_free (dir->old_cwd); g_free (dir); } + +void +_glnx_test_assert_fd_was_closed (int fd) +{ + /* We can't tell a fd was really closed without behaving as though it + * was still valid */ + if (g_test_undefined ()) + { + int result; + int errsv; + + errno = 0; + result = fcntl (fd, F_GETFD, 0); + errsv = errno; + g_assert_cmpint (result, <, 0); + g_assert_cmpint (errsv, ==, EBADF); + } +} diff --git a/tests/libglnx-testlib.h b/tests/libglnx-testlib.h index dccc7e558..009de74b8 100644 --- a/tests/libglnx-testlib.h +++ b/tests/libglnx-testlib.h @@ -1,7 +1,7 @@ /* -*- mode: C; c-file-style: "gnu"; indent-tabs-mode: nil; -*- * * Copyright (C) 2017 Red Hat, Inc. - * Copyright 2019 Collabora Ltd. + * Copyright 2019-2022 Collabora Ltd. * SPDX-License-Identifier: LGPL-2.0-or-later * * This library is free software; you can redistribute it and/or @@ -23,6 +23,7 @@ #pragma once #include +#include #include "glnx-backport-autoptr.h" @@ -47,3 +48,5 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(_GLnxTestAutoTempDir, _glnx_test_auto_temp_dir_lea #define _GLNX_TEST_SCOPED_TEMP_DIR \ G_GNUC_UNUSED g_autoptr(_GLnxTestAutoTempDir) temp_dir = _glnx_test_auto_temp_dir_enter () + +void _glnx_test_assert_fd_was_closed (int fd); From b478f3e7374a6ee85a71ea2cd42640c4c590b5bd Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 5 Mar 2026 16:03:16 +0000 Subject: [PATCH 3/3] tests: Assert that glnx_close_fd preserves errno We document glnx_close_fd as preserving errno, so let's assert that it really does. There are three code paths we need to exercise: 1. fd < 0: glnx_close_fd does nothing, successfully 2. fd >= 0 and close() succeeds 3. fd >= 0 and close() fails The first two are easy, but it's difficult to make close() fail on-demand with only valid code. close(2) documents EIO, but it's difficult to cause an I/O error on-demand. Similarly, close(2) documents ENOSPC and EDQUOT on NFS, but we are unlikely to have a full NFS filesystem available during testing. Instead, we can trigger a failure via the programming error of passing a fd to glnx_close_fd that was already closed, which makes close(2) fail with EBADF. In older libglnx, we wouldn't have been able to test this because it caused an assertion failure, but in GLib and new libglnx it only causes a critical warning, which we can catch and ignore. See also GLib commit GNOME/glib@f1f711dc "tests: Test EBADF and errno handling when closing fds". GLib doesn't have a 1:1 equivalent of glnx_close_fd as public API, but an internal version is used to implement g_autofd. Signed-off-by: Simon McVittie --- tests/test-libglnx-fdio.c | 121 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 121 insertions(+) diff --git a/tests/test-libglnx-fdio.c b/tests/test-libglnx-fdio.c index cfa5b648a..e237589c9 100644 --- a/tests/test-libglnx-fdio.c +++ b/tests/test-libglnx-fdio.c @@ -29,6 +29,125 @@ #include "libglnx-testlib.h" +static void +test_close (void) +{ + g_autoptr(GError) error = NULL; + int errsv; + int fd = -2; + int fd_borrowed; + + g_test_summary ("Exercise glnx_close_fd"); + + /* Closing a non-fd is a no-op, and preserves errno. + * EILSEQ is an arbitrary valid value of errno that is unlikely + * to be set accidentally as a side-effect of I/O. */ + g_test_message ("Closing a non-fd is a no-op and preserves errno..."); + errno = EILSEQ; + glnx_close_fd (&fd); + errsv = errno; + g_assert_cmpint (fd, ==, -1); + g_assert_cmpint (errsv, ==, EILSEQ); + + /* Closing a valid fd really closes it, and preserves errno. */ + g_test_message ("Closing a valid fd preserves errno..."); + glnx_opendirat (AT_FDCWD, "/", TRUE, &fd, &error); + g_assert_no_error (error); + g_assert_cmpint (fd, >=, 0); + fd_borrowed = fd; + errno = EILSEQ; + glnx_close_fd (&fd); + errsv = errno; + g_assert_cmpint (fd, ==, -1); + g_assert_cmpint (errsv, ==, EILSEQ); + _glnx_test_assert_fd_was_closed (fd_borrowed); +} + +/* Exercise glnx_close_fd in the case where close() fails. + * The only convenient way we can arrange for this to happen is to use + * an invalid fd, which is a programming error. + * + * This function is only run under g_test_undefined(), and assumes the + * implementation detail that GLib responds to that programming error + * with a critical warning rather than a fatal error. */ +static void +test_close_ebadf_subprocess (void) +{ + g_autoptr(GError) error = NULL; + int errsv; + int fd = -2; + int non_fd; + + /* Preparation: Open a fd, and close it, leaving non_fd set to the + * file descriptor number. */ + glnx_opendirat (AT_FDCWD, "/", TRUE, &fd, &error); + g_assert_no_error (error); + g_assert_cmpint (fd, >=, 0); + close (fd); + non_fd = fd; + _glnx_test_assert_fd_was_closed (non_fd); + + /* "Closing" the non-fd provokes a critical warning. */ + g_log_set_always_fatal (G_LOG_FATAL_MASK); + g_log_set_fatal_mask ("GLib", G_LOG_FATAL_MASK); + + errno = EILSEQ; + glnx_close_fd (&non_fd); + errsv = errno; + g_assert_cmpint (non_fd, ==, -1); + + /* We preserved errno. */ + g_assert_cmpint (errsv, ==, EILSEQ); + g_print ("Closing invalid fd preserved errno\n"); +} + +static void +test_close_ebadf (void) +{ + g_test_summary ("Exercise glnx_close_fd when close() fails"); + + /* If close() fails, it still preserves errno. + * The only convenient way to make close() fail on-demand is EBADF. */ + + if (g_test_subprocess ()) + { + test_close_ebadf_subprocess (); + return; + } + + if (g_test_undefined ()) + { + g_test_message ("Closing invalid fd preserves errno..."); + +#if GLIB_CHECK_VERSION (2, 38, 0) + g_test_trap_subprocess (NULL, 0, G_TEST_SUBPROCESS_DEFAULT); +#else + if (g_test_trap_fork (0, 0)) + { + test_close_ebadf_subprocess (); + _exit (0); + } +#endif + + g_test_trap_assert_passed (); + g_test_trap_assert_stdout ("*Closing invalid fd preserved errno*"); +#if !GLIB_CHECK_VERSION(2, 76, 0) + /* We can assert that our backport emits this message */ + g_test_trap_assert_stderr ("*_glnx_close(fd:*) failed with EBADF*"); +#else + /* We can't assert anything this specific about GLib's, + * but as an implementation detail, it's currently very similar */ +# if 0 + g_test_trap_assert_stderr ("*g_close(fd:*) failed with EBADF*"); +# endif +#endif + } + else + { + g_test_skip ("Can't test this without provoking undefined behaviour"); + } +} + static gboolean renameat_test_setup (int *out_srcfd, int *out_destfd, GError **error) @@ -445,6 +564,8 @@ int main (int argc, char **argv) g_test_init (&argc, &argv, NULL); + g_test_add_func ("/close", test_close); + g_test_add_func ("/close/ebadf", test_close_ebadf); g_test_add_func ("/tmpfile", test_tmpfile); g_test_add_func ("/stdio-file", test_stdio_file); g_test_add_func ("/filecopy", test_filecopy);