From 574ea045024706d9f94dcb8d1dec7c70486a1d4b Mon Sep 17 00:00:00 2001 From: Andrew Wood Date: Sun, 27 Jul 2025 21:23:41 +0100 Subject: [PATCH] Instead of adding an aliased display name and having to maintain it and treat it differently, go back to keeping the name only in the "control" sub-structure of the state, but share the global state "control" across all watched fds, and just change it before each pv_display() call so it points to the relevant fd display_name when needed (#165). --- src/include/pv-internal.h | 10 ---------- src/pv/display.c | 4 ++-- src/pv/format/name.c | 4 ++-- src/pv/loop.c | 12 +++++++++++- src/pv/state.c | 16 +--------------- src/pv/watchpid.c | 23 ----------------------- 6 files changed, 16 insertions(+), 53 deletions(-) diff --git a/src/include/pv-internal.h b/src/include/pv-internal.h index 895eec6..4c38807 100644 --- a/src/include/pv-internal.h +++ b/src/include/pv-internal.h @@ -245,14 +245,6 @@ struct pvstate_s { /***************** * Display state * *****************/ - /* - * Note that the "name" pointer is an alias - it should point to - * some other string that's allocated and freed separately, such as - * control.name. It is used here so that the name can be changed - * for different display structures without having to also change - * the control structure, for example when watching multiple file - * descriptors with --watchfd. - */ struct pvdisplay_s { struct pvdisplay_segment_s { /* format string broken into segments */ @@ -281,8 +273,6 @@ struct pvstate_s { off_t initial_offset; /* offset when first opened (when watching fds) */ size_t next_line_len; /* length of currently receiving line so far */ - /*@dependent@*/ /*@null@*/ const char *name; /* copy of pointer to string for %N */ - size_t format_segment_count; /* number of format string segments */ pvtransfercount_t count_type; /* type of count for transfer, rate, etc */ diff --git a/src/pv/display.c b/src/pv/display.c index c3f562d..8c0a176 100644 --- a/src/pv/display.c +++ b/src/pv/display.c @@ -1044,8 +1044,8 @@ bool pv_format(pvprogramstatus_t status, readonly_pvcontrol_t control, readonly_ size_t new_size; new_size = (size_t) ((4 * control->width) + 80); - if (NULL != display->name) - new_size += strlen(display->name); /* flawfinder: ignore */ + if (NULL != control->name) + new_size += strlen(control->name); /* flawfinder: ignore */ /* flawfinder: name is always set by pv_strdup(), which bounds with a \0. */ new_buffer = malloc(new_size + 16); diff --git a/src/pv/format/name.c b/src/pv/format/name.c index e54567e..203d652 100644 --- a/src/pv/format/name.c +++ b/src/pv/format/name.c @@ -35,8 +35,8 @@ pvdisplay_bytecount_t pv_formatter_name(pvformatter_args_t args) (void) pv_snprintf(string_format, sizeof(string_format), "%%%d.500s:", field_width); content[0] = '\0'; - if (NULL != args->display->name) { - (void) pv_snprintf(content, sizeof(content), string_format, args->display->name); + if (NULL != args->control->name) { + (void) pv_snprintf(content, sizeof(content), string_format, args->control->name); } return pv_formatter_segmentcontent(content, args); diff --git a/src/pv/loop.c b/src/pv/loop.c index 9e61129..e4d1916 100644 --- a/src/pv/loop.c +++ b/src/pv/loop.c @@ -789,6 +789,14 @@ int pv_watchpid_loop(pvstate_t state) return PV_ERROREXIT_ACCESS; } + /* + * Make sure there's no name set. + */ + if (NULL != state->control.name) { + free(state->control.name); + state->control.name = NULL; + } + /* * Make sure there's a format string, and then insert %N into it if * it's not present. @@ -990,10 +998,12 @@ int pv_watchpid_loop(pvstate_t state) if (NULL != info_array[idx].state) { info_array[idx].state->transfer.transferred = position_now; info_array[idx].state->transfer.total_written = position_now; + state->control.name = info_array[idx].display_name; pv_display(&(state->status), - &(info_array[idx].state->control), &(info_array[idx].state->flags), + &(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.name = NULL; displayed_lines++; } } diff --git a/src/pv/state.c b/src/pv/state.c index c80c803..1e81a51 100644 --- a/src/pv/state.c +++ b/src/pv/state.c @@ -45,7 +45,7 @@ static void pv_alloc_history(pvtransfercalc_t calc) } calc->history_first = calc->history_last = 0; - calc->history[0].elapsed_sec = 0.0; /* to be safe, memset() not recommended for doubles */ + calc->history[0].elapsed_sec = 0.0; /* to be safe, memset() not recommended for doubles */ } @@ -180,11 +180,6 @@ void pv_state_free(pvstate_t state) free(state->display.display_buffer); state->display.display_buffer = NULL; - if (NULL != state->display.name) { - /* The display name pointer is an alias, so don't free it. */ - state->display.name = NULL; - } - if (NULL != state->extra_display.display_buffer) free(state->extra_display.display_buffer); state->extra_display.display_buffer = NULL; @@ -307,11 +302,6 @@ void pv_state_set_format(pvstate_t state, bool progress, bool timer, bool eta, b if (NULL != name) state->control.name = pv_strdup(name); - /*@-onlytrans@ *//* splint correctly warns about possibly leaking memory. */ - /* Alias this new name pointer, even if it's NULL, to display.name. */ - state->display.name = state->control.name; - /*@+onlytrans@ */ - /* Tell pv_format() that the format has changed. */ state->flags.reparse_display = 1; } @@ -455,10 +445,6 @@ void pv_state_name_set(pvstate_t state, /*@null@ */ const char *val) } if (NULL != val) state->control.name = pv_strdup(val); - /*@-onlytrans@ *//* splint correctly warns about possibly leaking memory. */ - /* display.name is an alias - don't free it. */ - state->display.name = state->control.name; - /*@+onlytrans@ */ } void pv_state_default_bar_style_set(pvstate_t state, /*@null@ */ const char *val) diff --git a/src/pv/watchpid.c b/src/pv/watchpid.c index e977d2c..42c3516 100644 --- a/src/pv/watchpid.c +++ b/src/pv/watchpid.c @@ -333,7 +333,6 @@ static bool extend_info_array(int *array_length_ptr, pvwatchfd_t * info_array_pt int array_length = 0; struct pvwatchfd_s *info_array = NULL; struct pvwatchfd_s *new_info_array; - int idx; array_length = *array_length_ptr; info_array = *info_array_ptr; @@ -356,17 +355,6 @@ static bool extend_info_array(int *array_length_ptr, pvwatchfd_t * info_array_pt debug("%s", "extended info array"); - /* - * We now have to re-point all of the display.name pointers to their - * respective display_name buffers, in case realloc moved the - * array's base address. - */ - for (idx = 0; idx < array_length; idx++) { - if (NULL != new_info_array[idx].state) { - new_info_array[idx].state->display.name = new_info_array[idx].display_name; - } - } - *info_array_ptr = new_info_array; *array_length_ptr = array_length; return true; @@ -530,7 +518,6 @@ int pv_watchpid_scanfds(pvstate_t state, /*@-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->display.name = NULL; info_array[use_idx].state->calc.history = NULL; info_array[use_idx].state->calc.history_len = 0; /*@+mustfreeonly@ */ @@ -676,14 +663,4 @@ void pv_watchpid_setname(pvstate_t state, pvwatchfd_t info) } debug("%s: %d: [%s]", "set name for fd", info->watch_fd, info->display_name); - - /* - * Set the display.name alias so the display_name is used by - * pv_display(). - */ - if (NULL != info->state) { - info->state->display.name = info->display_name; - debug("%s: %d: [%s] / %p", "set display.name for fd", info->watch_fd, info->state->display.name, - info->state->display.name); - } }