diff --git a/src/include/pv-internal.h b/src/include/pv-internal.h index e2772fd..9c18ed2 100644 --- a/src/include/pv-internal.h +++ b/src/include/pv-internal.h @@ -131,6 +131,12 @@ typedef uint16_t pvdisplay_bytecount_t; typedef uint16_t pvdisplay_width_t; #define PVDISPLAY_WIDTH_MAX (65535) /* UINT16_MAX */ +/* + * Structure defining the current state of a single watched file descriptor. + */ +struct pvwatchfd_s; +typedef /*@null@*/ struct pvwatchfd_s *pvwatchfd_t; + /* String pointer, that is the only pointer to this resource, that can be null. */ typedef /*@only@*/ /*@null@*/ char * nullable_string_t; @@ -163,9 +169,16 @@ struct pvstate_s { /********************************* * Items to watch with --watchfd * *********************************/ - struct pvwatchspec_s { - /*@only@*/ /*@null@*/ pid_t *pid; /* array of processes to watch fds of */ - /*@only@*/ /*@null@*/ int *fd; /* array of fds to watch in each one (0=all) */ + struct { + /*@only@*/ /*@null@*/ + struct pvwatcheditem_s { /* array of PID or PID:FD items */ + pid_t pid; /* watched PID */ + int fd; /* watched fd, or -1 for all */ + /*@null@*/ + pvwatchfd_t info_array; /* watch information for each fd */ + int array_length; /* length of watch info array */ + bool finished; /* "PID:FD": fd closed; or PID gone */ + } *watching; unsigned int count; /* number of items in these arrays */ bool multiple_pids; /* true if more than one distinct PID */ } watchfd; @@ -468,6 +481,36 @@ typedef struct pvtransfercalc_s *pvtransfercalc_t; typedef struct pvcursorstate_s *pvcursorstate_t; typedef struct pvtransferstate_s *pvtransferstate_t; +/* + * Structure defining the current state of a single watched file descriptor. + * The full definition needs to go here as it refers to sub-structures of + * the main state, defined above. + */ +struct pvwatchfd_s { + struct pvtransientflags_s flags; /* transient flags */ + struct pvtransferstate_s transfer; /* transfer state */ + struct pvtransfercalc_s calc; /* calculated transfer state */ + struct pvdisplay_s display; /* display data */ +#ifdef __APPLE__ +#else + char file_fdinfo[PV_SIZEOF_FILE_FDINFO]; /* path to /proc fdinfo file */ + char file_fd[PV_SIZEOF_FILE_FD]; /* path to /proc fd symlink */ +#endif + char file_fdpath[PV_SIZEOF_FILE_FDPATH]; /* path to file that was opened */ + /*@keep@ */ char display_name[PV_SIZEOF_DISPLAY_NAME]; /* name to show on progress bar */ + struct stat sb_fd; /* stat of fd symlink */ + struct stat sb_fd_link; /* lstat of fd symlink */ + off_t size; /* size of whole file, 0 if unknown */ + off_t position; /* position last seen at */ + struct timespec start_time; /* time we started watching the fd */ + struct timespec end_time; /* time the fd was marked as closed */ + pid_t watch_pid; /* PID the fd belongs to */ + int watch_fd; /* fd to watch */ + bool closed; /* true once the fd is closed */ + bool displayable; /* false if not displayable */ + bool unused; /* true if free for re-use */ +}; + /* * Read-only counterparts to the above structure pointers, to be used in * function declarations where the function definitely shouldn't be altering @@ -514,36 +557,6 @@ struct pvdisplay_component_s { bool dynamic; /* whether it can scale with screen size */ }; - -/* - * Structure defining the current state of a single watched file descriptor. - */ -struct pvwatchfd_s { - struct pvtransientflags_s flags; /* transient flags */ - struct pvtransferstate_s transfer; /* transfer state */ - struct pvtransfercalc_s calc; /* calculated transfer state */ - struct pvdisplay_s display; /* display data */ -#ifdef __APPLE__ -#else - char file_fdinfo[PV_SIZEOF_FILE_FDINFO]; /* path to /proc fdinfo file */ - char file_fd[PV_SIZEOF_FILE_FD]; /* path to /proc fd symlink */ -#endif - char file_fdpath[PV_SIZEOF_FILE_FDPATH]; /* path to file that was opened */ - /*@keep@ */ char display_name[PV_SIZEOF_DISPLAY_NAME]; /* name to show on progress bar */ - struct stat sb_fd; /* stat of fd symlink */ - struct stat sb_fd_link; /* lstat of fd symlink */ - off_t size; /* size of whole file, 0 if unknown */ - off_t position; /* position last seen at */ - struct timespec start_time; /* time we started watching the fd */ - struct timespec end_time; /* time the fd was marked as closed */ - pid_t watch_pid; /* PID the fd belongs to */ - int watch_fd; /* fd to watch */ - bool closed; /* true once the fd is closed */ - bool displayable; /* false if not displayable */ - bool unused; /* true if free for re-use */ -}; -typedef struct pvwatchfd_s *pvwatchfd_t; - void pv_error(char *, ...); int pv_main_loop(pvstate_t); @@ -620,6 +633,7 @@ void pv_freecontents_display(pvdisplay_t); void pv_freecontents_transfer(pvtransferstate_t); void pv_freecontents_calc(pvtransfercalc_t); void pv_freecontents_watchfd(pvwatchfd_t); +void pv_freecontents_watchfd_items(struct pvwatcheditem_s *, unsigned int); void pv_write_retry(int, const char *, size_t); void pv_tty_write(readonly_pvtransientflags_t, const char *, size_t); diff --git a/src/pv/loop.c b/src/pv/loop.c index 1ea374b..cf25007 100644 --- a/src/pv/loop.c +++ b/src/pv/loop.c @@ -694,9 +694,7 @@ static void pv_watchfd_update_format_string(pvstate_t state) /* If there's nothing to watch, do nothing. */ if (state->watchfd.count < 1) return; - if (NULL == state->watchfd.pid) - return; - if (NULL == state->watchfd.fd) + if (NULL == state->watchfd.watching) return; /* @@ -712,7 +710,7 @@ static void pv_watchfd_update_format_string(pvstate_t state) memset(new_format_string, 0, sizeof(new_format_string)); - if (state->watchfd.count > 1 || -1 == state->watchfd.fd[0]) { + if (state->watchfd.count > 1 || -1 == state->watchfd.watching[0].fd) { /* Watching more than one single FD; need %N. */ if (!format_contains_name(original_format_string)) { (void) pv_snprintf(new_format_string, sizeof(new_format_string), "%%N %s", @@ -753,13 +751,7 @@ static void pv_watchfd_update_format_string(pvstate_t state) */ int pv_watchfd_loop(pvstate_t state) { - struct { /* details for each PID:FD specified by operator */ - pid_t pid; /* watched PID */ - int fd; /* watched fd, or -1 for all */ - pvwatchfd_t info_array; /* watch information for each fd */ - int array_length; /* length of watch info array */ - bool finished; /* "PID:FD": fd closed; or PID gone */ - } *watching; + struct pvwatcheditem_s *watching; unsigned int watch_idx; bool all_watching_finished; struct timespec next_update, next_remotecheck, cur_time; @@ -776,11 +768,12 @@ int pv_watchfd_loop(pvstate_t state) /* If there's nothing to watch, do nothing at all. */ if (state->watchfd.count < 1) return 0; - if (NULL == state->watchfd.pid) - return PV_ERROREXIT_MEMORY; - if (NULL == state->watchfd.fd) + if (NULL == state->watchfd.watching) return PV_ERROREXIT_MEMORY; + /* Local alias for less cumbersome access. */ + watching = state->watchfd.watching; + /* * Make sure there's no name set. */ @@ -792,16 +785,6 @@ int pv_watchfd_loop(pvstate_t state) /* Adjust the format string so PID:FD or FD prefixes are shown. */ pv_watchfd_update_format_string(state); - /* - * Allocate the watching array. - */ - watching = malloc(state->watchfd.count * sizeof(*watching)); - if (NULL == watching) { - pv_error("%s: %s", _("buffer allocation failed"), strerror(errno)); - return PV_ERROREXIT_MEMORY; - } - memset(watching, 0, state->watchfd.count * sizeof(*watching)); - /* * Populate the watching array. In the process of doing so, raise * errors if any of the initially specified PIDs or PID:FD pairs @@ -810,8 +793,6 @@ int pv_watchfd_loop(pvstate_t state) for (watch_idx = 0; watch_idx < state->watchfd.count; watch_idx++) { int rc; - watching[watch_idx].pid = state->watchfd.pid[watch_idx]; - watching[watch_idx].fd = state->watchfd.fd[watch_idx]; watching[watch_idx].info_array = NULL; watching[watch_idx].array_length = 0; watching[watch_idx].finished = false; @@ -849,7 +830,7 @@ int pv_watchfd_loop(pvstate_t state) /* Scan failed - mark as finished. */ state->status.exit_status |= PV_ERROREXIT_ACCESS; watching[watch_idx].finished = true; - } else if (0 == watching[watch_idx].array_length) { + } else if ((0 == watching[watch_idx].array_length) || (NULL == watching[watch_idx].info_array)) { /* FD not found - error, mark as finished. */ pv_error("%s %u: %s %d: %s", _("pid"), watching[watch_idx].pid, _("fd"), watching[watch_idx].fd, @@ -1189,29 +1170,10 @@ int pv_watchfd_loop(pvstate_t state) state->status.exit_status |= PV_ERROREXIT_SIGNAL; end_pv_watchfd_loop: - /* Free all allocated memory. */ - /*@-compdestroy@ */ - for (watch_idx = 0; NULL != watching && watch_idx < state->watchfd.count; watch_idx++) { - int info_idx; - if (NULL == watching[watch_idx].info_array) - continue; - for (info_idx = 0; info_idx < watching[watch_idx].array_length; info_idx++) { - pv_freecontents_watchfd(&(watching[watch_idx].info_array[info_idx])); - } - free(watching[watch_idx].info_array); - watching[watch_idx].info_array = NULL; - watching[watch_idx].array_length = 0; - } - free(watching); - watching = NULL; + /* Free all allocated sub-structures. */ + pv_freecontents_watchfd_items(watching, state->watchfd.count); return state->status.exit_status; - /*@+compdestroy @ */ - /* - * splint warns that watching[].info_array->transfer.transfer_buffer - * and other deep structures may not be deallocated and so leak - * memory, but that's what pv_freecontents_watchfd() takes care of. - */ } diff --git a/src/pv/state.c b/src/pv/state.c index a5c877d..ba30016 100644 --- a/src/pv/state.c +++ b/src/pv/state.c @@ -257,6 +257,39 @@ void pv_freecontents_calc(pvtransfercalc_t calc) } +/* + * Free the contents of a watchfd watched-items array. + */ +void pv_freecontents_watchfd_items(struct pvwatcheditem_s *watching, unsigned int count) +{ + unsigned int watch_idx; + + if (NULL == watching) + return; + + /*@-compdestroy@ */ + for (watch_idx = 0; watch_idx < count; watch_idx++) { + int info_idx; + if (NULL == watching[watch_idx].info_array) + continue; + for (info_idx = 0; info_idx < watching[watch_idx].array_length; info_idx++) { + pv_freecontents_watchfd(&(watching[watch_idx].info_array[info_idx])); + } + free(watching[watch_idx].info_array); + watching[watch_idx].info_array = NULL; + watching[watch_idx].array_length = 0; + } + + return; + /*@+compdestroy @ */ + /* + * splint warns that watching[].info_array->transfer.transfer_buffer + * and other deep structures may not be deallocated and so leak + * memory, but that's what pv_freecontents_watchfd() takes care of. + */ +} + + /* * Truncate the output file descriptor to its current position, if it's a * valid fd, we're in sparse output mode, and no lseek() failed. @@ -362,13 +395,10 @@ void pv_state_free(pvstate_t state) state->files.filename = NULL; } - if (NULL != state->watchfd.pid) { - free(state->watchfd.pid); - state->watchfd.pid = NULL; - } - if (NULL != state->watchfd.fd) { - free(state->watchfd.fd); - state->watchfd.fd = NULL; + if (NULL != state->watchfd.watching) { + pv_freecontents_watchfd_items(state->watchfd.watching, state->watchfd.count); + free(state->watchfd.watching); + state->watchfd.watching = NULL; } free(state); @@ -788,43 +818,38 @@ void pv_state_inputfiles(pvstate_t state, unsigned int input_file_count, const c void pv_state_watchfds(pvstate_t state, unsigned int watchfd_count, const pid_t * pids, const int *fds) { unsigned int item_idx; - /*@only@ */ pid_t *new_pid_array = NULL; - /*@only@ */ int *new_fd_array = NULL; + /*@only@ */ struct pvwatcheditem_s *new_array = NULL; /* Free the old arrays, if there were any. */ - if (NULL != state->watchfd.pid) { - free(state->watchfd.pid); - state->watchfd.pid = NULL; - } - if (NULL != state->watchfd.fd) { - free(state->watchfd.fd); - state->watchfd.fd = NULL; + if (NULL != state->watchfd.watching) { + /*@-compdestroy@ */ + pv_freecontents_watchfd_items(state->watchfd.watching, state->watchfd.count); + free(state->watchfd.watching); + state->watchfd.watching = NULL; + /*@+compdestroy@ */ + /* + * splint warns about deep structures not being deallocated, + * but that's what pv_freecontents_watchfd_items() does. + */ } state->watchfd.count = 0; state->watchfd.multiple_pids = false; - /* Allocate empty new arrays of the right size. */ - new_pid_array = calloc((size_t) (watchfd_count + 1), sizeof(pid_t)); - if (NULL == new_pid_array) { + /* Allocate an empty new array of the right size. */ + new_array = malloc((1 + state->watchfd.count) * sizeof(*new_array)); + if (NULL == new_array) { /*@-mustfreefresh@ *//* see similar _() issue above */ - pv_error("%s: %s", _("process list allocation failed"), strerror(errno)); + pv_error("%s: %s", _("buffer allocation failed"), strerror(errno)); /*@+mustfreefresh@ */ return; } - state->watchfd.pid = new_pid_array; - new_fd_array = calloc((size_t) (watchfd_count + 1), sizeof(int)); - if (NULL == new_fd_array) { - /*@-mustfreefresh@ *//* see similar _() issue above */ - pv_error("%s: %s", _("file descriptor list allocation failed"), strerror(errno)); - /*@+mustfreefresh@ */ - return; - } - state->watchfd.fd = new_fd_array; + memset(new_array, 0, (1 + state->watchfd.count) * sizeof(*new_array)); + state->watchfd.watching = new_array; - /* Populate the new arrays with the values supplied. */ + /* Populate the new array with the values supplied. */ for (item_idx = 0; item_idx < watchfd_count; item_idx++) { - state->watchfd.pid[item_idx] = pids[item_idx]; - state->watchfd.fd[item_idx] = fds[item_idx]; + state->watchfd.watching[item_idx].pid = pids[item_idx]; + state->watchfd.watching[item_idx].fd = fds[item_idx]; if ((item_idx > 0) && (pids[item_idx] != pids[item_idx - 1])) state->watchfd.multiple_pids = true; } diff --git a/src/pv/watchpid.c b/src/pv/watchpid.c index e20e3e5..7ad2c72 100644 --- a/src/pv/watchpid.c +++ b/src/pv/watchpid.c @@ -42,6 +42,8 @@ */ static bool filesize(pvwatchfd_t info) { + if (NULL == info) + return false; if (S_ISBLK(info->sb_fd.st_mode)) { int fd; @@ -222,6 +224,9 @@ bool pv_watchfd_changed(pvwatchfd_t info) { struct stat sb_fd, sb_fd_link; + if (NULL == info) + return false; + memset(&sb_fd, 0, sizeof(sb_fd)); memset(&sb_fd_link, 0, sizeof(sb_fd_link)); @@ -265,6 +270,9 @@ off_t pv_watchfd_position(pvwatchfd_t info) long long pos_long; FILE *fptr; + if (NULL == info) + return -1; + if (pv_watchfd_changed(info)) return -1; @@ -489,7 +497,7 @@ int pv_watchpid_scanfds(pvstate_t state, pid_t watch_pid, int watch_fd, int *arr * Skip if this fd is already known to us. */ found_idx = -1; - for (check_idx = 0; check_idx < array_length; check_idx++) { + for (check_idx = 0; check_idx < array_length && NULL != info_array; check_idx++) { if (info_array[check_idx].unused) continue; if (info_array[check_idx].watch_fd != fd) @@ -514,7 +522,7 @@ int pv_watchpid_scanfds(pvstate_t state, pid_t watch_pid, int watch_fd, int *arr * See if there's an empty slot we can re-use. */ use_idx = -1; - for (check_idx = 0; check_idx < array_length; check_idx++) { + for (check_idx = 0; check_idx < array_length && NULL != info_array; check_idx++) { if (info_array[check_idx].unused) { use_idx = check_idx; break; @@ -532,6 +540,10 @@ int pv_watchpid_scanfds(pvstate_t state, pid_t watch_pid, int watch_fd, int *arr use_idx = array_length - 1; } + /* At this point, the array should exist. */ + if (NULL == info_array) + return 2; + debug("%s: %d => index %d", "found new fd", fd, use_idx); changes_made = true; @@ -638,7 +650,12 @@ void pv_watchpid_setname(pvstate_t state, pvwatchfd_t info) { size_t path_length, cwd_length; int max_display_length; - char *file_fdpath = info->file_fdpath; + char *file_fdpath; + + if (NULL == info) + return; + + file_fdpath = info->file_fdpath; memset(info->display_name, 0, PV_SIZEOF_DISPLAY_NAME);