From d7327aaf954e4e431e2e8d6d443054964f331a19 Mon Sep 17 00:00:00 2001 From: Andrew Wood Date: Fri, 3 Apr 2026 20:00:42 +0100 Subject: [PATCH] Replace pv_state_set_format() with pv_set_format_options(), which takes a structure rather than a long list of arguments, to make it easier to pass around - and rely on pv_state_name_set() being called first rather than doing it within this function, so it is only doing one job (#67). --- src/include/pv-internal.h | 12 +------ src/include/pv.h | 22 ++++++++----- src/main/main.c | 36 ++++++++++++-------- src/pv/remote.c | 27 ++++++++++----- src/pv/state.c | 69 ++++++++++++++++++--------------------- 5 files changed, 87 insertions(+), 79 deletions(-) diff --git a/src/include/pv-internal.h b/src/include/pv-internal.h index d7f6491..a801505 100644 --- a/src/include/pv-internal.h +++ b/src/include/pv-internal.h @@ -209,17 +209,7 @@ struct pvstate_s { pvdisplay_width_t width; /* screen width */ unsigned int height; /* screen height */ unsigned int extra_displays; /* bitmask of extra display destinations */ - struct { /* old-style format options (used by -R) */ - size_t lastwritten; /* --last-written (amount) */ - bool progress; /* --progress */ - bool timer; /* --timer */ - bool eta; /* --eta */ - bool fineta; /* --fineta */ - bool rate; /* --rate */ - bool average_rate; /* --average-rate */ - bool bytes; /* --bytes */ - bool bufpercent; /* --buffer-percent */ - } format_option; + pvformatoptions_s format_option; /* old-style format options (used by -R) */ bool force; /* display even if not on terminal */ bool cursor; /* use cursor positioning */ bool numeric; /* numeric output only */ diff --git a/src/include/pv.h b/src/include/pv.h index c467ac0..8537b3b 100644 --- a/src/include/pv.h +++ b/src/include/pv.h @@ -193,15 +193,21 @@ extern /*@null@*/ /*@only@*/ pvstate_t pv_state_alloc(void); extern void pv_state_reset(pvstate_t state); /* - * Set the formatting string, given a set of old-style formatting options. + * Set the format options and use them to build a default formatting string. + * The default string is used if no format string is explicitly set. */ -extern void pv_state_set_format(pvstate_t state, bool progress, - bool timer, bool eta, - bool fineta, bool rate, - bool average_rate, bool bytes, - bool bufpercent, - size_t lastwritten, - /*@null@*/ const char *name); +typedef struct { + size_t lastwritten; /* --last-written (amount) */ + bool progress; /* --progress */ + bool timer; /* --timer */ + bool eta; /* --eta */ + bool fineta; /* --fineta */ + bool rate; /* --rate */ + bool average_rate; /* --average-rate */ + bool bytes; /* --bytes */ + bool bufpercent; /* --buffer-percent */ +} pvformatoptions_s; +extern void pv_state_set_format_options(pvstate_t, pvformatoptions_s); /* * Set the various options. diff --git a/src/main/main.c b/src/main/main.c index 604189f..98e9c28 100644 --- a/src/main/main.c +++ b/src/main/main.c @@ -163,7 +163,7 @@ static int pv__set_output(pvstate_t state, opts_t opts, /*@null@ */ const char * * with the input file list forced to be just the store-and-forward file. * Returns nonzero on error. */ -static int pv__store_and_forward(pvstate_t state, opts_t opts, bool can_have_eta) +static int pv__store_and_forward(pvstate_t state, opts_t opts, pvformatoptions_s format_options) { char tmp_filename[4096]; /* flawfinder: ignore */ bool use_temporary_file; @@ -224,11 +224,10 @@ static int pv__store_and_forward(pvstate_t state, opts_t opts, bool can_have_eta if (0 != retcode) goto end_store_and_forward; - /* Reset the formatting to set the displayed name to "(input)". */ + /* Set the displayed name to "(input)" and trigger a format reparse. */ /*@-mustfreefresh@ */ - pv_state_set_format(state, opts->progress, opts->timer, can_have_eta ? opts->eta : false, - can_have_eta ? opts->fineta : false, opts->rate, opts->average_rate, - opts->bytes, opts->bufpercent, opts->lastwritten, _("(input)")); + pv_state_name_set(state, _("(input)")); + pv_state_set_format_options(state, format_options); /*@+mustfreefresh@ *//* see below about gettext _() calls. */ /* Run the main loop as normal. */ @@ -250,10 +249,12 @@ static int pv__store_and_forward(pvstate_t state, opts_t opts, bool can_have_eta /* Recalculate the input size. */ pv_state_size_set(state, pv_calc_total_size(state)); + /* Set the displayed name to whatever was requested. */ + pv_state_name_set(state, opts->name); /* Reset the format, since we might have been asked to show ETA. */ - pv_state_set_format(state, opts->progress, opts->timer, opts->eta, - opts->fineta, opts->rate, opts->average_rate, - opts->bytes, opts->bufpercent, opts->lastwritten, opts->name); + format_options.eta = opts->eta; + format_options.fineta = opts->fineta; + pv_state_set_format_options(state, format_options); /* Reset calculated values in the state. */ pv_state_reset(state); @@ -552,7 +553,7 @@ x = 1; \ if (NULL != opts->format1) { pv_state_format_string_set(state, opts->format1); } - /* TODO: call pv_state_set_format(). */ + /* TODO: call pv_state_set_format_options(). */ /*@fallthrough@ */ /* falling through as "out" is in another process (above). */ #ifndef SPLINT @@ -624,6 +625,7 @@ int main(int argc, char **argv) int retcode = 0; bool can_have_eta = true; bool terminal_supports_utf8 = false; + pvformatoptions_s format_options; #if ! HAVE_SETPROCTITLE initproctitle(argc, argv); @@ -880,9 +882,17 @@ int main(int argc, char **argv) pv_state_extra_display_set(state, opts->extra_display); pv_state_average_rate_window_set(state, opts->average_rate_window); - pv_state_set_format(state, opts->progress, opts->timer, can_have_eta ? opts->eta : false, - can_have_eta ? opts->fineta : false, opts->rate, opts->average_rate, - opts->bytes, opts->bufpercent, opts->lastwritten, opts->name); + format_options.progress = opts->progress; + format_options.timer = opts->timer; + format_options.eta = can_have_eta ? opts->eta : false; + format_options.fineta = can_have_eta ? opts->fineta : false; + format_options.rate = opts->rate; + format_options.average_rate = opts->average_rate; + format_options.bytes = opts->bytes; + format_options.bufpercent = opts->bufpercent; + format_options.lastwritten = opts->lastwritten; + + pv_state_set_format_options(state, format_options); debug("%s: %s", "terminal_supports_utf8", terminal_supports_utf8 ? "true" : "false"); pv_state_set_terminal_supports_utf8(state, terminal_supports_utf8); @@ -899,7 +909,7 @@ int main(int argc, char **argv) case PV_ACTION_STORE_AND_FORWARD: /* Store-and-forward transfer mode. */ pv_state_cancel_output_if_empty_format_string(state); - retcode = pv__store_and_forward(state, opts, can_have_eta); + retcode = pv__store_and_forward(state, opts, format_options); break; case PV_ACTION_WATCHFD: /* "Watch file descriptor(s) of another process" mode. */ diff --git a/src/pv/remote.c b/src/pv/remote.c index aeeec91..4867a9f 100644 --- a/src/pv/remote.c +++ b/src/pv/remote.c @@ -247,8 +247,8 @@ int pv_remote_set(pvstate_t state, pid_t remote) * If a message was received, update the current process's options with the * ones in the message. * - * Note that this relies on pv_state_set_format() causing the output format - * to be reparsed. + * Note that this relies on pv_state_set_format_options() causing the output + * format to be reparsed. */ static bool pv__rxsignal_usr2(pvstate_t state) { @@ -256,6 +256,7 @@ static bool pv__rxsignal_usr2(pvstate_t state) char control_filename[4096]; /* flawfinder: ignore */ FILE *control_fptr; struct remote_msg msgbuf; + pvformatoptions_s format_options; /* flawfinder rationale: as above. */ @@ -306,11 +307,19 @@ static bool pv__rxsignal_usr2(pvstate_t state) msgbuf.format[sizeof(msgbuf.format) - 1] = '\0'; msgbuf.extra_display[sizeof(msgbuf.extra_display) - 1] = '\0'; - pv_state_set_format(state, msgbuf.progress, msgbuf.timer, - msgbuf.eta, msgbuf.fineta, msgbuf.rate, - msgbuf.average_rate, - msgbuf.bytes, msgbuf.bufpercent, - msgbuf.lastwritten, '\0' == msgbuf.name[0] ? NULL : msgbuf.name); + pv_state_name_set(state, '\0' == msgbuf.name[0] ? NULL : msgbuf.name); + + format_options.progress = msgbuf.progress; + format_options.timer = msgbuf.timer; + format_options.eta = msgbuf.eta; + format_options.fineta = msgbuf.fineta; + format_options.rate = msgbuf.rate; + format_options.average_rate = msgbuf.average_rate; + format_options.bytes = msgbuf.bytes; + format_options.bufpercent = msgbuf.bufpercent; + format_options.lastwritten = msgbuf.lastwritten; + + pv_state_set_format_options(state, format_options); if (msgbuf.rate_limit > 0) pv_state_rate_limit_set(state, msgbuf.rate_limit); @@ -475,8 +484,8 @@ static bool pv__rxsignal_usr1(pvstate_t state, pid_t match_sender) * send our transfer state to the sending process, depending on the content * of the message. * - * NB --remote relies on pv_state_set_format() causing the output format to - * be reparsed. + * NB --remote relies on pv_state_set_format_options() causing the output + * format to be reparsed. * * Returns true if a --remote message was received, false otherwise. */ diff --git a/src/pv/state.c b/src/pv/state.c index 298cdd2..9229133 100644 --- a/src/pv/state.c +++ b/src/pv/state.c @@ -411,12 +411,15 @@ void pv_state_free(pvstate_t state) /* - * Set the formatting string, given a set of old-style formatting options. + * Set the format options and use them to build a default formatting string. + * The default string is used if no format string is explicitly set. + * + * Call this *after* setting a name, so it can determine whether there + * should be a name in the default format. * * TODO: allow opts->ratio to be passed here as well. */ -void pv_state_set_format(pvstate_t state, bool progress, bool timer, bool eta, bool fineta, bool rate, bool average_rate, bool bytes, bool bufpercent, size_t lastwritten, /*@null@ */ - const char *name) +void pv_state_set_format_options(pvstate_t state, pvformatoptions_s format) { #define PV_ADDFORMAT(x,y) if (x) { \ if (state->control.default_format[0] != '\0') \ @@ -424,15 +427,15 @@ void pv_state_set_format(pvstate_t state, bool progress, bool timer, bool eta, b (void) pv_strlcat(state->control.default_format, y, sizeof(state->control.default_format)); \ } - state->control.format_option.progress = progress; - state->control.format_option.timer = timer; - state->control.format_option.eta = eta; - state->control.format_option.fineta = fineta; - state->control.format_option.rate = rate; - state->control.format_option.average_rate = average_rate; - state->control.format_option.bytes = bytes; - state->control.format_option.bufpercent = bufpercent; - state->control.format_option.lastwritten = lastwritten; + state->control.format_option.progress = format.progress; + state->control.format_option.timer = format.timer; + state->control.format_option.eta = format.eta; + state->control.format_option.fineta = format.fineta; + state->control.format_option.rate = format.rate; + state->control.format_option.average_rate = format.average_rate; + state->control.format_option.bytes = format.bytes; + state->control.format_option.bufpercent = format.bufpercent; + state->control.format_option.lastwritten = format.lastwritten; state->control.default_format[0] = '\0'; @@ -443,21 +446,21 @@ void pv_state_set_format(pvstate_t state, bool progress, bool timer, bool eta, b * Add the format strings for the enabled options in a * standard order. */ - PV_ADDFORMAT(name, "%N"); - PV_ADDFORMAT(bytes, "%b"); - PV_ADDFORMAT(bufpercent, "%T"); - PV_ADDFORMAT(timer, "%t"); - PV_ADDFORMAT(rate, "%r"); - PV_ADDFORMAT(average_rate, "%a"); - PV_ADDFORMAT(progress, "%p"); - PV_ADDFORMAT(eta, "%e"); - PV_ADDFORMAT(fineta, "%I"); + PV_ADDFORMAT(NULL != state->control.name, "%N"); + PV_ADDFORMAT(format.bytes, "%b"); + PV_ADDFORMAT(format.bufpercent, "%T"); + PV_ADDFORMAT(format.timer, "%t"); + PV_ADDFORMAT(format.rate, "%r"); + PV_ADDFORMAT(format.average_rate, "%a"); + PV_ADDFORMAT(format.progress, "%p"); + PV_ADDFORMAT(format.eta, "%e"); + PV_ADDFORMAT(format.fineta, "%I"); - if (lastwritten > 0) { + if (format.lastwritten > 0) { char buf[16]; /* flawfinder: ignore */ memset(buf, 0, sizeof(buf)); - (void) pv_snprintf(buf, sizeof(buf), "%%%uA", (unsigned int) lastwritten); - PV_ADDFORMAT(lastwritten > 0, buf); + (void) pv_snprintf(buf, sizeof(buf), "%%%uA", (unsigned int) format.lastwritten); + PV_ADDFORMAT(format.lastwritten > 0, buf); /* * flawfinder rationale: large enough for string, * zeroed before use, only written to by @@ -468,25 +471,15 @@ void pv_state_set_format(pvstate_t state, bool progress, bool timer, bool eta, b } else { /* Numeric mode has different behaviour. */ - PV_ADDFORMAT(timer, "%t"); - PV_ADDFORMAT(bytes, "%b"); - PV_ADDFORMAT(rate, "%r"); - PV_ADDFORMAT(!(bytes || rate), "%{progress-amount-only}"); + PV_ADDFORMAT(format.timer, "%t"); + PV_ADDFORMAT(format.bytes, "%b"); + PV_ADDFORMAT(format.rate, "%r"); + PV_ADDFORMAT(!(format.bytes || format.rate), "%{progress-amount-only}"); } debug("%s: [%s]", "default format set", state->control.default_format); - /* Free any previously set name. */ - if (NULL != state->control.name) { - free(state->control.name); - state->control.name = NULL; - } - - /* Set a new name if one was given. */ - if (NULL != name) - state->control.name = pv_strdup(name); - /* Tell pv_format() that the format has changed. */ state->flags.reparse_display = 1; }