From 1bd9e95e17cca189071a9a1f64681cd62cac5a02 Mon Sep 17 00:00:00 2001 From: Marc Herbert Date: Thu, 25 Nov 2021 02:45:52 +0000 Subject: [PATCH 01/20] logger: open /sys/debug/fw_version _after_ /sys/debug/[e]trace Open /sys/kernel/debug/sof/fw_version _after_ /sys/kernel/debug/sof/[e]trace because reading the former is optional and the latter is not. So when the driver is not loaded, we get the same (missing trace) error trace message whether we use the -n option or not. Signed-off-by: Marc Herbert (cherry picked from commit af6bd41a994ccf9998a5c6c32d1fe27974bc612e) --- tools/logger/logger.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/tools/logger/logger.c b/tools/logger/logger.c index 543e8e85a0c8..a9aa08c77d28 100644 --- a/tools/logger/logger.c +++ b/tools/logger/logger.c @@ -304,17 +304,6 @@ int main(int argc, char *argv[]) goto out; } - if (config.version_fw) { - config.version_fd = fopen(config.version_file, "rb"); - if (!config.version_fd && !config.dump_ldc) { - ret = errno; - fprintf(stderr, - "error: Unable to open version file %s: %s\n", - config.version_file, strerror(ret)); - goto out; - } - } - if (config.out_file) { config.out_fd = fopen(config.out_file, "w"); if (!config.out_fd) { @@ -360,6 +349,17 @@ int main(int argc, char *argv[]) if (isatty(fileno(config.out_fd)) != 1) config.use_colors = 0; + if (config.version_fw) { + config.version_fd = fopen(config.version_file, "rb"); + if (!config.version_fd && !config.dump_ldc) { + ret = errno; + fprintf(stderr, + "error: Unable to open version file %s: %s\n", + config.version_file, strerror(ret)); + goto out; + } + } + ret = -convert(&config); out: From 34be370d81b250aee2490acdd78703c8c42fc859 Mon Sep 17 00:00:00 2001 From: Marc Herbert Date: Thu, 25 Nov 2021 02:51:29 +0000 Subject: [PATCH 02/20] logger: allow starting before the driver is loaded Don't fail immediately when the driver is not loaded. Use inotify instead to wait for /sys/kernel/debug/sof/[e]trace to appear. This makes it possible to start before the driver is loaded which reduces considerably the chances of missing early logs. Fixes a small part of https://github.com/thesofproject/linux/issues/3275 Signed-off-by: Marc Herbert (cherry picked from commit dcf0577a7725413aad1a378d629b9cc6659550ef) --- tools/logger/logger.c | 114 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 113 insertions(+), 1 deletion(-) diff --git a/tools/logger/logger.c b/tools/logger/logger.c index a9aa08c77d28..a28661e53caa 100644 --- a/tools/logger/logger.c +++ b/tools/logger/logger.c @@ -15,6 +15,12 @@ #include #include #include +#include +#include +#include +#include +#include + #include "convert.h" #include "misc.h" @@ -170,6 +176,93 @@ static int append_filter_config(struct convert_config *config, const char *input return 0; } +/* + * This 5 minutes timeout is for "backward compatible safety" in case + * any user/script of an earlier, pre-inotify version of sof-logger + * (accidentally?) _expected_ the tool to quit when the driver was not + * loaded. This expectation was always wrong but an infinite wait is too + * severe of a punishment. This timeout cannot be too small otherwise it + * would defeat the new feature. + */ +#ifndef SOF_LOGGER_WAIT_OPEN_TIMEOUT_SECS +#define SOF_LOGGER_WAIT_OPEN_TIMEOUT_SECS (5 * 60) +#endif + +/* + * Using inotify, wait up to 5 minutes for 'expected_name' to appear in + * 'watched_dir'. Then return opendir/fopen(expected_name). fopen() and + * opendir() failures are NOT handled; e.g. NULL from fopen() is just + * passed through. + * + * Returns a FILE * or DIR * + */ +static void *wait_open(const char *watched_dir, const char *expected_file) +{ + if (access(watched_dir, R_OK)) { + fprintf(stderr, "error: %s() cannot read %s\n", __func__, watched_dir); + return NULL; + } + + const int iqueue = inotify_init(); + const int dwatch = inotify_add_watch(iqueue, watched_dir, IN_CREATE); + struct stat expected_stat; + void *ret_stream = NULL; + + char * const fpath = malloc(strlen(watched_dir) + 1 + strlen(expected_file) + 1); + + strcpy(fpath, watched_dir); + strcat(fpath, "/"); + strcat(fpath, expected_file); + + /* Not racy because the inotify watch was set first. */ + if (!access(fpath, F_OK)) + goto fopenit; + + fprintf(stdout, "%s: waiting in %s/ for %s\n", APP_NAME, watched_dir, expected_file); + fflush(stdout); + + while (1) { + char evlist[1 * sizeof(struct inotify_event)]; + struct pollfd pfd[1] = {{ .fd = iqueue, .events = POLLIN, .revents = 0 }}; + int pret = poll(pfd, sizeof(pfd) / sizeof(struct pollfd), + SOF_LOGGER_WAIT_OPEN_TIMEOUT_SECS * 1000); + + if (pret == 0) { + fprintf(stderr, "error: no %s after waiting %d seconds\n", + expected_file, SOF_LOGGER_WAIT_OPEN_TIMEOUT_SECS); + goto cleanup; + } + + if (pret < 0 || pfd[0].revents != POLLIN) { + fprintf(stderr, "error: in %s, poll(%s) returned %d, %d\n", + __func__, watched_dir, pret, pfd[0].revents); + goto cleanup; + } + + /* Silence __attribute__((warn_unused_result)) which is + * enabled by some Linux distros even when broken in gcc: + * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 + */ + (void)!read(iqueue, evlist, sizeof(evlist)); + + if (!access(fpath, F_OK)) + goto fopenit; + } + +fopenit: + stat(fpath, &expected_stat); + if ((expected_stat.st_mode & S_IFMT) == S_IFDIR) + ret_stream = opendir(fpath); + else + ret_stream = fopen(fpath, "rb"); + +cleanup: + free(fpath); + inotify_rm_watch(iqueue, dwatch); + + return ret_stream; +} + int main(int argc, char *argv[]) { static const char optstring[] = "ho:i:l:ps:c:u:tv:rd:Le:f:gF:n"; @@ -337,7 +430,26 @@ int main(int argc, char *argv[]) goto out; } } else if (config.in_file) { - config.in_fd = fopen(config.in_file, "rb"); + static const char sys_debug[] = "/sys/kernel/debug"; + static const char sys_debug_sof[] = "/sys/kernel/debug/sof"; + /* In the future we could add an option to force waiting + * for _any_ input file, not just for /sys/kernel/debug/sof/[e]trace + */ + config.in_fd = NULL; + if (strncmp(config.in_file, sys_debug, strlen(sys_debug))) { + config.in_fd = fopen(config.in_file, "rb"); + } else { + DIR *dbg_sof = (DIR *)wait_open(sys_debug, "sof"); + + if (dbg_sof) { + closedir(dbg_sof); + config.in_fd = + (FILE *)wait_open(sys_debug_sof, + config.in_file + + strlen(sys_debug_sof) + 1); + } + } + if (!config.in_fd) { ret = errno; fprintf(stderr, From 78f24c04b82ec8d5d2c4651707c1e0bb5ac06a0e Mon Sep 17 00:00:00 2001 From: Marc Herbert Date: Thu, 25 Aug 2022 15:14:40 -0700 Subject: [PATCH 03/20] sof-logger: make inotify optional Restores ability to compile on Windows with MSYS. Fixes commit dcf0577a7725 ("logger: allow starting before the driver is loaded") Signed-off-by: Marc Herbert (cherry picked from commit 92d828bc37329020b4e92b29109efadb28f794ca) --- tools/logger/CMakeLists.txt | 8 ++++++++ tools/logger/config.h.in | 1 + tools/logger/logger.c | 24 ++++++++++++++++++++---- 3 files changed, 29 insertions(+), 4 deletions(-) create mode 100644 tools/logger/config.h.in diff --git a/tools/logger/CMakeLists.txt b/tools/logger/CMakeLists.txt index d84c92ce0cc5..57637706c5d6 100644 --- a/tools/logger/CMakeLists.txt +++ b/tools/logger/CMakeLists.txt @@ -1,5 +1,13 @@ # SPDX-License-Identifier: BSD-3-Clause +# https://gitlab.kitware.com/cmake/community/-/wikis/doc/tutorials/How-To-Write-Platform-Checks +INCLUDE (CheckIncludeFiles) +CHECK_INCLUDE_FILES(sys/inotify.h HAS_INOTIFY) + +CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/config.h.in + ${CMAKE_CURRENT_BINARY_DIR}/config.h) +include_directories(${CMAKE_CURRENT_BINARY_DIR}) + add_executable(sof-logger logger.c convert.c diff --git a/tools/logger/config.h.in b/tools/logger/config.h.in new file mode 100644 index 000000000000..fecbafbd274c --- /dev/null +++ b/tools/logger/config.h.in @@ -0,0 +1 @@ +#cmakedefine01 HAS_INOTIFY diff --git a/tools/logger/logger.c b/tools/logger/logger.c index a28661e53caa..b5df7e185d32 100644 --- a/tools/logger/logger.c +++ b/tools/logger/logger.c @@ -5,6 +5,7 @@ // Author: Bartosz Kokoszko // Artur Kloniecki +#include #include #include #include @@ -15,11 +16,17 @@ #include #include #include + +#include +#include + +#include "config.h" + +#if HAS_INOTIFY #include #include -#include #include -#include +#endif #include "convert.h" #include "misc.h" @@ -176,6 +183,12 @@ static int append_filter_config(struct convert_config *config, const char *input return 0; } +#if !HAS_INOTIFY +static void *wait_open(const char *watched_dir, const char *expected_file) +{ + assert(HAS_INOTIFY); /* Should never be called */ +} +#else /* * This 5 minutes timeout is for "backward compatible safety" in case * any user/script of an earlier, pre-inotify version of sof-logger @@ -262,6 +275,8 @@ static void *wait_open(const char *watched_dir, const char *expected_file) return ret_stream; } +#endif // HAS_INOTIFY + int main(int argc, char *argv[]) { @@ -436,9 +451,10 @@ int main(int argc, char *argv[]) * for _any_ input file, not just for /sys/kernel/debug/sof/[e]trace */ config.in_fd = NULL; - if (strncmp(config.in_file, sys_debug, strlen(sys_debug))) { + if (!HAS_INOTIFY || + strncmp(config.in_file, sys_debug, strlen(sys_debug))) { config.in_fd = fopen(config.in_file, "rb"); - } else { + } else { // both inotify and /sys/kernel/debug/ DIR *dbg_sof = (DIR *)wait_open(sys_debug, "sof"); if (dbg_sof) { From bbe1dcad49e71aaa8e7780c3ecaf7aa23a4aa0a8 Mon Sep 17 00:00:00 2001 From: Kai Vehmanen Date: Mon, 19 Dec 2022 17:15:36 +0200 Subject: [PATCH 04/20] sof-logger: exit with error if malloc fails In user-space tools, memory allocations can reasonably be expected to always succeed. Make this assumption explicit by adding error handling after malloc. Signed-off-by: Kai Vehmanen (cherry picked from commit 4d64893b868297e259f578267a18e18275e538c1) --- tools/logger/logger.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tools/logger/logger.c b/tools/logger/logger.c index b5df7e185d32..11e3753638bf 100644 --- a/tools/logger/logger.c +++ b/tools/logger/logger.c @@ -223,6 +223,11 @@ static void *wait_open(const char *watched_dir, const char *expected_file) char * const fpath = malloc(strlen(watched_dir) + 1 + strlen(expected_file) + 1); + if (!fpath) { + fprintf(stderr, "error: can't allocate memory\n"); + exit(EXIT_FAILURE); + } + strcpy(fpath, watched_dir); strcat(fpath, "/"); strcat(fpath, expected_file); From f9a1aae4ca2c6709fba58598b798cc677bdc4f46 Mon Sep 17 00:00:00 2001 From: Kai Vehmanen Date: Mon, 19 Dec 2022 16:41:48 +0200 Subject: [PATCH 05/20] logger: exit with error if calloc fails In user-space tools, memory allocations can reasonably be expected to always succeed. Make this assumption explicit by adding error handling after calloc. Signed-off-by: Kai Vehmanen (cherry picked from commit 4bec5b292c708a84a22a0a4d84e9747d61ecd70d) --- tools/logger/convert.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tools/logger/convert.c b/tools/logger/convert.c index 569f81579f52..fb0c8cdc2777 100644 --- a/tools/logger/convert.c +++ b/tools/logger/convert.c @@ -131,6 +131,10 @@ static const char *format_uid(uint32_t uid_ptr, int use_colors, bool be, bool up if (uid_ptr < uids_dict->base_address || uid_ptr >= uids_dict->base_address + uids_dict->data_length) { str = calloc(1, strlen(BAD_PTR_STR) + 1 + 6); + if (!str) { + log_err("can't allocate memory\n"); + exit(EXIT_FAILURE); + } sprintf(str, BAD_PTR_STR, uid_ptr); } else { uid_entry = get_uuid_entry(uid_ptr); From c82d7dba3889851a0034eb51c82a255bb0934874 Mon Sep 17 00:00:00 2001 From: Kai Vehmanen Date: Mon, 19 Dec 2022 16:55:24 +0200 Subject: [PATCH 06/20] sof-logger: print error if -u uart option is given with no infile sof-logger -u 115200 -d /lib/firmware/sof-foo.ldc Leads to silent failure as a NULL is passed to open(). Add explicit error handling for this case. Signed-off-by: Kai Vehmanen (cherry picked from commit 80adcdf36a1d0e0590012fe73c7fd4d593a9b3cd) --- tools/logger/logger.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tools/logger/logger.c b/tools/logger/logger.c index 11e3753638bf..b61023a3f301 100644 --- a/tools/logger/logger.c +++ b/tools/logger/logger.c @@ -441,6 +441,11 @@ int main(int argc, char *argv[]) if (!config.in_file && !config.dump_ldc) config.in_file = "/sys/kernel/debug/sof/etrace"; + if (!config.in_file && baud) { + fprintf(stderr, "error: No UART device specified\n"); + usage(); + } + if (config.input_std) { config.in_fd = stdin; } else if (baud) { From 3671e6a71fe111e1b90259886e044141145843ec Mon Sep 17 00:00:00 2001 From: Marc Herbert Date: Tue, 20 Dec 2022 22:11:17 +0000 Subject: [PATCH 07/20] logger: make "global_config" truly global Finish the job that commit 5b29dae9c870 ("logger: Create global convert_config variable to avoid spaghetti code.") started but did not finish, leaving behind a supposedly "global" variable that is actually a confusing global pointer to a struct local to the main() function. This confuses some static analyzer complaining that stack values are being returned, see #6858 and #6738. This is a false positive because the main()'s stack lifespan is the same as a global but let's simplify things anyway. Also stop using 'extern' in .c files, use a proper .h file instead. Signed-off-by: Marc Herbert (cherry picked from commit 327a26bf8ad70113f1eda933e5a53df175bc8e95) --- tools/logger/convert.c | 13 ++++++++----- tools/logger/convert.h | 6 +++++- tools/logger/filter.c | 2 -- tools/logger/logger.c | 5 ++++- tools/logger/misc.c | 2 -- 5 files changed, 17 insertions(+), 11 deletions(-) diff --git a/tools/logger/convert.c b/tools/logger/convert.c index fb0c8cdc2777..503966b2fb1f 100644 --- a/tools/logger/convert.c +++ b/tools/logger/convert.c @@ -65,9 +65,6 @@ static const char *BAD_PTR_STR = ""; #define UUID_LOWER "%s%s%s<%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x>%s%s%s" #define UUID_UPPER "%s%s%s<%08X-%04X-%04X-%02X%02X-%02X%02X%02X%02X%02X%02X>%s%s%s" -/* pointer to config for global context */ -struct convert_config *global_config; - static int read_entry_from_ldc_file(struct ldc_entry *entry, uint32_t log_entry_address); char *format_uid_raw(const struct sof_uuid_entry *uid_entry, int use_colors, int name_first, @@ -1037,14 +1034,20 @@ static int dump_ldc_info(void) return 0; } -int convert(struct convert_config *config) +int convert(void) { struct snd_sof_logs_header snd; struct snd_sof_uids_header uids_hdr; int count, ret = 0; + /* const pointer initialized at build time */ + if (!global_config) + abort(); + + /* just a shorter alias */ + struct convert_config * const config = global_config; + config->logs_header = &snd; - global_config = config; count = fread(&snd, sizeof(snd), 1, config->ldc_fd); if (!count) { diff --git a/tools/logger/convert.h b/tools/logger/convert.h index 607e5e59e049..dcf8d0db06b7 100644 --- a/tools/logger/convert.h +++ b/tools/logger/convert.h @@ -47,4 +47,8 @@ struct convert_config { }; uint32_t get_uuid_key(const struct sof_uuid_entry *entry); -int convert(struct convert_config *config); + +/* pointer to config for global context */ +extern struct convert_config * const global_config; + +int convert(void); diff --git a/tools/logger/filter.c b/tools/logger/filter.c index b70af55f156b..80b5f72345fd 100644 --- a/tools/logger/filter.c +++ b/tools/logger/filter.c @@ -22,8 +22,6 @@ #define COMPONENTS_SEPARATOR ',' #define COMPONENT_NAME_SCAN_STRING_LENGTH 32 -extern struct convert_config *global_config; - /** map between log level given by user and enum value */ static const struct { const char name[16]; diff --git a/tools/logger/logger.c b/tools/logger/logger.c index b61023a3f301..14f3fc88f657 100644 --- a/tools/logger/logger.c +++ b/tools/logger/logger.c @@ -282,6 +282,8 @@ static void *wait_open(const char *watched_dir, const char *expected_file) } #endif // HAS_INOTIFY +static struct convert_config _global_config; +struct convert_config * const global_config = &_global_config; int main(int argc, char *argv[]) { @@ -498,7 +500,8 @@ int main(int argc, char *argv[]) } } - ret = -convert(&config); + _global_config = config; + ret = -convert(); out: /* free memory */ diff --git a/tools/logger/misc.c b/tools/logger/misc.c index 37ca30adb9c1..20e5db9f95b8 100644 --- a/tools/logger/misc.c +++ b/tools/logger/misc.c @@ -41,8 +41,6 @@ char *log_asprintf(const char *format, ...) return result; } -extern struct convert_config *global_config; - /** Prints 1. once to stderr. 2. a second time to the global out_fd if * out_fd is neither stderr nor stdout (because the -o option was used). */ From 8ac1e153142b6a61044b142deca825229855c692 Mon Sep 17 00:00:00 2001 From: Marc Herbert Date: Tue, 20 Dec 2022 23:15:48 +0000 Subject: [PATCH 08/20] logger: convert.c: move global_config->logs_header to the heap Finish the job that commit 5b29dae9c870 ("logger: Create global convert_config variable to avoid spaghetti code.") started but did not complete, leaving a confusing mix of globals and locals. This confuses some static analyzer complaining that stack values are being returned, see #6858 and #6738. This is a false positive because convert's() stack lifespan is practically the same as a global but let's simplify things anyway. Signed-off-by: Marc Herbert (cherry picked from commit 2dfaee6a7d872ae8baab03026a56da76da6a64f3) --- tools/logger/convert.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/tools/logger/convert.c b/tools/logger/convert.c index 503966b2fb1f..c802c67b8951 100644 --- a/tools/logger/convert.c +++ b/tools/logger/convert.c @@ -1036,7 +1036,7 @@ static int dump_ldc_info(void) int convert(void) { - struct snd_sof_logs_header snd; + struct snd_sof_logs_header * const logs_hdr = malloc(sizeof(*logs_hdr)); struct snd_sof_uids_header uids_hdr; int count, ret = 0; @@ -1044,32 +1044,35 @@ int convert(void) if (!global_config) abort(); + if (!logs_hdr) + abort(); + /* just a shorter alias */ struct convert_config * const config = global_config; - config->logs_header = &snd; + config->logs_header = logs_hdr; - count = fread(&snd, sizeof(snd), 1, config->ldc_fd); + count = fread(logs_hdr, sizeof(*logs_hdr), 1, config->ldc_fd); if (!count) { log_err("Error while reading %s.\n", config->ldc_file); return -ferror(config->ldc_fd); } - if (strncmp((char *) snd.sig, SND_SOF_LOGS_SIG, SND_SOF_LOGS_SIG_SIZE)) { + if (strncmp((char *)logs_hdr->sig, SND_SOF_LOGS_SIG, SND_SOF_LOGS_SIG_SIZE)) { log_err("Invalid ldc file signature.\n"); return -EINVAL; } if (global_config->version_fw && /* -n option */ !global_config->dump_ldc) { - ret = verify_ldc_checksum(global_config->logs_header->version.src_hash); + ret = verify_ldc_checksum(logs_hdr->version.src_hash); if (ret) return ret; } /* default logger and ldc_file abi verification */ if (SOF_ABI_VERSION_INCOMPATIBLE(SOF_ABI_DBG_VERSION, - snd.version.abi_version)) { + logs_hdr->version.abi_version)) { log_err("abi version in %s file does not coincide with abi version used by logger.\n", config->ldc_file); log_err("logger ABI Version is %d:%d:%d\n", @@ -1077,14 +1080,14 @@ int convert(void) SOF_ABI_VERSION_MINOR(SOF_ABI_DBG_VERSION), SOF_ABI_VERSION_PATCH(SOF_ABI_DBG_VERSION)); log_err("ldc_file ABI Version is %d:%d:%d\n", - SOF_ABI_VERSION_MAJOR(snd.version.abi_version), - SOF_ABI_VERSION_MINOR(snd.version.abi_version), - SOF_ABI_VERSION_PATCH(snd.version.abi_version)); + SOF_ABI_VERSION_MAJOR(logs_hdr->version.abi_version), + SOF_ABI_VERSION_MINOR(logs_hdr->version.abi_version), + SOF_ABI_VERSION_PATCH(logs_hdr->version.abi_version)); return -EINVAL; } /* read uuid section header */ - fseek(config->ldc_fd, snd.data_offset + snd.data_length, SEEK_SET); + fseek(config->ldc_fd, logs_hdr->data_offset + logs_hdr->data_length, SEEK_SET); count = fread(&uids_hdr, sizeof(uids_hdr), 1, config->ldc_fd); if (!count) { log_err("Error while reading uuids header from %s.\n", config->ldc_file); From 8b5def3997fc6e5e7fba56d9f5157b91656bc0fb Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Tue, 20 Dec 2022 14:28:40 +0100 Subject: [PATCH 09/20] logger: check localtime() return value localtime() can return NULL in error cases. Check the return value before dereferencing it. Signed-off-by: Guennadi Liakhovetski (cherry picked from commit 84b2dd2ae94ccc4101f44ecbea9564ffdd87d754) --- tools/logger/convert.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tools/logger/convert.c b/tools/logger/convert.c index c802c67b8951..0b23cac276c6 100644 --- a/tools/logger/convert.c +++ b/tools/logger/convert.c @@ -356,11 +356,14 @@ static inline void print_table_header(void) fprintf(out_fd, "%s", "CONTENT"); if (global_config->time_precision >= 0) { + struct tm *ltime = localtime(&epoc_secs); + /* e.g.: ktime=4263.487s @ 2021-04-27 14:21:13 -0700 PDT */ fprintf(out_fd, "\tktime=%lu.%03lus", ktime.tv_sec, ktime.tv_nsec / 1000000); - if (strftime(date_string, sizeof(date_string), - "%F %X %z %Z", localtime(&epoc_secs))) + + if (ltime && strftime(date_string, sizeof(date_string), + "%F %X %z %Z", ltime)) fprintf(out_fd, " @ %s", date_string); } From ace183cc4c22894541b0b755a254740abbff32d1 Mon Sep 17 00:00:00 2001 From: Kai Vehmanen Date: Wed, 21 Dec 2022 12:56:20 +0200 Subject: [PATCH 10/20] sof-logger: ensure NULL string is not passed to printf/fprintf Due to allocation failures, or invalid content in dictionary, "params" entries in "struct proc_ldc_entry" may be NULL. In print_entry_params(), the NULL entries may be passed as arguments to fprintf(). While e.g. glibc handles these without error, this is not guaranteed behaviour and may result in segfault on some platforms. Fix the issue by aborting program if allocation fails and explicitly handling the cases when asprintf_entry_text returns NULL. Signed-off-by: Kai Vehmanen (cherry picked from commit 1a7a36a84cfe31a6f6b40488300d5b6cd709250c) --- tools/logger/convert.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/tools/logger/convert.c b/tools/logger/convert.c index 0b23cac276c6..417d3ee6e8b4 100644 --- a/tools/logger/convert.c +++ b/tools/logger/convert.c @@ -65,6 +65,8 @@ static const char *BAD_PTR_STR = ""; #define UUID_LOWER "%s%s%s<%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x>%s%s%s" #define UUID_UPPER "%s%s%s<%08X-%04X-%04X-%02X%02X-%02X%02X%02X%02X%02X%02X>%s%s%s" +static const char *missing = ""; + static int read_entry_from_ldc_file(struct ldc_entry *entry, uint32_t log_entry_address); char *format_uid_raw(const struct sof_uuid_entry *uid_entry, int use_colors, int name_first, @@ -249,6 +251,8 @@ static void process_params(struct proc_ldc_entry *pe, /* check for string printing, because it leads to logger crash */ log_err("String printing is not supported\n"); pe->params[i] = (uintptr_t)log_asprintf("", raw_param); + if (!pe->params[i]) + abort(); pe->subst_mask |= 1 << i; ++i; p += 2; @@ -257,6 +261,8 @@ static void process_params(struct proc_ldc_entry *pe, /* substitute UUID entry address with formatted string pointer from heap */ pe->params[i] = (uintptr_t)asprintf_uuid(p, raw_param, use_colors, &uuid_fmt_len); + if (!pe->params[i]) + abort(); pe->subst_mask |= 1 << i; ++i; /* replace uuid formatter with %s */ @@ -268,7 +274,12 @@ static void process_params(struct proc_ldc_entry *pe, /* %pQ format specifier */ /* substitute log entry address with formatted entry text */ pe->params[i] = (uintptr_t)asprintf_entry_text(raw_param); - pe->subst_mask |= 1 << i; + + if (pe->params[i]) + pe->subst_mask |= 1 << i; + else + pe->params[i] = (uintptr_t)missing; + ++i; /* replace entry formatter with %s */ @@ -1021,7 +1032,7 @@ static int dump_ldc_info(void) while (remaining > 0) { name = format_uid_raw(&uid_ptr[cnt], 0, 0, false, false); uid_addr = get_uuid_key(&uid_ptr[cnt]); - fprintf(out_fd, "\t%p %s\n", (void *)uid_addr, name); + fprintf(out_fd, "\t%p %s\n", (void *)uid_addr, name ? name : missing); if (name) { free(name); From 9aad8bc7cb2ee4a47a4d4d17f402f8f8fc6f1f2d Mon Sep 17 00:00:00 2001 From: Adrian Warecki Date: Mon, 18 Sep 2023 15:08:26 +0200 Subject: [PATCH 11/20] smex: elf: Removed unnecessary initialization of local variables The values assigned when declaring variables were overwritten in the code. Redundant initialization was removed. Signed-off-by: Adrian Warecki (cherry picked from commit 74d093b6bfcbc6106e445e6a6898e2cdcb2f8d3a) --- smex/elf.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/smex/elf.c b/smex/elf.c index bd088716691d..3aeb62e3d4fc 100644 --- a/smex/elf.c +++ b/smex/elf.c @@ -17,7 +17,7 @@ static int elf_read_sections(struct elf_module *module, bool verbose) { Elf32_Ehdr *hdr = &module->hdr; - Elf32_Shdr *section = module->section; + Elf32_Shdr *section; size_t count; int i, ret; uint32_t valid = (SHF_WRITE | SHF_ALLOC | SHF_EXECINSTR); @@ -128,7 +128,7 @@ static int elf_read_sections(struct elf_module *module, bool verbose) static int elf_read_programs(struct elf_module *module, bool verbose) { Elf32_Ehdr *hdr = &module->hdr; - Elf32_Phdr *prg = module->prg; + Elf32_Phdr *prg; size_t count; int i, ret; @@ -431,7 +431,7 @@ int elf_read_section(const struct elf_module *module, const char *section_name, const Elf32_Shdr **dst_section, void **dst_buff) { const Elf32_Shdr *section; - int section_index = -1; + int section_index; int read; section_index = elf_find_section(module, section_name); From b9911e6f4ae662c5a016117c5892b7b799fefec9 Mon Sep 17 00:00:00 2001 From: Adrian Warecki Date: Mon, 18 Sep 2023 15:10:11 +0200 Subject: [PATCH 12/20] smex: elf: Added checking of value returned by file operation function Added checking of the value returned by fseek function and added memory release when an error is detected. Signed-off-by: Adrian Warecki (cherry picked from commit 36448d0b63325adb041d9d162905a67028f47623) --- smex/elf.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/smex/elf.c b/smex/elf.c index 3aeb62e3d4fc..9a4f13be7d2c 100644 --- a/smex/elf.c +++ b/smex/elf.c @@ -432,7 +432,7 @@ int elf_read_section(const struct elf_module *module, const char *section_name, { const Elf32_Shdr *section; int section_index; - int read; + int ret; section_index = elf_find_section(module, section_name); if (section_index < 0) { @@ -451,17 +451,25 @@ int elf_read_section(const struct elf_module *module, const char *section_name, return -ENOMEM; /* fill buffer with section content */ - fseek(module->fd, section->off, SEEK_SET); - read = fread(*dst_buff, 1, section->size, module->fd); - if (read != section->size) { - fprintf(stderr, - "error: can't read %s section %d\n", section_name, - -errno); - free(*dst_buff); - return -errno; + ret = fseek(module->fd, section->off, SEEK_SET); + if (ret) { + fprintf(stderr, "error: can't seek to %s section %d\n", section_name, -errno); + ret = -errno; + goto error; + } + + ret = fread(*dst_buff, 1, section->size, module->fd); + if (ret != section->size) { + fprintf(stderr, "error: can't read %s section %d\n", section_name, -errno); + ret = -errno; + goto error; } return section->size; + +error: + free(*dst_buff); + return ret; } int elf_read_module(struct elf_module *module, const char *name, bool verbose) From d93e32fb3949fe1ee8fcfe9075ab3534b552ea49 Mon Sep 17 00:00:00 2001 From: Adrian Warecki Date: Mon, 18 Sep 2023 15:11:09 +0200 Subject: [PATCH 13/20] smex: elf: elf_find_section: Check function input data String terminator was added to the buffer with a list of section names in the elf file. Added check to the section name index to make sure it doesn't go beyond the buffer size. Signed-off-by: Adrian Warecki (cherry picked from commit 5b837af2237ec609421ddeb1d5f3032e660a9a9a) --- smex/elf.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/smex/elf.c b/smex/elf.c index 9a4f13be7d2c..711e061bcdd7 100644 --- a/smex/elf.c +++ b/smex/elf.c @@ -408,10 +408,17 @@ int elf_find_section(const struct elf_module *module, const char *name) ret = -errno; goto out; } + buffer[section->size - 1] = '\0'; /* find section with name */ for (i = 0; i < hdr->shnum; i++) { s = &module->section[i]; + if (s->name >= section->size) { + fprintf(stderr, "error: invalid section name string index %d\n", s->name); + ret = -EINVAL; + goto out; + } + if (!strcmp(name, buffer + s->name)) { ret = i; goto out; From f4f43256c844921f6fd3b244d8fec196c8a6c7ba Mon Sep 17 00:00:00 2001 From: Adrian Warecki Date: Mon, 18 Sep 2023 15:23:36 +0200 Subject: [PATCH 14/20] logger: convert: Fixed handling of an error reported by clock_gettime The clock_gettime function only returns information that an error occurred. The error code should be taken from the errno variable. Signed-off-by: Adrian Warecki (cherry picked from commit a119fad6ad8ab8c47e75f0014294324a6d754936) --- tools/logger/convert.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/logger/convert.c b/tools/logger/convert.c index 417d3ee6e8b4..bf26f758e188 100644 --- a/tools/logger/convert.c +++ b/tools/logger/convert.c @@ -349,7 +349,7 @@ static inline void print_table_header(void) if (gettime_ret) { log_err("clock_gettime() failed: %s\n", - strerror(gettime_ret)); + strerror(errno)); exit(1); } From e2fcd55c73e762b28951aaaf5bccc9cfcd0885c9 Mon Sep 17 00:00:00 2001 From: Adrian Warecki Date: Mon, 18 Sep 2023 15:26:37 +0200 Subject: [PATCH 15/20] logger: convert: Added error handling for file operation functions. Added checking of value returned by file operation functions. In case of an error, message is printed and error code is returned. Signed-off-by: Adrian Warecki (cherry picked from commit b83a50e339464149cf6cad9bdd762bc81f0b76b4) --- tools/logger/convert.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/tools/logger/convert.c b/tools/logger/convert.c index bf26f758e188..87661569369e 100644 --- a/tools/logger/convert.c +++ b/tools/logger/convert.c @@ -624,7 +624,13 @@ static int read_entry_from_ldc_file(struct ldc_entry *entry, uint32_t log_entry_ entry->params = NULL; /* set file position to beginning of processed entry */ - fseek(global_config->ldc_fd, entry_offset, SEEK_SET); + ret = fseek(global_config->ldc_fd, entry_offset, SEEK_SET); + if (ret) { + log_err("Failed to seek to entry header for offset 0x%x in dictionary.\n", + entry_offset); + ret = -errno; + goto out; + } /* fetching elf header params */ ret = fread(&entry->header, sizeof(entry->header), 1, global_config->ldc_fd); @@ -915,8 +921,13 @@ static int logger_read(void) /* When the address is not correct, move forward by one DWORD (not * entire struct dma_log) */ - fseek(global_config->in_fd, -(sizeof(dma_log) - sizeof(uint32_t)), + ret = fseek(global_config->in_fd, -(sizeof(dma_log) - sizeof(uint32_t)), SEEK_CUR); + if (ret) { + log_err("fetch_entry() failed on seek, aborting\n"); + ret = -errno; + break; + } skipped_dwords++; continue; @@ -1013,7 +1024,7 @@ static int dump_ldc_info(void) if (global_config->version_fd) { struct sof_ipc_fw_version ver; - if (fread(&ver, sizeof(ver), 1, global_config->version_fd)) + if (fread(&ver, sizeof(ver), 1, global_config->version_fd) == 1) fprintf(out_fd, "Loaded FW expects checksum\t0x%08x\n", ver.src_hash); } @@ -1101,7 +1112,12 @@ int convert(void) } /* read uuid section header */ - fseek(config->ldc_fd, logs_hdr->data_offset + logs_hdr->data_length, SEEK_SET); + ret = fseek(config->ldc_fd, logs_hdr->data_offset + logs_hdr->data_length, SEEK_SET); + if (ret) { + log_err("Error while seeking to uuids header from %s.\n", config->ldc_file); + return -errno; + } + count = fread(&uids_hdr, sizeof(uids_hdr), 1, config->ldc_fd); if (!count) { log_err("Error while reading uuids header from %s.\n", config->ldc_file); From b024938371d9ba5bb11df963f9e39537708a60c5 Mon Sep 17 00:00:00 2001 From: Adrian Warecki Date: Mon, 18 Sep 2023 15:31:28 +0200 Subject: [PATCH 16/20] logger: convert: Simplified printing of a timestamp The timestamp printing process has been simplified by eliminating the dynamic creation of the formatting string. All necessary parameters are now passed directly to the printing function. Signed-off-by: Adrian Warecki (cherry picked from commit 8516f1d223b85ee1c956ddd2862138bb07ae71b3) --- tools/logger/convert.c | 35 +++++++++++++---------------------- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/tools/logger/convert.c b/tools/logger/convert.c index 87661569369e..4dee37479dd7 100644 --- a/tools/logger/convert.c +++ b/tools/logger/convert.c @@ -339,7 +339,6 @@ static inline void print_table_header(void) { FILE *out_fd = global_config->out_fd; int hide_location = global_config->hide_location; - char time_fmt[32]; char date_string[64]; const time_t epoc_secs = time(NULL); @@ -354,11 +353,9 @@ static inline void print_table_header(void) } if (global_config->time_precision >= 0) { - const unsigned int ts_width = - timestamp_width(global_config->time_precision); - snprintf(time_fmt, sizeof(time_fmt), "%%-%ds(us)%%%ds ", - ts_width, ts_width); - fprintf(out_fd, time_fmt, " TIMESTAMP", "DELTA"); + const unsigned int ts_width = timestamp_width(global_config->time_precision); + + fprintf(out_fd, "%*s(us)%*s ", -ts_width, " TIMESTAMP", ts_width, "DELTA"); } fprintf(out_fd, "%2s %-18s ", "C#", "COMPONENT"); @@ -476,7 +473,6 @@ static void print_entry_params(const struct log_entry_header *dma_log, char ids[TRACE_MAX_IDS_STR]; float dt = to_usecs(dma_log->timestamp - last_timestamp); struct proc_ldc_entry proc_entry; - static char time_fmt[64]; int ret; if (raw_output) @@ -517,13 +513,7 @@ static void print_entry_params(const struct log_entry_header *dma_log, ids[0] = '\0'; if (raw_output) { /* "raw" means script-friendly (not all hex) */ - const char *entry_fmt = "%s%u %u %s%s%s "; - - if (time_precision >= 0) - snprintf(time_fmt, sizeof(time_fmt), "%%.%df %%.%df ", - time_precision, time_precision); - - fprintf(out_fd, entry_fmt, + fprintf(out_fd, "%s%u %u %s%s%s ", entry->header.level == use_colors ? (LOG_LEVEL_CRITICAL ? KRED : KNRM) : "", dma_log->core_id, @@ -531,9 +521,12 @@ static void print_entry_params(const struct log_entry_header *dma_log, get_component_name(entry->header.component_class, dma_log->uid), raw_output && strlen(ids) ? "-" : "", ids); + if (time_precision >= 0) - fprintf(out_fd, time_fmt, - to_usecs(dma_log->timestamp - timestamp_origin), dt); + fprintf(out_fd, "%.*f %.*f ", + time_precision, to_usecs(dma_log->timestamp - timestamp_origin), + time_precision, dt); + if (!hide_location) fprintf(out_fd, "(%s:%u) ", format_file_name(entry->file_name, raw_output), @@ -542,13 +535,11 @@ static void print_entry_params(const struct log_entry_header *dma_log, if (time_precision >= 0) { const unsigned int ts_width = timestamp_width(time_precision); - snprintf(time_fmt, sizeof(time_fmt), - "%%s[%%%d.%df] (%%%d.%df)%%s ", - ts_width, time_precision, ts_width, time_precision); - - fprintf(out_fd, time_fmt, + fprintf(out_fd, "%s[%*.*f] (%*.*f)%s ", use_colors ? KGRN : "", - to_usecs(dma_log->timestamp - timestamp_origin), dt, + ts_width, time_precision, + to_usecs(dma_log->timestamp - timestamp_origin), + ts_width, time_precision, dt, use_colors ? KNRM : ""); } From 0f95a392e9a14717b17d5054e0a9122deb4e6612 Mon Sep 17 00:00:00 2001 From: Adrian Warecki Date: Mon, 18 Sep 2023 15:35:11 +0200 Subject: [PATCH 17/20] logger: convert: read_entry_from_ldc_file: Make sure string null terminated Added a null string terminator to be sure that strings read from a file are terminated correctly. Signed-off-by: Adrian Warecki (cherry picked from commit b0b79f15620348c1610916208572355caf2f3a69) --- tools/logger/convert.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tools/logger/convert.c b/tools/logger/convert.c index 4dee37479dd7..472d25465d19 100644 --- a/tools/logger/convert.c +++ b/tools/logger/convert.c @@ -637,7 +637,7 @@ static int read_entry_from_ldc_file(struct ldc_entry *entry, uint32_t log_entry_ ret = -EINVAL; goto out; } - entry->file_name = (char *)malloc(entry->header.file_name_len); + entry->file_name = (char *)malloc(entry->header.file_name_len + 1); if (!entry->file_name) { log_err("can't allocate %d byte for entry.file_name\n", @@ -648,6 +648,8 @@ static int read_entry_from_ldc_file(struct ldc_entry *entry, uint32_t log_entry_ ret = fread(entry->file_name, sizeof(char), entry->header.file_name_len, global_config->ldc_fd); + entry->file_name[entry->header.file_name_len] = '\0'; + if (ret != entry->header.file_name_len) { log_err("Failed to read source filename for offset 0x%x in dictionary.\n", entry_offset); @@ -661,7 +663,7 @@ static int read_entry_from_ldc_file(struct ldc_entry *entry, uint32_t log_entry_ ret = -EINVAL; goto out; } - entry->text = (char *)malloc(entry->header.text_len); + entry->text = (char *)malloc(entry->header.text_len + 1); if (!entry->text) { log_err("can't allocate %d byte for entry.text\n", entry->header.text_len); ret = -ENOMEM; @@ -674,6 +676,7 @@ static int read_entry_from_ldc_file(struct ldc_entry *entry, uint32_t log_entry_ ret = -1; goto out; } + entry->text[entry->header.text_len] = '\0'; return 0; From db43d770cfa232e50e043857cb439700003594d5 Mon Sep 17 00:00:00 2001 From: Adrian Warecki Date: Mon, 18 Sep 2023 15:37:12 +0200 Subject: [PATCH 18/20] logger: convert: Code quality improvements The precision check condition has been simplified, the unsigned value cannot be negative. Added definitions containing an error message instead of using a constant variable. Signed-off-by: Adrian Warecki (cherry picked from commit 1951b8d16c2501ce17935997ce6e79688530e556) --- tools/logger/convert.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tools/logger/convert.c b/tools/logger/convert.c index 472d25465d19..f8488d70e2d8 100644 --- a/tools/logger/convert.c +++ b/tools/logger/convert.c @@ -60,8 +60,7 @@ struct proc_ldc_entry { uintptr_t params[TRACE_MAX_PARAMS_COUNT]; }; -static const char *BAD_PTR_STR = ""; - +#define BAD_PTR_STR "" #define UUID_LOWER "%s%s%s<%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x>%s%s%s" #define UUID_UPPER "%s%s%s<%08X-%04X-%04X-%02X%02X-%02X%02X%02X%02X%02X%02X>%s%s%s" @@ -325,7 +324,7 @@ static unsigned int timestamp_width(unsigned int precision) * gcc 9.3, this avoids a very long precision causing snprintf() * to truncate time_fmt */ - assert(precision >= 0 && precision < 20); + assert(precision < 20); /* * 12 digits for units is enough for 1M seconds = 11 days which * should be enough for most test runs. From b36b94e3fe4d4e960332a0ffa4fb8f926555d16d Mon Sep 17 00:00:00 2001 From: Adrian Warecki Date: Mon, 18 Sep 2023 15:42:56 +0200 Subject: [PATCH 19/20] tools: logger: Use a safe variant of the string manipulation functions Used string manipulation functions that check the size of the available buffer. Signed-off-by: Adrian Warecki (cherry picked from commit 50f936dbcf74e1790af63e86bf35d50517a297d8) --- tools/logger/logger.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/tools/logger/logger.c b/tools/logger/logger.c index 14f3fc88f657..5991289ee296 100644 --- a/tools/logger/logger.c +++ b/tools/logger/logger.c @@ -104,8 +104,8 @@ static int snapshot(const char *name) for (i = 0; i < ARRAY_SIZE(debugfs); i++) { - sprintf(pinname, "%s/%s", path, debugfs[i]); - sprintf(poutname, "%s.%s.txt", name, debugfs[i]); + snprintf(pinname, sizeof(pinname), "%s/%s", path, debugfs[i]); + snprintf(poutname, sizeof(poutname), "%s.%s.txt", name, debugfs[i]); /* open debugfs for reading */ in_fd = fopen(pinname, "rb"); @@ -132,7 +132,7 @@ static int snapshot(const char *name) if (count != 4) break; - sprintf(buffer, "0x%6.6x: 0x%8.8x\n", addr, val); + snprintf(buffer, sizeof(buffer), "0x%6.6x: 0x%8.8x\n", addr, val); count = fwrite(buffer, 1, strlen(buffer), out_fd); @@ -220,17 +220,15 @@ static void *wait_open(const char *watched_dir, const char *expected_file) const int dwatch = inotify_add_watch(iqueue, watched_dir, IN_CREATE); struct stat expected_stat; void *ret_stream = NULL; - - char * const fpath = malloc(strlen(watched_dir) + 1 + strlen(expected_file) + 1); + const int fpath_len = strlen(watched_dir) + 1 + strlen(expected_file) + 1; + char * const fpath = malloc(fpath_len); if (!fpath) { fprintf(stderr, "error: can't allocate memory\n"); exit(EXIT_FAILURE); } - strcpy(fpath, watched_dir); - strcat(fpath, "/"); - strcat(fpath, expected_file); + snprintf(fpath, fpath_len, "%s/%s", watched_dir, expected_file); /* Not racy because the inotify watch was set first. */ if (!access(fpath, F_OK)) From fbb4eededf92194d28e60897237d9e1e64619939 Mon Sep 17 00:00:00 2001 From: Adrian Warecki Date: Mon, 18 Sep 2023 15:52:14 +0200 Subject: [PATCH 20/20] tools: logger: Fix resources release Improved release of resources when an error is detected. Signed-off-by: Adrian Warecki (cherry picked from commit 636bd70b0a06aa2e01c33d6aa736f75fca538f50) --- tools/logger/logger.c | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/tools/logger/logger.c b/tools/logger/logger.c index 5991289ee296..b6c2b26bd629 100644 --- a/tools/logger/logger.c +++ b/tools/logger/logger.c @@ -134,7 +134,12 @@ static int snapshot(const char *name) snprintf(buffer, sizeof(buffer), "0x%6.6x: 0x%8.8x\n", addr, val); - count = fwrite(buffer, 1, strlen(buffer), out_fd); + i = strlen(buffer); + count = fwrite(buffer, 1, i, out_fd); + if (count != i) { + fprintf(stderr, "error: an error occurred during write to %s: %s\n", + poutname, strerror(errno)); + } addr += 4; } @@ -164,7 +169,12 @@ static int configure_uart(const char *file, unsigned int baud) tio.c_cc[VMIN] = 1; ret = tcsetattr(fd, TCSANOW, &tio); - return ret < 0 ? -errno : fd; + if (ret < 0) { + close(fd); + return -errno; + } + + return fd; } /* Concantenate `config->filter_config` with `input` + `\n` */ @@ -266,7 +276,9 @@ static void *wait_open(const char *watched_dir, const char *expected_file) } fopenit: - stat(fpath, &expected_stat); + if (stat(fpath, &expected_stat)) + goto cleanup; + if ((expected_stat.st_mode & S_IFMT) == S_IFDIR) ret_stream = opendir(fpath); else @@ -362,7 +374,8 @@ int main(int argc, char *argv[]) if (i < 0 || 1 < i) { fprintf(stderr, "%s: invalid option: -e %s\n", APP_NAME, optarg); - return -EINVAL; + ret = -EINVAL; + goto out; } config.relative_timestamps = i; break; @@ -371,7 +384,8 @@ int main(int argc, char *argv[]) config.time_precision = atoi(optarg); if (config.time_precision < 0) { usage(); - return -EINVAL; + ret = -EINVAL; + goto out; } break; case 'g': @@ -401,8 +415,10 @@ int main(int argc, char *argv[]) usage(); } - if (snapshot_file) - return baud ? EINVAL : -snapshot(snapshot_file); + if (snapshot_file) { + ret = baud ? EINVAL : -snapshot(snapshot_file); + goto out; + } if (!config.ldc_file) { fprintf(stderr, "error: Missing ldc file\n");