Adjustments and annotations as per splint and flawfinder reports.

src/include/options.h

@@ -55,16 +55,17 @@ struct opts_s {           /* structure describing run-time options */
 	/*@keep@*/ /*@null@*/ char *name;    /* display name, if any */
 	/*@keep@*/ /*@null@*/ char *format;  /* output format, if any */
 	/*@keep@*/ /*@null@*/ char *pidfile; /* PID file, if any */
-	int argc;                      /* number of non-option arguments */
+	unsigned int argc;             /* number of non-option arguments */
 	/*@keep@*/ /*@null@*/ char **argv;   /* array of non-option arguments */
+	unsigned int argv_length;      /* allocated array size */
 };
 
 /*@-exportlocal@*/
 /* splint thinks opts_free is exported but not used - it is used. */
 
-extern /*@null@*/ /*@only@*/ opts_t opts_parse(int, char **);
+extern /*@null@*/ /*@only@*/ opts_t opts_parse(unsigned int, char **);
 extern void opts_free(/*@only@*/ opts_t);
-
+extern bool opts_add_file(opts_t, char *);
 
 #ifdef __cplusplus
 }

src/include/pv-internal.h

@@ -77,7 +77,7 @@ struct pvstate_s {
 	/***************
 	 * Input files *
 	 ***************/
-	int input_file_count;		 /* number of input files */
+	unsigned int input_file_count;	 /* number of input files */
 	const char **input_files;	 /* input files (0=first) */
 
 	/*******************
@@ -106,7 +106,7 @@ struct pvstate_s {
 	int watch_fd;			 /* fd to watch */
 	unsigned int width;              /* screen width */
 	unsigned int height;             /* screen height */
-	const char *name;		 /* display name */
+	/*@null@*/ const char *name;	 /* display name */
 	char default_format[PV_SIZEOF_DEFAULT_FORMAT];	 /* default format string */
 	const char *format_string;	 /* output format string */
 

src/include/pv.h

@@ -79,7 +79,7 @@ extern size_t pv_strlcat(char *, const char *, size_t);
 /*
  * Create a new state structure, and return it, or 0 (NULL) on error.
  */
-extern pvstate_t pv_state_alloc(const char *);
+extern /*@null@*/ /*@only@*/ pvstate_t pv_state_alloc(const char *);
 
 /*
  * Set the formatting string, given a set of old-style formatting options.
@@ -90,7 +90,7 @@ extern void pv_state_set_format(pvstate_t state, bool progress,
 				bool average_rate, bool bytes,
 				bool bufpercent,
 				unsigned int lastwritten,
-				const char *name);
+				/*@null@*/ const char *name);
 
 /*
  * Set the various options.
@@ -119,9 +119,9 @@ extern void pv_state_name_set(pvstate_t, const char *);
 extern void pv_state_format_string_set(pvstate_t, const char *);
 extern void pv_state_watch_pid_set(pvstate_t, unsigned int);
 extern void pv_state_watch_fd_set(pvstate_t, int);
-extern void pv_state_average_rate_window_set(pvstate_t, int);
+extern void pv_state_average_rate_window_set(pvstate_t, unsigned int);
 
-extern void pv_state_inputfiles(pvstate_t, int, const char **);
+extern void pv_state_inputfiles(pvstate_t, unsigned int, const char **);
 
 /*
  * Work out whether we are in the foreground.
@@ -166,7 +166,7 @@ extern void pv_sig_fini(pvstate_t);
 /*
  * Free a state structure, after which it can no longer be used.
  */
-extern void pv_state_free(pvstate_t);
+extern void pv_state_free(/*@only@*/ pvstate_t);
 
 
 #ifdef ENABLE_DEBUGGING

src/main/main.c

