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

@@ -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 */

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.

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. */

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.
  */

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;
 }