From 4bef6c6f9a81a7f8698b35664e52dbc53733c1d4 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Thu, 12 Oct 2023 12:01:50 +0200 Subject: [PATCH] Make errno handling cleaner with errint_t type In some places we return an int which is -1 for error and errno set, and in some places we return -errno for error. This adds a new errint_t type to make it more clear what is happening. Also all users are changed to use an errint_t err for the new type, and keep int res for the old type. Signed-off-by: Alexander Larsson --- libcomposefs/lcfs-internal.h | 3 ++ libcomposefs/lcfs-mount.c | 66 ++++++++++++++++++++---------------- tools/cfs-fuse.c | 18 +++++----- tools/mkcomposefs.c | 7 ++-- 4 files changed, 52 insertions(+), 42 deletions(-) diff --git a/libcomposefs/lcfs-internal.h b/libcomposefs/lcfs-internal.h index f8cebe18..5c3d566a 100644 --- a/libcomposefs/lcfs-internal.h +++ b/libcomposefs/lcfs-internal.h @@ -33,6 +33,9 @@ #include "lcfs-fsverity.h" #include "hash.h" +/* This is used for (internal) functions that return zero or -errno, functions that set errno return int */ +typedef int errint_t; + /* When using LCFS_BUILD_INLINE_SMALL in lcfs_load_node_from_file() inline files below this size * We pick 64 which is the size of a sha256 digest that would otherwise be used as a redirect * xattr, so the inlined file is smaller. diff --git a/libcomposefs/lcfs-mount.c b/libcomposefs/lcfs-mount.c index 693e1b04..3dd518b4 100644 --- a/libcomposefs/lcfs-mount.c +++ b/libcomposefs/lcfs-mount.c @@ -170,7 +170,7 @@ static char *escape_mount_option(const char *str) return res; } -static int lcfs_validate_mount_options(struct lcfs_mount_state_s *state) +static errint_t lcfs_validate_mount_options(struct lcfs_mount_state_s *state) { struct lcfs_mount_options_s *options = state->options; @@ -200,7 +200,7 @@ static int lcfs_validate_mount_options(struct lcfs_mount_state_s *state) return 0; } -static int lcfs_validate_verity_fd(struct lcfs_mount_state_s *state) +static errint_t lcfs_validate_verity_fd(struct lcfs_mount_state_s *state) { struct { struct fsverity_digest fsv; @@ -225,7 +225,7 @@ static int lcfs_validate_verity_fd(struct lcfs_mount_state_s *state) return 0; } -static int setup_loopback(int fd, const char *image_path, char *loopname) +static errint_t setup_loopback(int fd, const char *image_path, char *loopname) { struct loop_config loopconfig = { 0 }; int loopctlfd, loopfd; @@ -296,7 +296,7 @@ static char *compute_lower(const char *imagemount, return lower; } -static int lcfs_mount_ovl_legacy(struct lcfs_mount_state_s *state, char *imagemount) +static errint_t lcfs_mount_ovl_legacy(struct lcfs_mount_state_s *state, char *imagemount) { struct lcfs_mount_options_s *options = state->options; @@ -351,21 +351,22 @@ static int lcfs_mount_ovl_legacy(struct lcfs_mount_state_s *state, char *imagemo if (lowerdir_target == lowerdir_1) mount_flags |= MS_SILENT; + errint_t err = 0; res = mount("overlay", state->mountpoint, "overlay", mount_flags, overlay_options); if (res != 0) { - res = -errno; + err = -errno; } - if (res == -EINVAL && lowerdir_target == lowerdir_1) { + if (err == -EINVAL && lowerdir_target == lowerdir_1) { lowerdir_target = lowerdir_2; goto retry; } - return res; + return err; } -static int lcfs_mount_ovl(struct lcfs_mount_state_s *state, char *imagemount) +static errint_t lcfs_mount_ovl(struct lcfs_mount_state_s *state, char *imagemount) { #ifdef HAVE_NEW_MOUNT_API struct lcfs_mount_options_s *options = state->options; @@ -476,8 +477,9 @@ static int lcfs_mount_ovl(struct lcfs_mount_state_s *state, char *imagemount) #endif } -static int lcfs_mount_erofs(const char *source, const char *target, - uint32_t image_flags, struct lcfs_mount_state_s *state) +static errint_t lcfs_mount_erofs(const char *source, const char *target, + uint32_t image_flags, + struct lcfs_mount_state_s *state) { bool image_has_acls = (image_flags & LCFS_EROFS_FLAGS_HAS_ACL) != 0; bool use_idmap = (state->options->flags & LCFS_MOUNT_FLAGS_IDMAP) != 0; @@ -553,8 +555,8 @@ static int lcfs_mount_erofs(const char *source, const char *target, #define HEADER_SIZE sizeof(struct lcfs_erofs_header_s) -static int lcfs_mount_erofs_ovl(struct lcfs_mount_state_s *state, - struct lcfs_erofs_header_s *header) +static errint_t lcfs_mount_erofs_ovl(struct lcfs_mount_state_s *state, + struct lcfs_erofs_header_s *header) { struct lcfs_mount_options_s *options = state->options; uint32_t image_flags; @@ -562,7 +564,8 @@ static int lcfs_mount_erofs_ovl(struct lcfs_mount_state_s *state, char *imagemount; bool created_tmpdir = false; char loopname[PATH_MAX]; - int res, errsv; + int errsv; + errint_t err; int loopfd; image_flags = lcfs_u32_from_file(header->flags); @@ -583,37 +586,38 @@ static int lcfs_mount_erofs_ovl(struct lcfs_mount_state_s *state, created_tmpdir = true; } - res = lcfs_mount_erofs(loopname, imagemount, image_flags, state); + err = lcfs_mount_erofs(loopname, imagemount, image_flags, state); close(loopfd); - if (res < 0) { + if (err < 0) { rmdir(imagemount); - return res; + return err; } /* We use the legacy API to mount overlayfs, because the new API doesn't allow use * to pass in escaped directory names */ - res = lcfs_mount_ovl(state, imagemount); - if (res == -ENOSYS) - res = lcfs_mount_ovl_legacy(state, imagemount); + err = lcfs_mount_ovl(state, imagemount); + if (err == -ENOSYS) + err = lcfs_mount_ovl_legacy(state, imagemount); umount2(imagemount, MNT_DETACH); if (created_tmpdir) { rmdir(imagemount); } - return res; + return err; } -static int lcfs_mount(struct lcfs_mount_state_s *state) +static errint_t lcfs_mount(struct lcfs_mount_state_s *state) { uint8_t header_data[HEADER_SIZE]; struct lcfs_erofs_header_s *erofs_header; + int err; int res; - res = lcfs_validate_verity_fd(state); - if (res < 0) - return res; + err = lcfs_validate_verity_fd(state); + if (err < 0) + return err; res = pread(state->fd, &header_data, HEADER_SIZE, 0); if (res < 0) @@ -631,11 +635,12 @@ int lcfs_mount_fd(int fd, const char *mountpoint, struct lcfs_mount_options_s *o struct lcfs_mount_state_s state = { .mountpoint = mountpoint, .options = options, .fd = fd }; + errint_t err; int res; - res = lcfs_validate_mount_options(&state); - if (res < 0) { - errno = -res; + err = lcfs_validate_mount_options(&state); + if (err < 0) { + errno = -err; return -1; } @@ -654,11 +659,12 @@ int lcfs_mount_image(const char *path, const char *mountpoint, .mountpoint = mountpoint, .options = options, .fd = -1 }; + errint_t err; int fd, res; - res = lcfs_validate_mount_options(&state); - if (res < 0) { - errno = -res; + err = lcfs_validate_mount_options(&state); + if (err < 0) { + errno = -err; return -1; } diff --git a/tools/cfs-fuse.c b/tools/cfs-fuse.c index c95f3a11..8d65682a 100644 --- a/tools/cfs-fuse.c +++ b/tools/cfs-fuse.c @@ -695,8 +695,8 @@ static const char *cfs_xattr_rewrite(int name_index, const char *name, return name; } -static int cfs_listxattr_element(const struct erofs_xattr_entry *entry, - char *buf, size_t *buf_size, size_t max_buf_size) +static errint_t cfs_listxattr_element(const struct erofs_xattr_entry *entry, + char *buf, size_t *buf_size, size_t max_buf_size) { const char *name = (const char *)entry + sizeof(struct erofs_xattr_entry); uint8_t name_len = entry->e_name_len; @@ -747,7 +747,7 @@ static void cfs_listxattr(fuse_req_t req, fuse_ino_t ino, size_t max_size) const uint8_t *xattrs_inline; const uint8_t *xattrs_start; const uint8_t *xattrs_end; - int res; + errint_t err; if (cino == NULL) { fuse_reply_err(req, ENOENT); @@ -788,9 +788,9 @@ static void cfs_listxattr(fuse_req_t req, fuse_ino_t ino, size_t max_size) size_t el_size = round_up( sizeof(struct erofs_xattr_entry) + name_len + value_size, 4); - res = cfs_listxattr_element(entry, buf, &buf_size, max_size); - if (res < 0) { - fuse_reply_err(req, -res); + err = cfs_listxattr_element(entry, buf, &buf_size, max_size); + if (err < 0) { + fuse_reply_err(req, -err); return; } xattrs_inline += el_size; @@ -802,9 +802,9 @@ static void cfs_listxattr(fuse_req_t req, fuse_ino_t ino, size_t max_size) const struct erofs_xattr_entry *entry = (const struct erofs_xattr_entry *)(erofs_xattrdata + idx * 4); - res = cfs_listxattr_element(entry, buf, &buf_size, max_size); - if (res < 0) { - fuse_reply_err(req, -res); + err = cfs_listxattr_element(entry, buf, &buf_size, max_size); + if (err < 0) { + fuse_reply_err(req, -err); return; } } diff --git a/tools/mkcomposefs.c b/tools/mkcomposefs.c index bb262186..0f273f78 100644 --- a/tools/mkcomposefs.c +++ b/tools/mkcomposefs.c @@ -165,7 +165,7 @@ static int join_paths(char **out, const char *path1, const char *path2) return asprintf(out, "%.*s%s%s", len, path1, sep, path2); } -static int enable_verity(int fd) +static errint_t enable_verity(int fd) { struct fsverity_enable_arg arg = {}; @@ -200,6 +200,7 @@ static int copy_file_with_dirs_if_needed(const char *src, const char *dst_base, cleanup_free char *pathbuf = NULL; cleanup_unlink_free char *tmppath = NULL; int ret, res; + errint_t err; cleanup_fd int sfd = -1; cleanup_fd int dfd = -1; struct stat statbuf; @@ -258,8 +259,8 @@ static int copy_file_with_dirs_if_needed(const char *src, const char *dst_base, } if (fstat(dfd, &statbuf) == 0) { - res = enable_verity(dfd); - if (res < 0) { + err = enable_verity(dfd); + if (err < 0) { /* Ignore errors, we're only trying to enable it */ } }