From d06374a0f1dbb35e2e1531301d41c705e4192655 Mon Sep 17 00:00:00 2001 From: Andrew Wood Date: Sun, 22 Oct 2023 00:08:35 +0100 Subject: [PATCH] Addressed more issues raised by splint and flawfinder. --- src/include/pv-internal.h | 4 ++-- src/pv/loop.c | 43 ++++++++++++++++++++++----------------- src/pv/watchpid.c | 22 ++++++++++---------- 3 files changed, 37 insertions(+), 32 deletions(-) diff --git a/src/include/pv-internal.h b/src/include/pv-internal.h index 2e2f201..46583fd 100644 --- a/src/include/pv-internal.h +++ b/src/include/pv-internal.h @@ -337,9 +337,9 @@ void pv_remote_fini(pvstate_t); int pv_remote_set(pvstate_t); int pv_watchfd_info(pvstate_t, pvwatchfd_t, bool); -int pv_watchfd_changed(pvwatchfd_t); +bool pv_watchfd_changed(pvwatchfd_t); off_t pv_watchfd_position(pvwatchfd_t); -int pv_watchpid_scanfds(pvstate_t, pvstate_t, unsigned int, int *, pvwatchfd_t *, pvstate_t *, int *); +int pv_watchpid_scanfds(pvstate_t, pid_t, int *, pvwatchfd_t *, pvstate_t *, int *); void pv_watchpid_setname(pvstate_t, pvwatchfd_t); #ifdef __cplusplus diff --git a/src/pv/loop.c b/src/pv/loop.c index b1d618d..0bfc75e 100644 --- a/src/pv/loop.c +++ b/src/pv/loop.c @@ -399,8 +399,9 @@ int pv_watchfd_loop(pvstate_t state) char *fmt; while (NULL != (fmt = strstr(state->control.default_format, "%e"))) { debug("%s", "zero size - removing ETA"); - /* strlen-1 here to include trailing NUL */ - memmove(fmt, fmt + 2, strlen(fmt) - 1); + /* strlen-1 here to include trailing \0 */ + memmove(fmt, fmt + 2, strlen(fmt) - 1); /* flawfinder: ignore */ + /* flawfinder: default_format is always \0 terminated */ state->flag.reparse_display = 1; } } @@ -522,13 +523,15 @@ int pv_watchfd_loop(pvstate_t state) * and show details about the transfers on standard error according to the * given options. * + * Replaces format_string in "state" so that starts with "%N " if it doesn't + * already do so. + * * Returns nonzero on error. */ int pv_watchpid_loop(pvstate_t state) { - struct pvstate_s state_copy; const char *original_format_string; - char new_format_string[512]; + char new_format_string[512]; /* flawfinder: ignore */ struct pvwatchfd_s *info_array = NULL; struct pvstate_s *state_array = NULL; int array_length = 0; @@ -538,6 +541,12 @@ int pv_watchpid_loop(pvstate_t state) int prev_displayed_lines, blank_lines; bool first_pass = true; + /* + * flawfinder rationale (new_format_string): zeroed with memset(), + * only written to with pv_snprintf() which checks boundaries, and + * explicitly terminated with \0. + */ + /* * Make sure the process exists first, so we can give an error if * it's not there at the start. @@ -548,28 +557,24 @@ int pv_watchpid_loop(pvstate_t state) return 2; } - /* - * Make a copy of our state, ready to change in preparation for - * duplication. - */ - memcpy(&state_copy, state, sizeof(state_copy)); - /* * Make sure there's a format string, and then insert %N into it if * it's not present. */ original_format_string = - state->control.format_string ? state->control.format_string : state->control.default_format; + NULL != state->control.format_string ? state->control.format_string : state->control.default_format; memset(new_format_string, 0, sizeof(new_format_string)); - if (NULL == strstr(original_format_string, "%N")) { + if (NULL == original_format_string) { + (void) pv_snprintf(new_format_string, sizeof(new_format_string), "%%N"); + } else if (NULL == strstr(original_format_string, "%N")) { (void) pv_snprintf(new_format_string, sizeof(new_format_string), "%%N %s", original_format_string); } else { (void) pv_snprintf(new_format_string, sizeof(new_format_string), "%s", original_format_string); } new_format_string[sizeof(new_format_string) - 1] = '\0'; - state_copy.control.format_string = NULL; - (void) pv_snprintf(state_copy.control.default_format, PV_SIZEOF_DEFAULT_FORMAT, "%.510s", new_format_string); - state_copy.control.default_format[PV_SIZEOF_DEFAULT_FORMAT - 1] = '\0'; + if (NULL != state->control.format_string) + free(state->control.format_string); + state->control.format_string = pv_strdup(new_format_string); /* * Get things ready for the main loop. @@ -628,7 +633,7 @@ int pv_watchpid_loop(pvstate_t state) if (1 == state->flag.terminal_resized) { state->flag.terminal_resized = 0; pv_screensize(&(state->control.width), &(state->control.height)); - for (idx = 0; idx < array_length; idx++) { + for (idx = 0; NULL != state_array && NULL != info_array && idx < array_length; idx++) { state_array[idx].control.width = state->control.width; state_array[idx].control.height = state->control.height; pv_watchpid_setname(state, &(info_array[idx])); @@ -636,8 +641,8 @@ int pv_watchpid_loop(pvstate_t state) } } - rc = pv_watchpid_scanfds(state, &state_copy, - state->control.watch_pid, &array_length, &info_array, &state_array, fd_to_idx); + rc = pv_watchpid_scanfds(state, state->control.watch_pid, &array_length, &info_array, &state_array, + fd_to_idx); if (rc != 0) { if (first_pass) { pv_error(state, "%s %u: %s", _("pid"), state->control.watch_pid, strerror(errno)); @@ -654,7 +659,7 @@ int pv_watchpid_loop(pvstate_t state) first_pass = false; displayed_lines = 0; - for (fd = 0; fd < FD_SETSIZE; fd++) { + for (fd = 0; fd < FD_SETSIZE && NULL != info_array && NULL != state_array; fd++) { off_t position_now, transferred_since_last; struct timespec init_time, transfer_elapsed; long double elapsed_seconds; diff --git a/src/pv/watchpid.c b/src/pv/watchpid.c index f94805c..ddb9f5e 100644 --- a/src/pv/watchpid.c +++ b/src/pv/watchpid.c @@ -177,16 +177,16 @@ int pv_watchfd_info(pvstate_t state, pvwatchfd_t info, bool automatic) #endif #ifdef __APPLE__ -int pv_watchfd_changed(pvwatchfd_t info) +bool pv_watchfd_changed(pvwatchfd_t info) { - return 1; + return true; } #else /* - * Return nonzero if the given file descriptor has changed in some way since - * we started looking at it (i.e. changed destination or permissions). + * Return true if the given file descriptor has changed in some way since + * we started looking at it (i.e. changed destination or permissions). */ -int pv_watchfd_changed(pvwatchfd_t info) +bool pv_watchfd_changed(pvwatchfd_t info) { struct stat sb_fd, sb_fd_link; @@ -196,13 +196,13 @@ int pv_watchfd_changed(pvwatchfd_t info) || (sb_fd.st_ino != info->sb_fd.st_ino) || (sb_fd_link.st_mode != info->sb_fd_link.st_mode) ) { - return 1; + return true; } } else { - return 1; + return true; } - return 0; + return false; } #endif @@ -278,8 +278,8 @@ static int pidfds(pvstate_t state, unsigned int pid, struct proc_fdinfo **fds, i * Returns 0 on success, 1 if the process no longer exists or could not be * read, or 2 for a memory allocation error. */ -int pv_watchpid_scanfds(pvstate_t state, pvstate_t pristine, - unsigned int watch_pid, int *array_length_ptr, +int pv_watchpid_scanfds(pvstate_t state, + pid_t watch_pid, int *array_length_ptr, pvwatchfd_t * info_array_ptr, pvstate_t * state_array_ptr, int *fd_to_idx) { int array_length = 0; @@ -395,7 +395,7 @@ int pv_watchpid_scanfds(pvstate_t state, pvstate_t pristine, /* * Initialise the details of this new entry. */ - memcpy(&(state_array[use_idx]), pristine, sizeof(*pristine)); + memcpy(&(state_array[use_idx]), state, sizeof(*state)); memset(&(info_array[use_idx]), 0, sizeof(info_array[use_idx])); info_array[use_idx].watch_pid = watch_pid;