From ad1d0e1c9a3ea207beea5e000943343253ea40d3 Mon Sep 17 00:00:00 2001 From: Andrew Wood Date: Mon, 28 Jul 2025 00:25:14 +0100 Subject: [PATCH] In the watchfd info array, instead of using copies of the global state, each fd has its own flags, transfer, calc, and display sub-structures (#165). --- src/include/pv-internal.h | 10 ++- src/pv/loop.c | 67 +++++++------------ src/pv/state.c | 64 ++++++++++++------ src/pv/watchpid.c | 132 +++++++++++--------------------------- 4 files changed, 113 insertions(+), 160 deletions(-) diff --git a/src/include/pv-internal.h b/src/include/pv-internal.h index bd901f3..d7f96de 100644 --- a/src/include/pv-internal.h +++ b/src/include/pv-internal.h @@ -488,6 +488,10 @@ struct pvdisplay_component_s { 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 */ @@ -500,7 +504,6 @@ struct pvwatchfd_s { 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 */ - /*@null@*/ pvstate_t state; /* state object for flags and display */ pid_t watch_pid; /* PID to watch */ int watch_fd; /* fd to watch, -1 = not displayed */ }; @@ -577,6 +580,11 @@ void pv_reset_calc(pvtransfercalc_t); void pv_reset_transfer(pvtransferstate_t); void pv_reset_flags(pvtransientflags_t); void pv_reset_display(pvdisplay_t); +void pv_reset_watchfd(pvwatchfd_t); +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_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 9607b41..6efa0be 100644 --- a/src/pv/loop.c +++ b/src/pv/loop.c @@ -602,9 +602,11 @@ int pv_watchfd_loop(pvstate_t state) memset(&info, 0, sizeof(info)); info.watch_pid = state->control.watch_pid; info.watch_fd = state->control.watch_fd; + pv_reset_watchfd(&info); rc = pv_watchfd_info(state, &info, false); if (0 != rc) { state->status.exit_status |= PV_ERROREXIT_ACCESS; + pv_freecontents_watchfd(&info); /*@-compdestroy@ */ return state->status.exit_status; /*@+compdestroy@ */ @@ -734,15 +736,7 @@ int pv_watchfd_loop(pvstate_t state) if (1 == state->flags.trigger_exit) state->status.exit_status |= PV_ERROREXIT_SIGNAL; - /* - * Free the state structure specific to this file descriptor. - * Unused so far - this is so that in future we could watch - * multiple file descriptors similar to pv_watchpid_loop(). - */ - if (NULL != info.state) { - pv_state_free(info.state); - info.state = NULL; - } + pv_freecontents_watchfd(&info); /*@-compdestroy@ */ return state->status.exit_status; @@ -884,12 +878,10 @@ int pv_watchpid_loop(pvstate_t state) state->control.height = new_height; for (idx = 0; NULL != info_array && idx < array_length; idx++) { - if (NULL == info_array[idx].state) + if (info_array[idx].watch_fd < 0) continue; - info_array[idx].state->control.width = state->control.width; - info_array[idx].state->control.height = state->control.height; pv_watchpid_setname(state, &(info_array[idx])); - info_array[idx].state->flags.reparse_display = 1; + info_array[idx].flags.reparse_display = 1; } } @@ -928,18 +920,15 @@ int pv_watchpid_loop(pvstate_t state) if (pv_watchfd_changed(&(info_array[idx]))) { fd_to_idx[fd] = -1; info_array[idx].watch_pid = 0; - if (NULL != info_array[idx].state) - pv_state_free(info_array[idx].state); - /*@-mustfreeonly@ *//* not a leak - we've just free()d it. */ - info_array[idx].state = NULL; - /*@+mustfreeonly@ */ + info_array[idx].watch_fd = -1; + pv_freecontents_watchfd(&(info_array[idx])); debug("%s %d: %s", "fd", fd, "removing"); } continue; } - if (NULL == info_array[idx].state) { - debug("%s %d: %s", "fd", fd, "null state - skipping"); + if (info_array[idx].watch_fd < 0) { + debug("%s %d: %s", "fd", fd, "negative fd - skipping"); continue; } @@ -952,11 +941,8 @@ int pv_watchpid_loop(pvstate_t state) if (position_now < 0) { fd_to_idx[fd] = -1; info_array[idx].watch_pid = 0; - if (NULL != info_array[idx].state) - pv_state_free(info_array[idx].state); - /*@-mustfreeonly@ *//* not a leak - we've just free()d it. */ - info_array[idx].state = NULL; - /*@+mustfreeonly@ */ + info_array[idx].watch_fd = -1; + pv_freecontents_watchfd(&(info_array[idx])); debug("%s %d: %s", "fd", fd, "removing"); continue; } @@ -978,32 +964,25 @@ int pv_watchpid_loop(pvstate_t state) */ pv_elapsedtime_subtract(&transfer_elapsed, &cur_time, &init_time); - if (NULL != info_array[idx].state) { - info_array[idx].state->transfer.elapsed_seconds = - pv_elapsedtime_seconds(&transfer_elapsed); - } + info_array[idx].transfer.elapsed_seconds = pv_elapsedtime_seconds(&transfer_elapsed); if (displayed_lines > 0) { debug("%s", "adding newline"); pv_tty_write(&(state->flags), "\n", 1); } - if (NULL == info_array[idx].state) { - debug("%s %d [%d]: %s / %Ld", "fd", fd, idx, "(null state)", position_now); - } else { - debug("%s %d [%d]: %Lf / %Ld", "fd", fd, idx, - info_array[idx].state->transfer.elapsed_seconds, position_now); - } + debug("%s %d [%d]: %Lf / %Ld", "fd", fd, idx, + info_array[idx].transfer.elapsed_seconds, position_now); - if (NULL != info_array[idx].state) { - info_array[idx].state->transfer.transferred = position_now; - info_array[idx].state->transfer.total_written = position_now; + if (info_array[idx].watch_fd >= 0) { + info_array[idx].transfer.transferred = position_now; + info_array[idx].transfer.total_written = position_now; state->control.name = info_array[idx].display_name; state->control.size = info_array[idx].size; pv_display(&(state->status), - &(state->control), &(info_array[idx].state->flags), - &(info_array[idx].state->transfer), &(info_array[idx].state->calc), - &(state->cursor), &(info_array[idx].state->display), NULL, false); + &(state->control), &(info_array[idx].flags), + &(info_array[idx].transfer), &(info_array[idx].calc), + &(state->cursor), &(info_array[idx].display), NULL, false); /*@-mustfreeonly@ */ state->control.name = NULL; /* @@ -1067,10 +1046,8 @@ int pv_watchpid_loop(pvstate_t state) * Free the per-fd state. */ for (idx = 0; NULL != info_array && idx < array_length; idx++) { - if (NULL == info_array[idx].state) - continue; - pv_state_free(info_array[idx].state); - info_array[idx].state = NULL; + pv_freecontents_watchfd(&(info_array[idx])); + info_array[idx].watch_fd = -1; } if (NULL != info_array) diff --git a/src/pv/state.c b/src/pv/state.c index 09a6200..1b0b84e 100644 --- a/src/pv/state.c +++ b/src/pv/state.c @@ -216,6 +216,46 @@ pvstate_t pv_state_alloc(void) } +/* + * Free dynamic contents of a display structure. + */ +void pv_freecontents_display(pvdisplay_t display) +{ + if (NULL != display->display_buffer) + free(display->display_buffer); + display->display_buffer = NULL; +} + + +/* + * Free dynamic contents of a transfer state structure. + */ +void pv_freecontents_transfer(pvtransferstate_t transfer) +{ + /*@-keeptrans@ */ + if (NULL != transfer->transfer_buffer) + free(transfer->transfer_buffer); + transfer->transfer_buffer = NULL; + /*@+keeptrans@ */ + /* splint - explicitly freeing this structure, so free() here is OK. */ + + if (NULL != transfer->line_positions) + free(transfer->line_positions); + transfer->line_positions = NULL; +} + + +/* + * Free dynamic contents of a calculated transfer state structure. + */ +void pv_freecontents_calc(pvtransfercalc_t calc) +{ + if (NULL != calc->history) + free(calc->history); + calc->history = NULL; +} + + /* * Free a state structure, after which it can no longer be used. */ @@ -244,13 +284,8 @@ void pv_state_free(pvstate_t state) state->control.output_name = NULL; } - if (NULL != state->display.display_buffer) - free(state->display.display_buffer); - state->display.display_buffer = NULL; - - if (NULL != state->extra_display.display_buffer) - free(state->extra_display.display_buffer); - state->extra_display.display_buffer = NULL; + pv_freecontents_display(&(state->display)); + pv_freecontents_display(&(state->extra_display)); if (NULL != state->control.name) { free(state->control.name); @@ -272,20 +307,9 @@ void pv_state_free(pvstate_t state) state->control.extra_format_string = NULL; } - /*@-keeptrans@ */ - if (NULL != state->transfer.transfer_buffer) - free(state->transfer.transfer_buffer); - state->transfer.transfer_buffer = NULL; - /*@+keeptrans@ */ - /* splint - explicitly freeing this structure, so free() here is OK. */ + pv_freecontents_transfer(&(state->transfer)); - if (NULL != state->transfer.line_positions) - free(state->transfer.line_positions); - state->transfer.line_positions = NULL; - - if (NULL != state->calc.history) - free(state->calc.history); - state->calc.history = NULL; + pv_freecontents_calc(&(state->calc)); if (NULL != state->files.filename) { unsigned int file_idx; diff --git a/src/pv/watchpid.c b/src/pv/watchpid.c index 482020c..c4cc5f4 100644 --- a/src/pv/watchpid.c +++ b/src/pv/watchpid.c @@ -365,6 +365,33 @@ static bool extend_info_array(int *array_length_ptr, pvwatchfd_t * info_array_pt /*@+compdef @ */ +/* + * Reset calculated values in the given watchfd info structure. + */ +void pv_reset_watchfd(pvwatchfd_t info) +{ + if (NULL == info) + return; + pv_reset_calc(&(info->calc)); + pv_reset_transfer(&(info->transfer)); + pv_reset_flags(&(info->flags)); + pv_reset_display(&(info->display)); +} + + +/* + * Free dynamically allocated areas in the given watchfd info structure. + */ +void pv_freecontents_watchfd(pvwatchfd_t info) +{ + if (NULL == info) + return; + pv_freecontents_calc(&(info->calc)); + pv_freecontents_transfer(&(info->transfer)); + pv_freecontents_display(&(info->display)); +} + + /* * Scan the given process and update the arrays with any new file * descriptors. @@ -413,7 +440,6 @@ int pv_watchpid_scanfds(pvstate_t state, #endif int fd, check_idx, use_idx, rc; off_t position_now; - const char *use_format_string; fd = -1; #ifdef __APPLE__ @@ -464,64 +490,16 @@ int pv_watchpid_scanfds(pvstate_t state, */ memset(&(info_array[use_idx]), 0, sizeof(info_array[use_idx])); + pv_reset_watchfd(&(info_array[use_idx])); info_array[use_idx].watch_pid = watch_pid; info_array[use_idx].watch_fd = fd; - /* Allocate new display state. */ - /*@-mustfreeonly@ *//* splint - this is not a leak, this is a new entry. */ - info_array[use_idx].state = pv_state_alloc(); - /*@+mustfreeonly@ */ - if (NULL == info_array[use_idx].state) - return 2; - - /* Copy the main status.cwd value to the new state. */ - memcpy(&(info_array[use_idx].state->status.cwd), &(state->status.cwd), sizeof(state->status.cwd)); /* flawfinder: ignore */ - /* flawfinder: bounded to destination object size. */ - /* - * Copy over all the control values, blanking out the - * dynamically allocated strings, and setting the default - * format (fixed buffer) to the required format string - * instead of allocating a new dynamic format string for the - * copy (so there's less dynamic allocation going on). + * Set set the average rate window so that a new history + * buffer is allocated for this state. */ - memcpy(&(info_array[use_idx].state->control), &(state->control), sizeof(state->control)); /* flawfinder: ignore */ - /* flawfinder: bounded to destination object size. */ - /*@-mustfreeonly@ *//* splint - this is not a leak, this is a new entry. */ - info_array[use_idx].state->control.name = NULL; - info_array[use_idx].state->control.format_string = NULL; - info_array[use_idx].state->control.output_name = NULL; - info_array[use_idx].state->control.default_bar_style = NULL; - /*@+mustfreeonly@ */ - info_array[use_idx].state->control.default_format[0] = '\0'; - use_format_string = - NULL != state->control.format_string ? state->control.format_string : state->control.default_format; - if (NULL != use_format_string) - (void) pv_snprintf(info_array[use_idx].state->control.default_format, PV_SIZEOF_DEFAULT_FORMAT, - "%.510s", use_format_string); - - /* - * Duplicate the default bar style string, if there is one. - */ - if (NULL != state->control.default_bar_style) { - pv_state_default_bar_style_set(info_array[use_idx].state, state->control.default_bar_style); - } - - /* - * Copy over all the display values, blanking out the - * dynamically allocated parts as above; then set the - * average rate window so that a new history buffer is - * allocated for this state. - */ - memcpy(&(info_array[use_idx].state->display), &(state->display), sizeof(state->display)); /* flawfinder: ignore */ - /* flawfinder: bounded to destination object size. */ - /*@-mustfreeonly@ *//* splint - this is not a leak, this is a new entry. */ - info_array[use_idx].state->display.display_buffer = NULL; - info_array[use_idx].state->display.display_buffer_size = 0; - info_array[use_idx].state->calc.history = NULL; - info_array[use_idx].state->calc.history_len = 0; - /*@+mustfreeonly@ */ - pv_state_average_rate_window_set(info_array[use_idx].state, state->control.average_rate_window); + (void) pv_update_calc_average_rate_window(&(info_array[use_idx].calc), + state->control.average_rate_window); #ifdef __APPLE__ if (fd_infos[i].proc_fdtype != PROX_FDTYPE_VNODE) { @@ -536,12 +514,9 @@ int pv_watchpid_scanfds(pvstate_t state, */ if ((rc != 0) && (rc != 4)) { debug("%s %d: %s: %d", "fd", fd, "lookup failed - marking slot for re-use", use_idx); + pv_freecontents_watchfd(&(info_array[use_idx])); info_array[use_idx].watch_pid = 0; - if (NULL != info_array[use_idx].state) - pv_state_free(info_array[use_idx].state); - /*@-mustfreeonly@ *//* not a leak - we've just free()d it. */ - info_array[use_idx].state = NULL; - /*@+mustfreeonly@ */ + info_array[use_idx].watch_fd = -1; continue; } @@ -556,42 +531,11 @@ int pv_watchpid_scanfds(pvstate_t state, info_array[use_idx].watch_fd = -1; } - /* NULL check on the state - skip if not usable. */ - if (NULL == info_array[use_idx].state) { - debug("%s %d: %s", "fd", fd, "state is NULL - marking as not displayable"); - info_array[use_idx].watch_fd = -1; - continue; - } - - /* - * Set the displayed size appropriately; if the size is 0, - * or not known, remove %e and %I from the state's default - * format string so that neither estimated time remaining - * nor estimated time of completion are displayed. - */ - info_array[use_idx].state->control.size = info_array[use_idx].size; - if (info_array[use_idx].size < 1) { - char *fmt; - while (NULL != (fmt = strstr(info_array[use_idx].state->control.default_format, "%e"))) { - debug("%s", "zero size - removing estimated time remaining"); - /* strlen-1 here to include trailing \0 */ - memmove(fmt, fmt + 2, strlen(fmt) - 1); /* flawfinder: ignore */ - info_array[use_idx].state->flags.reparse_display = 1; - } - while (NULL != (fmt = strstr(info_array[use_idx].state->control.default_format, "%I"))) { - debug("%s", "zero size - removing estimated completion time"); - /* strlen-1 here to include trailing \0 */ - memmove(fmt, fmt + 2, strlen(fmt) - 1); /* flawfinder: ignore */ - info_array[use_idx].state->flags.reparse_display = 1; - } - /* flawfinder: default_format is always \0-terminated. */ - } - /* Set the info display_name appropriately. */ pv_watchpid_setname(state, &(info_array[use_idx])); - /* Reparse. */ - info_array[use_idx].state->flags.reparse_display = 1; + /* Force the display to be re-parsed. */ + info_array[use_idx].flags.reparse_display = 1; pv_elapsedtime_read(&(info_array[use_idx].start_time)); @@ -600,11 +544,11 @@ int pv_watchpid_scanfds(pvstate_t state, * display), if known, so that ETA and so on are calculated * correctly. */ - info_array[use_idx].state->display.initial_offset = 0; + info_array[use_idx].display.initial_offset = 0; info_array[use_idx].position = 0; position_now = pv_watchfd_position(&(info_array[use_idx])); if (position_now >= 0) { - info_array[use_idx].state->display.initial_offset = position_now; + info_array[use_idx].display.initial_offset = position_now; info_array[use_idx].position = position_now; } }