From 2eb0da552382fc38b81e6cb933ba586777740e66 Mon Sep 17 00:00:00 2001 From: Leandro Dorileo Date: Mon, 9 Sep 2019 14:32:34 -0700 Subject: [PATCH] log: add check helpers This patch just introduces some cosmetic macros for log and condition checking. Signed-off-by: Leandro Dorileo --- src/bootman/bootman.c | 75 ++++++++++++--------------------- src/bootman/kernel.c | 12 +++--- src/bootman/sysconfig.c | 32 ++++++--------- src/lib/log.h | 91 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 135 insertions(+), 75 deletions(-) diff --git a/src/bootman/bootman.c b/src/bootman/bootman.c index 8790c8e..807cb17 100644 --- a/src/bootman/bootman.c +++ b/src/bootman/bootman.c @@ -125,10 +125,8 @@ static bool boot_manager_select_bootloader(BootManager *self) } } - if (!selected) { - LOG_FATAL("Failed to find an appropriate bootloader for this system"); - return false; - } + CHECK_FATAL_RET_VAL(!selected, false, "Failed to find an appropriate" + " bootloader for this system"); self->bootloader = selected; @@ -157,17 +155,14 @@ bool boot_manager_set_prefix(BootManager *self, char *prefix) char *initrd_dir = NULL; SystemConfig *config = NULL; - if (!prefix) { - return false; - } + CHECK_DBG_RET_VAL(!prefix, false, "Invalid prefix value: null"); cbm_free_sysconfig(self->sysconfig); self->sysconfig = NULL; config = cbm_inspect_root(prefix, self->image_mode); - if (!config) { - return false; - } + CHECK_DBG_RET_VAL(!config, false, "Could not inspect root"); + self->sysconfig = config; if (self->kernel_dir) { @@ -323,10 +318,7 @@ int detect_and_mount_boot(BootManager *self, char **boot_dir) { boot_dev = get_legacy_boot_device((char *)prefix); } - if (!boot_dev) { - LOG_DEBUG("No boot partition, nothing to mount."); - return 0; - } + CHECK_DBG_RET_VAL(!boot_dev, 0, "No boot partition, nothing to mount."); return mount_boot(self, boot_dir); } @@ -340,24 +332,19 @@ bool boot_manager_set_default_kernel(BootManager *self, const Kernel *kernel) bool matched = false; bool default_set = false; - if (!self->bootloader) { - return false; - } - if (!cbm_is_sysconfig_sane(self->sysconfig)) { - return false; - } + CHECK_DBG_RET_VAL(!self->bootloader, false, "Invalid boot loader: null"); + + CHECK_DBG_RET_VAL(!cbm_is_sysconfig_sane(self->sysconfig), false, + "Sysconfig is not sane"); /* Grab the available kernels */ kernels = boot_manager_get_kernels(self); - if (!kernels || kernels->len == 0) { - LOG_ERROR("No kernels discovered in %s, bailing", self->kernel_dir); - return false; - } + CHECK_ERR_RET_VAL(!kernels || kernels->len == 0, false, + "No kernels discovered in %s, bailing", self->kernel_dir); did_mount = detect_and_mount_boot(self, &boot_dir); - if (did_mount < 0) { - return false; - } + CHECK_DBG_RET_VAL(did_mount < 0, false, "Boot was not mounted"); + for (uint16_t i = 0; i < kernels->len; i++) { const Kernel *k = nc_array_get(kernels, i); if (streq(kernel->meta.ktype, k->meta.ktype) && @@ -372,9 +359,8 @@ bool boot_manager_set_default_kernel(BootManager *self, const Kernel *kernel) umount_boot(boot_dir); } - if (!matched) { - LOG_ERROR("No matching kernel in %s, bailing", self->kernel_dir); - }; + CHECK_ERR(!matched, "No matching kernel in %s, bailing", self->kernel_dir); + return default_set; } @@ -382,12 +368,9 @@ char *boot_manager_get_default_kernel(BootManager *self) { assert(self != NULL); - if (!self->bootloader) { - return NULL; - } - if (!cbm_is_sysconfig_sane(self->sysconfig)) { - return NULL; - } + CHECK_DBG_RET_VAL(!self->bootloader, NULL, "Invalid bootloader value: null"); + CHECK_DBG_RET_VAL(!cbm_is_sysconfig_sane(self->sysconfig), NULL, + "Sysconfig is not sane"); return self->bootloader->get_default_kernel(self); } @@ -457,20 +440,17 @@ int mount_boot(BootManager *self, char **boot_directory) /* Determine root device */ root_base = self->sysconfig->boot_device; - if (!root_base) { - LOG_FATAL("Cannot determine boot device"); - goto out; - } + CHECK_FATAL_GOTO(!root_base, out, "Cannot determine boot device"); abs_bootdir = cbm_system_get_mountpoint_for_device(root_base); if (abs_bootdir) { LOG_DEBUG("Boot device already mounted at %s", abs_bootdir); + /* User has already mounted the ESP somewhere else, use that */ - if (!boot_manager_set_boot_dir(self, abs_bootdir)) { - LOG_FATAL("Cannot initialise with premounted ESP"); - goto out; - } + CHECK_FATAL_GOTO(!boot_manager_set_boot_dir(self, abs_bootdir), out, + "Cannot initialize with premounted ESP"); + /* Successfully using their premounted ESP, go use it */ LOG_INFO("Skipping to native update"); *boot_directory = strdup(boot_dir); @@ -499,10 +479,9 @@ int mount_boot(BootManager *self, char **boot_directory) * as it may have paths that already exist, and we must adjust for case * sensitivity (ignorant) issues */ - if (!boot_manager_set_boot_dir(self, boot_dir)) { - LOG_FATAL("Cannot initialise with newly mounted ESP"); - goto out; - } + CHECK_FATAL_GOTO(!boot_manager_set_boot_dir(self, boot_dir), out, + "Cannot initialize with newly mounted ESP"); + *boot_directory = strdup(boot_dir); if (*boot_directory) { ret = 1; diff --git a/src/bootman/kernel.c b/src/bootman/kernel.c index 92485e2..1a1f421 100644 --- a/src/bootman/kernel.c +++ b/src/bootman/kernel.c @@ -115,10 +115,9 @@ Kernel *boot_manager_inspect_kernel(BootManager *self, char *path) /* TODO: We may actually be uninstalling a partially flopped kernel, * so validity of existing kernels may be questionable * Thus, flag it, and return kernel */ - if (!nc_file_exists(cmdline)) { - LOG_ERROR("Valid kernel found with no cmdline: %s (expected %s)", path, cmdline); - return NULL; - } + CHECK_ERR_RET_VAL(!nc_file_exists(cmdline), NULL, + "Valid kernel found with no cmdline: %s (expected %s)", + path, cmdline); /* Check local modules */ module_dir = string_printf("%s/%s/%s-%d.%s", @@ -345,9 +344,8 @@ Kernel *boot_manager_get_default_for_type(BootManager *self, KernelArray *kernel default_file = string_printf("%s/default-%s", self->kernel_dir, type); - if (readlink(default_file, linkbuf, sizeof(linkbuf)) < 0) { - return NULL; - } + CHECK_DBG_RET_VAL(readlink(default_file, linkbuf, sizeof(linkbuf)) < 0, + NULL, "Could not resolve symlink"); for (uint16_t i = 0; i < kernels->len; i++) { Kernel *k = nc_array_get(kernels, i); diff --git a/src/bootman/sysconfig.c b/src/bootman/sysconfig.c index e158735..bf4ff4f 100644 --- a/src/bootman/sysconfig.c +++ b/src/bootman/sysconfig.c @@ -157,22 +157,14 @@ SystemConfig *cbm_inspect_root(const char *path, bool image_mode) char *realp = NULL; char *rel = NULL; - if (!path) { - return NULL; - } + CHECK_ERR_RET_VAL(!path, NULL, "invalid \"path\" value: null"); realp = realpath(path, NULL); - if (!realp) { - LOG_ERROR("Path specified does not exist: %s", path); - return NULL; - } + CHECK_ERR_RET_VAL(!realp, NULL, "Path specified does not exist: %s", path); c = calloc(1, sizeof(struct SystemConfig)); - if (!c) { - DECLARE_OOM(); - free(realp); - return NULL; - } + CHECK_ERR_GOTO(!c, error, "Could not allocate SystemConfig"); + c->prefix = realp; c->wanted_boot_mask = 0; @@ -203,18 +195,18 @@ SystemConfig *cbm_inspect_root(const char *path, bool image_mode) c->root_device = cbm_probe_path(realp); return c; + + error: + DECLARE_OOM(); + free(realp); + return NULL; } bool cbm_is_sysconfig_sane(SystemConfig *config) { - if (!config) { - LOG_FATAL("sysconfig insane: Missing config"); - return false; - } - if (!config->root_device) { - LOG_FATAL("sysconfig insane: Missing root device"); - return false; - } + CHECK_FATAL_RET_VAL(!config, false, "sysconfig insane: Missing config"); + CHECK_FATAL_RET_VAL(!config->root_device, false, + "sysconfig insane: Missing root device"); return true; } diff --git a/src/lib/log.h b/src/lib/log.h index 093ad74..ae06e3d 100644 --- a/src/lib/log.h +++ b/src/lib/log.h @@ -70,6 +70,97 @@ void cbm_log(CbmLogLevel level, const char *file, int line, const char *format, */ #define LOG_WARNING(...) (cbm_log(CBM_LOG_WARNING, __FILE__, __LINE__, __VA_ARGS__)) +#define check_common_ret_val(level, exp, ret_val, ...) \ + do { \ + if (exp) { \ + LOG_##level(__VA_ARGS__); \ + return ret_val; \ + } \ + } while(false) \ + +#define check_common_ret(level, exp, ...) \ + do { \ + if (exp) { \ + LOG_##level(__VA_ARGS__); \ + return; \ + } \ + } while(false) \ + +#define check_common_goto(level, exp, label, ...) \ + do { \ + if (exp) { \ + LOG_##level(__VA_ARGS__); \ + goto label; \ + } \ + } while(false) \ + +#define check_common(level, exp, ...) \ + do { \ + if (exp) { \ + LOG_##level(__VA_ARGS__); \ + } \ + } while(false) \ + +#define CHECK_ERR(exp, ...) \ + check_common(ERROR, exp, __VA_ARGS__) \ + +#define CHECK_ERR_RET_VAL(exp, ret_val, ...) \ + check_common_ret_val(ERROR, exp, ret_val, __VA_ARGS__) \ + +#define CHECK_ERR_RET(exp, ...) \ + check_common_ret(ERROR, exp, __VA_ARGS__) \ + +#define CHECK_ERR_GOTO(exp, label, ...) \ + check_common_goto(ERROR, exp, label, __VA_ARGS__) \ + +#define CHECK_WARN(exp, ...) \ + check_common(WARNING, exp, __VA_ARGS__) \ + +#define CHECK_WARN_RET_VAL(exp, ret_val, ...) \ + check_common_ret_val(WARNING, exp, ret_val, __VA_ARGS__) \ + +#define CHECK_WARN_RET(exp, ...) \ + check_common_ret(WARNING, exp, __VA_ARGS__) \ + +#define CHECK_WARN_GOTO(exp, label, ...) \ + check_common_goto(WARNING, exp, label, __VA_ARGS__) \ + +#define CHECK_INF(exp, ...) \ + check_common(INFO, exp, __VA_ARGS__) \ + +#define CHECK_INF_RET_VAL(exp, ret_val, ...) \ + check_common_ret_val(INFO, exp, ret_val, __VA_ARGS__) \ + +#define CHECK_INF_RET(exp, ...) \ + check_common_ret(INFO, exp, __VA_ARGS__) \ + +#define CHECK_INF_GOTO(exp, label, ...) \ + check_common_goto(INFO, exp, label, __VA_ARGS__) \ + +#define CHECK_DBG(exp, ...) \ + check_common(DEBUG, exp, __VA_ARGS__) \ + +#define CHECK_DBG_RET_VAL(exp, ret_val, ...) \ + check_common_ret_val(DEBUG, exp, ret_val, __VA_ARGS__) \ + +#define CHECK_DBG_RET(exp, ...) \ + check_common_ret(DEBUG, exp, __VA_ARGS__) \ + +#define CHECK_DBG_GOTO(exp, label, ...) \ + check_common_goto(DEBUG, exp, label, __VA_ARGS__) \ + +#define CHECK_FATAL(exp, ...) \ + check_common(FATAL, exp, __VA_ARGS__) \ + +#define CHECK_FATAL_RET_VAL(exp, ret_val, ...) \ + check_common_ret_val(FATAL, exp, ret_val, __VA_ARGS__) \ + +#define CHECK_FATAL_RET(exp, ...) \ + check_common_ret(FATAL, exp, __VA_ARGS__) \ + +#define CHECK_FATAL_GOTO(exp, label, ...) \ + check_common_goto(FATAL, exp, label, __VA_ARGS__) \ + /* * Editor modelines - https://www.wireshark.org/tools/modelines.html *