@@ -35,8 +35,8 @@ void pv_remote_fini(void);
 int main(int argc, char **argv)
 {
 	struct termios t, t_save;
-	opts_t opts;
-	pvstate_t state;
+	/*@only@ */ opts_t opts = NULL;
+	/*@only@ */ pvstate_t state = NULL;
 	bool t_saved, t_needs_reset;
 	int retcode = 0;
 
@@ -46,7 +46,7 @@ int main(int argc, char **argv)
 	(void) textdomain(PACKAGE);
 #endif
 
-	opts = opts_parse(argc, argv);
+	opts = opts_parse(argc >= 0 ? (unsigned int) argc : 0, argv);
 	if (NULL == opts) {
 		debug("%s: %d", "exiting with status", 64);
 		return 64;
@@ -72,10 +72,17 @@ int main(int argc, char **argv)
 	 */
 	state = pv_state_alloc(opts->program_name);
 	if (NULL == state) {
+		/*@-mustfreefresh@ */
+		/*
+		 * splint note: the gettext calls made by _() cause memory
+		 * leak warnings, but in this case it's unavoidable, and
+		 * mitigated by the fact we only translate each string once.
+		 */
 		fprintf(stderr, "%s: %s: %s\n", opts->program_name, _("state allocation failed"), strerror(errno));
 		opts_free(opts);
 		debug("%s: %d", "exiting with status", 64);
 		return 64;
+		/*@+mustfreefresh@ */
 	}
 
 	/*
@@ -85,10 +92,12 @@ int main(int argc, char **argv)
 		FILE *pidfptr;
 		pidfptr = fopen(opts->pidfile, "w");
 		if (NULL == pidfptr) {
+			/*@-mustfreefresh@ *//* see above */
 			fprintf(stderr, "%s: %s: %s\n", opts->program_name, opts->pidfile, strerror(errno));
 			pv_state_free(state);
 			opts_free(opts);
 			return 1;
+			/*@+mustfreefresh@ */
 		}
 		fprintf(pidfptr, "%d\n", getpid());
 		if (0 != fclose(pidfptr)) {
@@ -101,13 +110,19 @@ int main(int argc, char **argv)
 	 */
 	if (0 == opts->argc) {
 		debug("%s", "no files given - adding fake argument `-'");
-		opts->argv[opts->argc++] = "-";
+		if (!opts_add_file(opts, "-")) {
+			pv_state_free(state);
+			opts_free(opts);
+			return 64;
+		}
 	}
 
 	/*
 	 * Put our list of files into the PV internal state.
 	 */
-	pv_state_inputfiles(state, opts->argc, (const char **) (opts->argv));
+	if (NULL != opts->argv) {
+		pv_state_inputfiles(state, opts->argc, (const char **) (opts->argv));
+	}
 
 	if (0 == opts->watch_pid) {
 		/*
@@ -200,8 +215,13 @@ int main(int argc, char **argv)
 	pv_state_target_buffer_size_set(state, opts->buffer_size);
 	pv_state_no_splice_set(state, opts->no_splice);
 	pv_state_size_set(state, opts->size);
-	pv_state_name_set(state, opts->name);
-	pv_state_format_string_set(state, opts->format);
+
+	if (NULL != opts->name)
+		pv_state_name_set(state, opts->name);
+
+	if (NULL != opts->format)
+		pv_state_format_string_set(state, opts->format);
+
 	pv_state_watch_pid_set(state, opts->watch_pid);
 	pv_state_watch_fd_set(state, opts->watch_fd);
 	pv_state_average_rate_window_set(state, opts->average_rate_window);
@@ -239,8 +259,10 @@ int main(int argc, char **argv)
 			debug("%s", "saved terminal attributes");
 			t_saved = true;
 		} else {
+			/*@-mustfreefresh@ *//* see above */
 			fprintf(stderr, "%s: %s: %s\n", opts->program_name,
 				_("failed to read terminal attributes"), strerror(errno));
+			/*@+mustfreefresh@ */
 		}
 	}
 	t_save = t;

src/main/options.c

@@ -39,7 +39,7 @@ char *xstrdup(const char *original)
 		return NULL;
 	}
 
-	length = strlen(original);	/* flawfinder: ignore */
+	length = strlen(original);	    /* flawfinder: ignore */
 	/*
 	 * flawfinder rationale: the original string is explicitly required
 	 * to be \0 terminated.
@@ -84,18 +84,54 @@ void opts_free( /*@only@ */ opts_t opts)
 	free(opts);
 }
 
