From 97cd6a6c1d58a90b52edcff2e2dcb69c8cf7855a Mon Sep 17 00:00:00 2001 From: Colin Walters <walters@verbum.org> Date: Fri, 6 Oct 2017 10:09:18 -0400 Subject: [PATCH] Add glnx_fd_close() and glnx_autofd I'd like to have the checks for `EBADF` as well as the "assign to -1" in more places. The cleanup function we had for `glnx_fd_close` is actually what we want. Let's rename the cleanup macro to `glnx_autofd` to better match other autocleanups like `g_autofree`. Then we can use `glnx_fd_close()` as a replacement for plain Unix `close()`. I left the `glnx_close_fd` macro, but it's obviously confusing now with the former. We'll eventually remove it. --- glnx-dirfd.c | 3 +-- glnx-local-alloc.h | 38 ++++++++++++++++++++++++++------------ glnx-lockfile.c | 7 ++----- 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/glnx-dirfd.c b/glnx-dirfd.c index ea12c8f9d..e6c3319c1 100644 --- a/glnx-dirfd.c +++ b/glnx-dirfd.c @@ -382,8 +382,7 @@ _glnx_tmpdir_free (GLnxTmpDir *tmpd, if (!(tmpd && tmpd->initialized)) return TRUE; g_assert_cmpint (tmpd->fd, !=, -1); - (void) close (tmpd->fd); - tmpd->fd = -1; + glnx_close_fd (&tmpd->fd); g_assert (tmpd->path); g_assert_cmpint (tmpd->src_dfd, !=, -1); g_autofree char *path = tmpd->path; /* Take ownership */ diff --git a/glnx-local-alloc.h b/glnx-local-alloc.h index 46dd9d25d..3be1fa43b 100644 --- a/glnx-local-alloc.h +++ b/glnx-local-alloc.h @@ -42,14 +42,30 @@ glnx_local_obj_unref (void *v) } #define glnx_unref_object __attribute__ ((cleanup(glnx_local_obj_unref))) +static inline int +glnx_steal_fd (int *fdp) +{ + int fd = *fdp; + *fdp = -1; + return fd; +} + +/** + * glnx_close_fd: + * @fdp: Pointer to fd + * + * Effectively `close (glnx_steal_fd (&fd))`. Also + * asserts that `close()` did not raise `EBADF` - encountering + * that error is usually a critical bug in the program. + */ static inline void -glnx_cleanup_close_fdp (int *fdp) +glnx_close_fd (int *fdp) { - int fd, errsv; + int errsv; g_assert (fdp); - fd = *fdp; + int fd = glnx_steal_fd (fdp); if (fd >= 0) { errsv = errno; @@ -62,16 +78,14 @@ glnx_cleanup_close_fdp (int *fdp) /** * glnx_fd_close: * + * Deprecated in favor of `glnx_autofd`. + */ +#define glnx_fd_close __attribute__((cleanup(glnx_close_fd))) +/** + * glnx_autofd: + * * Call close() on a variable location when it goes out of scope. */ -#define glnx_fd_close __attribute__((cleanup(glnx_cleanup_close_fdp))) - -static inline int -glnx_steal_fd (int *fdp) -{ - int fd = *fdp; - *fdp = -1; - return fd; -} +#define glnx_autofd __attribute__((cleanup(glnx_close_fd))) G_END_DECLS diff --git a/glnx-lockfile.c b/glnx-lockfile.c index cd5c978c6..2956edafe 100644 --- a/glnx-lockfile.c +++ b/glnx-lockfile.c @@ -119,8 +119,7 @@ glnx_make_lock_file(int dfd, const char *p, int operation, GLnxLockFile *out_loc if (st.st_nlink > 0) break; - (void) close(fd); - fd = -1; + glnx_close_fd (&fd); } /* Note that if this is not AT_FDCWD, the caller takes responsibility @@ -174,9 +173,7 @@ void glnx_release_lock_file(GLnxLockFile *f) { f->path = NULL; } - if (f->fd != -1) - (void) close (f->fd); - f->fd = -1; + glnx_close_fd (&f->fd); f->operation = 0; f->initialized = FALSE; } -- GitLab