From b3934133217748ba17eeef7575e102a44c22542d Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 21 Oct 2022 17:39:35 +0100 Subject: [PATCH] gstdio: Add g_clear_fd() and g_autofd Inspired by libglnx's glnx_close_fd() and glnx_autofd, these let us have the same patterns as g_clear_object() and g_autoptr(GObject), but for file descriptors. g_clear_fd() is cross-platform, while g_autofd is syntactic sugar requiring a supported compiler (gcc or clang). Now that g_close() checks for EBADF as a programming error, we can implement the equivalent of glnx_autofd as an inline function without needing to have errno and EBADF in the header file. g_clear_fd() is like glnx_close_fd(), but with error checking. The private _g_clear_fd_ignore_error() function used to implement g_autofd is a closer equivalent of glnx_close_fd(). Signed-off-by: Simon McVittie --- docs/reference/glib/glib-sections.txt.in | 2 + glib/gmacros.h | 2 +- glib/gstdio.c | 100 +++++++++++++++++++++++ glib/gstdio.h | 32 ++++++++ glib/tests/fileutils.c | 76 +++++++++++++++++ 5 files changed, 211 insertions(+), 1 deletion(-) diff --git a/docs/reference/glib/glib-sections.txt.in b/docs/reference/glib/glib-sections.txt.in index 90b0f3258..f81b454d3 100644 --- a/docs/reference/glib/glib-sections.txt.in +++ b/docs/reference/glib/glib-sections.txt.in @@ -1454,6 +1454,8 @@ g_creat g_chdir g_utime g_close +g_clear_fd +g_autofd g_file_error_quark diff --git a/glib/gmacros.h b/glib/gmacros.h index f9e20407e..530284110 100644 --- a/glib/gmacros.h +++ b/glib/gmacros.h @@ -1276,7 +1276,7 @@ #if g_macro__has_attribute(cleanup) -/* these macros are private */ +/* these macros are private; note that gstdio.h also uses _GLIB_CLEANUP */ #define _GLIB_AUTOPTR_FUNC_NAME(TypeName) glib_autoptr_cleanup_##TypeName #define _GLIB_AUTOPTR_CLEAR_FUNC_NAME(TypeName) glib_autoptr_clear_##TypeName #define _GLIB_AUTOPTR_TYPENAME(TypeName) TypeName##_autoptr diff --git a/glib/gstdio.c b/glib/gstdio.c index 4a481505a..eafa80b21 100644 --- a/glib/gstdio.c +++ b/glib/gstdio.c @@ -1837,3 +1837,103 @@ g_close (gint fd, return TRUE; } + +/** + * g_clear_fd: (skip) + * @fd_ptr: (not nullable): a pointer to a file descriptor + * @error: Used to return an error on failure + * + * If @fd_ptr points to a file descriptor, close it and return + * whether closing it was successful, like g_close(). + * If @fd_ptr points to a negative number, return %TRUE without closing + * anything. + * In both cases, set @fd_ptr to `-1` before returning. + * + * It is a programming error for @fd_ptr to point to a non-negative + * number that is not a valid file descriptor. + * + * A typical use of this function is to clean up a file descriptor at + * the end of its scope, whether it has been set successfully or not: + * + * |[ + * gboolean + * operate_on_fd (GError **error) + * { + * gboolean ret = FALSE; + * int fd = -1; + * + * fd = open_a_fd (error); + * + * if (fd < 0) + * goto out; + * + * if (!do_something (fd, error)) + * goto out; + * + * if (!g_clear_fd (&fd, error)) + * goto out; + * + * ret = TRUE; + * + * out: + * // OK to call even if fd was never opened or was already closed + * g_clear_fd (&fd, NULL); + * return ret; + * } + * ]| + * + * This function is also useful in conjunction with #g_autofd. + * + * Returns: %TRUE on success + * Since: 2.76 + */ + +/** + * g_autofd: (skip) + * + * Macro to add an attribute to a file descriptor variable to ensure + * automatic cleanup using g_clear_fd(). + * + * This macro behaves like #g_autofree rather than g_autoptr(): it is + * an attribute supplied before the type name, rather than wrapping the + * type definition. + * + * Otherwise, this macro has similar constraints as g_autoptr(): it is + * only supported on GCC and clang, and the variable must be initialized + * (to either a valid file descriptor or a negative number). + * + * Any error from closing the file descriptor when it goes out of scope + * is ignored. Use g_clear_fd() if error-checking is required. + * + * |[ + * gboolean + * operate_on_fds (GError **error) + * { + * g_autofd int fd1 = open_a_fd (..., error); + * g_autofd int fd2 = -1; + * + * // it is safe to return early here, nothing will be closed + * if (fd1 < 0) + * return FALSE; + * + * fd2 = open_a_fd (..., error); + * + * // fd1 will be closed automatically if we return here + * if (fd2 < 0) + * return FALSE; + * + * // fd1 and fd2 will be closed automatically if we return here + * if (!do_something_useful (fd1, fd2, error)) + * return FALSE; + * + * // fd2 will be closed automatically if we return here + * if (!g_clear_fd (&fd1, error)) + * return FALSE; + * + * // fd2 will be automatically closed here if still open + * return TRUE; + * } + * ]| + * + * Since: 2.76 + */ diff --git a/glib/gstdio.h b/glib/gstdio.h index 441029848..9f677567a 100644 --- a/glib/gstdio.h +++ b/glib/gstdio.h @@ -178,6 +178,38 @@ GLIB_AVAILABLE_IN_2_36 gboolean g_close (gint fd, GError **error); +GLIB_AVAILABLE_STATIC_INLINE_IN_2_76 +static inline gboolean +g_clear_fd (int *fd_ptr, + GError **error) +{ + int fd = *fd_ptr; + + *fd_ptr = -1; + + if (fd < 0) + return TRUE; + + return g_close (fd, error); +} + +/* g_autofd should be defined on the same compilers where g_autofree is + * This avoids duplicating the feature-detection here. */ +#ifdef g_autofree +/* Not public API */ +static inline void +_g_clear_fd_ignore_error (int *fd_ptr) +{ + 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(). */ + } +} + +#define g_autofd _GLIB_CLEANUP(_g_clear_fd_ignore_error) +#endif + G_END_DECLS #endif /* __G_STDIO_H__ */ diff --git a/glib/tests/fileutils.c b/glib/tests/fileutils.c index 5218aedec..b957d26c3 100644 --- a/glib/tests/fileutils.c +++ b/glib/tests/fileutils.c @@ -2453,6 +2453,81 @@ test_win32_zero_terminate_symlink (void) #endif +static void +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 = g_fsync (fd); + int errsv = errno; + + g_assert_cmpint (result, !=, 0); + g_assert_cmpint (errsv, ==, EBADF); + } +} + +static void +test_clear_fd (void) +{ + char *name = NULL; + GError *error = NULL; + int fd; + int copy_of_fd; + +#ifdef g_autofree + g_test_summary ("Test g_clear_fd() and g_autofd"); +#else + g_test_summary ("Test g_clear_fd() (g_autofd unsupported by this compiler)"); +#endif + + /* g_clear_fd() normalizes any negative number to -1 */ + fd = -23; + g_clear_fd (&fd, &error); + g_assert_cmpint (fd, ==, -1); + g_assert_no_error (error); + + /* Nothing special about g_file_open_tmp, it's just a convenient way + * to get an open fd */ + fd = g_file_open_tmp (NULL, &name, &error); + g_assert_cmpint (fd, !=, -1); + g_assert_no_error (error); + g_assert_nonnull (name); + copy_of_fd = fd; + g_clear_fd (&fd, &error); + g_assert_cmpint (fd, ==, -1); + g_assert_no_error (error); + assert_fd_was_closed (copy_of_fd); + g_unlink (name); + g_free (name); + + /* g_clear_fd() is idempotent */ + g_clear_fd (&fd, &error); + g_assert_cmpint (fd, ==, -1); + g_assert_no_error (error); + +#ifdef g_autofree + fd = g_file_open_tmp (NULL, &name, &error); + g_assert_cmpint (fd, !=, -1); + g_assert_no_error (error); + g_assert_nonnull (name); + + { + g_autofd int close_me = fd; + g_autofd int was_never_set = -42; + + /* This avoids clang warnings about the variables being unused */ + g_test_message ("Will be closed by autocleanup: %d, %d", + close_me, was_never_set); + } + + assert_fd_was_closed (fd); + g_unlink (name); + g_free (name); +#endif +} + int main (int argc, char *argv[]) @@ -2489,6 +2564,7 @@ main (int argc, g_test_add_func ("/fileutils/read-link", test_read_link); 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); return g_test_run (); }