Skip to content

Commit

Permalink
Make errno handling cleaner with errint_t type
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@alexlarsson
alexlarsson committed Oct 12, 2023
1 parent 110d64b commit 4bef6c6
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 42 deletions.
3 changes: 3 additions & 0 deletions libcomposefs/lcfs-internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
66 changes: 36 additions & 30 deletions libcomposefs/lcfs-mount.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -553,16 +555,17 @@ 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;
char imagemountbuf[] = "/tmp/.composefs.XXXXXX";
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);
Expand All @@ -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)
Expand All @@ -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;
}

Expand All @@ -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;
}

Expand Down
18 changes: 9 additions & 9 deletions tools/cfs-fuse.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}
}
Expand Down
7 changes: 4 additions & 3 deletions tools/mkcomposefs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {};

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 */
}
}
Expand Down

0 comments on commit 4bef6c6

Please sign in to comment.