From 52e82f4c66c9efad0a9c8d3fa3cea78787d0b8ab Mon Sep 17 00:00:00 2001 From: Karol Trzcinski Date: Fri, 25 Sep 2020 08:42:39 +0200 Subject: [PATCH] logger: Refactor fread() error check in logger_read() fread() returns number of readed blocks, 0 when nothing read. Comparison fread return value with "!ret" is quite misleading - may suggests that negative value is returrned after fail. Swapping if content makes flow easier, then first is error check, and eventyally return statement, next try to reopen file. It allows to check error condition only in one place, so there won't be possibility to use different error checks in subsequent stages (like ferror() and errno). in_file alignment with trace entry size check has been added, to warn about corrupted file. Signed-off-by: Karol Trzcinski --- tools/logger/convert.c | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/tools/logger/convert.c b/tools/logger/convert.c index 0a1277c32..f9a3cd658 100644 --- a/tools/logger/convert.c +++ b/tools/logger/convert.c @@ -638,8 +638,20 @@ static int logger_read(void) while (!ferror(global_config->in_fd)) { /* getting entry parameters from dma dump */ ret = fread(&dma_log, sizeof(dma_log), 1, global_config->in_fd); - if (!ret) { - if (global_config->trace && !ferror(global_config->in_fd)) { + if (ret != 1) { + /* + * use ferror (not errno) to check fread fail - + * see https://www.gnu.org/software/gnulib/manual/html_node/fread.html + */ + ret = -ferror(global_config->in_fd); + if (ret) { + log_err("in %s(), fread(..., %s) failed: %s(%d)\n", + __func__, global_config->in_file, + strerror(-ret), ret); + return ret; + } + /* for trace mode, try to reopen */ + if (global_config->trace) { if (freopen(NULL, "rb", global_config->in_fd)) { continue; } else { @@ -648,13 +660,13 @@ static int logger_read(void) strerror(errno), errno); return -errno; } + } else { + /* EOF */ + if (!feof(global_config->in_fd)) + log_err("file '%s' is unaligned with trace entry size (%ld)\n", + global_config->in_file, sizeof(dma_log)); + break; } - ret = -ferror(global_config->in_fd); - if (ret) - log_err("in %s(), fread(..., %s) failed: %s(%d)\n", - __func__, global_config->in_file, - strerror(-ret), ret); - return ret; } /* checking if received trace address is located in