From e8dab7daf7a55e4b4ea18afaeba1f098286f731c Mon Sep 17 00:00:00 2001 From: Andrew Wood Date: Thu, 7 Sep 2023 22:59:19 +0100 Subject: [PATCH] Addressed more issues raised by splint. --- src/include/pv-internal.h | 3 +- src/pv/file.c | 79 +++++++++++++++++++++++++++++++++++---- src/pv/state.c | 2 +- src/pv/transfer.c | 15 +++++--- 4 files changed, 84 insertions(+), 15 deletions(-) diff --git a/src/include/pv-internal.h b/src/include/pv-internal.h index ff72d38..3a4f049 100644 --- a/src/include/pv-internal.h +++ b/src/include/pv-internal.h @@ -129,7 +129,7 @@ struct pvstate_s { ******************/ const char *program_name; /* program name for error reporting */ char cwd[PV_SIZEOF_CWD]; /* current working directory for relative path */ - const char *current_file; /* current file being read */ + int current_input_file; /* index of current file being read */ int exit_status; /* exit status to give (0=OK) */ /******************* @@ -288,6 +288,7 @@ void pv_display(pvstate_t, long double, long long, long long); long pv_transfer(pvstate_t, int, int *, int *, unsigned long long, long *); void pv_set_buffer_size(unsigned long long, int); int pv_next_file(pvstate_t, unsigned int, int); +/*@out@*/ const char *pv_current_file_name(pvstate_t); void pv_write_retry(int, const char *, size_t); diff --git a/src/pv/file.c b/src/pv/file.c index 81d980e..3992e6e 100644 --- a/src/pv/file.c +++ b/src/pv/file.c @@ -19,6 +19,7 @@ #include #include + /*@-type@*/ /* splint has trouble with off_t and mode_t throughout this file. */ @@ -249,7 +250,7 @@ unsigned long long pv_calc_total_size(pvstate_t state) * error). It is an error if the next input file is the same as the file * stdout is pointing to. * - * Updates state->current_file in the process. + * Updates state->current_input_file in the process. */ int pv_next_file(pvstate_t state, unsigned int filenum, int oldfd) { @@ -320,16 +321,20 @@ int pv_next_file(pvstate_t state, unsigned int filenum, int oldfd) return -1; } - state->current_file = state->input_files[filenum]; - if (0 == strcmp(state->input_files[filenum], "-")) { - state->current_file = "(stdin)"; - } + state->current_input_file = filenum; #ifdef O_DIRECT /* * Set or clear O_DIRECT on the file descriptor. */ if (0 != fcntl(fd, F_SETFL, (state->direct_io ? O_DIRECT : 0) | fcntl(fd, F_GETFL))) { - debug("%s: %s: %s", state->current_file, "fcntl", strerror(errno)); + /*@-compdef@ */ + /* + * splint - passed or returned storage is undefined - but at + * this point we know the input file list is been populated, + * so that's OK. + */ + debug("%s: %s: %s", pv_current_file_name(state), "fcntl", strerror(errno)); + /*@+compdef@ */ } /* * We don't clear direct_io_changed here, to avoid race conditions @@ -337,9 +342,69 @@ int pv_next_file(pvstate_t state, unsigned int filenum, int oldfd) */ #endif /* O_DIRECT */ - debug("%s: %d: %s: fd=%d", "next file opened", filenum, state->current_file, fd); + debug("%s: %d: %s: fd=%d", "next file opened", filenum, pv_current_file_name(state), fd); return fd; } + +/* + * Return the name of the current file. The returned buffer may point to + * internal state and must not be passed to free() or used after "state" is + * freed. + */ +/*@out@*/ const char *pv_current_file_name(pvstate_t state) +{ + static char *str_none = NULL; + static char *str_stdin = NULL; + const char *input_file_name = NULL; + + /*@-observertrans@ */ + /*@-onlytrans@ */ + /*@-statictrans@ */ + /* + * Here we are doing bad things with regards to whether the returned + * string is an allocated string from the state->input_files array, + * a constant string, or a returned string from gettext(), but it + * has no impact. We explicitly document, above, that the returned + * string expires with the state, and hence switch off the + * associated splint warnings. + */ + + if (NULL == str_none) + str_none = _("(none)"); + if (NULL == str_stdin) + str_stdin = _("(stdin)"); + + /* Fallback in case of translation failure. */ + if (NULL == str_none) + str_none = "(none)"; + if (NULL == str_stdin) + str_stdin = "(stdin)"; + + if (state->current_input_file < 0) + return str_none; + if ((unsigned int) (state->current_input_file) >= state->input_file_count) + return str_none; + + input_file_name = state->input_files[state->current_input_file]; + if (NULL == input_file_name) + return str_none; + if (0 == strcmp(input_file_name, "-")) + return str_stdin; + + /*@-compdef@ */ + return input_file_name; + /*@+compdef@ */ + /* + * splint warns about state->input_files being undefined, but we + * know it's been populated fully by the time this function is + * called. + */ + + /*@+statictrans@ */ + /*@+onlytrans@ */ + /*@+observertrans@ */ +} + /* EOF */ diff --git a/src/pv/state.c b/src/pv/state.c index 7117de2..45db642 100644 --- a/src/pv/state.c +++ b/src/pv/state.c @@ -56,7 +56,7 @@ pvstate_t pv_state_alloc(const char *program_name) state->crs_lock_fd = -1; state->reparse_display = 1; - state->current_file = _("none"); + state->current_input_file = -1; #ifdef HAVE_SPLICE state->splice_failed_fd = -1; #endif /* HAVE_SPLICE */ diff --git a/src/pv/transfer.c b/src/pv/transfer.c index 4d41ba8..c936223 100644 --- a/src/pv/transfer.c +++ b/src/pv/transfer.c @@ -367,7 +367,7 @@ static int pv__transfer_read(pvstate_t state, int fd, int *eof_in, int *eof_out, * reached the end of this file. */ if (do_not_skip_errors) { - pv_error(state, "%s: %s: %s", state->current_file, _("read failed"), strerror(errno)); + pv_error(state, "%s: %s: %s", pv_current_file_name(state), _("read failed"), strerror(errno)); *eof_in = 1; if (state->write_position >= state->read_position) { *eof_out = 1; @@ -382,7 +382,8 @@ static int pv__transfer_read(pvstate_t state, int fd, int *eof_in, int *eof_out, amount_skipped = -1; if (!state->read_error_warning_shown) { - pv_error(state, "%s: %s: %s", state->current_file, _("warning: read errors detected"), strerror(errno)); + pv_error(state, "%s: %s: %s", pv_current_file_name(state), _("warning: read errors detected"), + strerror(errno)); state->read_error_warning_shown = 1; } @@ -394,7 +395,7 @@ static int pv__transfer_read(pvstate_t state, int fd, int *eof_in, int *eof_out, * of the file. */ if (0 > orig_offset) { - pv_error(state, "%s: %s: %s", state->current_file, _("file is not seekable"), strerror(errno)); + pv_error(state, "%s: %s: %s", pv_current_file_name(state), _("file is not seekable"), strerror(errno)); *eof_in = 1; if (state->write_position >= state->read_position) { *eof_out = 1; @@ -462,7 +463,8 @@ static int pv__transfer_read(pvstate_t state, int fd, int *eof_in, int *eof_out, */ if (EINVAL != errno) { pv_error(state, - "%s: %s: %s", state->current_file, _("failed to seek past error"), strerror(errno)); + "%s: %s: %s", pv_current_file_name(state), _("failed to seek past error"), + strerror(errno)); } } else { amount_skipped = skip_offset - orig_offset; @@ -477,7 +479,7 @@ static int pv__transfer_read(pvstate_t state, int fd, int *eof_in, int *eof_out, state->read_position += amount_skipped; if (state->skip_errors < 2) { pv_error(state, "%s: %s: %ld - %ld (%ld %s)", - state->current_file, + pv_current_file_name(state), _("skipped past read error"), orig_offset, skip_offset, amount_skipped, _("B")); } } else { @@ -854,7 +856,8 @@ long pv_transfer(pvstate_t state, int fd, int *eof_in, int *eof_out, unsigned lo /* * Any other error is a problem and we must report back. */ - pv_error(state, "%s: %s: %d: %s", state->current_file, _("select call failed"), n, strerror(errno)); + pv_error(state, "%s: %s: %d: %s", pv_current_file_name(state), _("select call failed"), n, + strerror(errno)); state->exit_status |= 16;