+/*
+ * Add a filename to the list of non-option arguments, returning false on
+ * error.  The filename is not copied - the pointer is stored.
+ */
+bool opts_add_file(opts_t opts, char *filename)
+{
+	/*@-branchstate@ */
+	if ((opts->argc >= opts->argv_length) || (NULL == opts->argv)) {
+		opts->argv_length = opts->argc + 10;
+		/*@-keeptrans@ */
+		opts->argv = realloc(opts->argv, opts->argv_length * sizeof(char *));
+		/*@+keeptrans@ */
+		if (NULL == opts->argv) {
+			fprintf(stderr, "%s: %s\n", opts->program_name, strerror(errno));
+			opts->argv_length = 0;
+			opts->argc = 0;
+			return false;
+		}
+	}
+	/*@+branchstate@ */
+
+	/*
+	 * splint notes: we turned off "branchstate" above because depending
+	 * on whether we have to extend the array, we change argv from
+	 * "keep" to "only", which is also why we turned off "keeptrans";
+	 * there doesn't seem to be a clean way to tell splint that everyone
+	 * else should not touch argv but we're allowed to reallocate it and
+	 * so is opts_parse.
+	 */
+
+	opts->argv[opts->argc++] = filename;
+
+	return true;
+}
+
 
 /*
  * Parse the given command-line arguments into an opts_t object, handling
  * "help" and "version" options internally.
  *
- * Returns an opts_t, or 0 on error.
+ * Returns an opts_t, or NULL on error.
  *
  * Note that the contents of *argv[] (i.e. the command line parameters)
  * aren't copied anywhere, just the pointers are copied, so make sure the
  * command line data isn't overwritten or argv[1] free()d or whatever.
  */
-								 /*@null@ *//*@only@ */ opts_t opts_parse(int argc, char **argv)
+		      /*@null@ *//*@only@ */
+opts_t opts_parse(unsigned int argc, char **argv)
 {
 #ifdef HAVE_GETOPT_LONG
 	/*@-nullassign@ */
@@ -189,6 +225,7 @@ void opts_free( /*@only@ */ opts_t opts)
 		return NULL;
 		/*@+mustfreefresh@ */
 	}
+	opts->argv_length = 1 + argc;
 
 	numopts = 0;
 
@@ -200,9 +237,9 @@ void opts_free( /*@only@ */ opts_t opts)
 
 	do {
 #ifdef HAVE_GETOPT_LONG
-		c = getopt_long(argc, argv, short_options, long_options, &option_index);	/* flawfinder: ignore */
+		c = getopt_long((int) argc, argv, short_options, long_options, &option_index);	/* flawfinder: ignore */
 #else
-		c = getopt(argc, argv, short_options);	/* flawfinder: ignore */
+		c = getopt((int) argc, argv, short_options);	/* flawfinder: ignore */
 #endif
 		/*
 		 * flawfinder rationale: we have to pass argv to getopt, and
@@ -519,7 +556,7 @@ void opts_free( /*@only@ */ opts_t opts)
 			/*@+mustfreefresh@ */
 		}
 
-		if (optind < argc) {
+		if (optind < (int) argc) {
 			/*@-mustfreefresh@ *//* see above */
 			fprintf(stderr, "%s: %s\n", opts->program_name,
 				_("cannot transfer files when watching file descriptors"));
@@ -559,8 +596,11 @@ void opts_free( /*@only@ */ opts_t opts)
 	/*
 	 * Store remaining command-line arguments.
 	 */
-	while (optind < argc) {
-		opts->argv[opts->argc++] = argv[optind++];
+	while (optind < (int) argc) {
+		if (!opts_add_file(opts, argv[optind++])) {
+			opts_free(opts);
+			return NULL;
+		}
 	}
 
 	return opts;

src/pv/state.c

@@ -109,10 +109,8 @@ void pv_state_free(pvstate_t state)
 /*
  * Set the formatting string, given a set of old-style formatting options.
  */
-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, unsigned int lastwritten, const char *name)
+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, unsigned int lastwritten,	/*@null@ */
+			 const char *name)
 {
 #define PV_ADDFORMAT(x,y) if (x) { \
 		if (state->default_format[0] != '\0') \
@@ -263,7 +261,7 @@ void pv_state_watch_fd_set(pvstate_t state, int val)
 	state->watch_fd = val;
 };
 
-void pv_state_average_rate_window_set(pvstate_t state, int val)
+void pv_state_average_rate_window_set(pvstate_t state, unsigned int val)
 {
 	if (val < 1)
 		val = 1;
@@ -281,7 +279,7 @@ void pv_state_average_rate_window_set(pvstate_t state, int val)
 /*
  * Set the array of input files.
  */
-void pv_state_inputfiles(pvstate_t state, int input_file_count, const char **input_files)
+void pv_state_inputfiles(pvstate_t state, unsigned int input_file_count, const char **input_files)
 {
 	state->input_file_count = input_file_count;
 	state->input_files = input_files;