Use dormant code to calculate total line count in line mode when all inputs are regular files, and fix unwise variable naming in size calculation functions, no more "i" and "j".

doc/NEWS.md

@@ -4,6 +4,7 @@
   * feature: now uses `posix_fadvise()` like `cat`(1) does, to improve efficiency ([#39](https://codeberg.org/a-j-wood/pv/issues/39))
   * security: with "`--pidfile`", write to a temporary file and rename it into place, to improve security
   * security: keep self-contained copies of name and format string in PV internal state for memory safety
+  * fix: auto-calculate total line count with "`--line-mode`" when all inputs are regular files
   * cleanup: switched the build system to GNU Automake
   * cleanup: added a test for terminal width detection to "`make check`"
   * cleanup: added a test to "`make check`" to ensure that "`make install`" installs everything expected

doc/pv.1

@@ -273,9 +273,13 @@ to watch all file descriptors of a process, but will work with
 Instead of counting bytes, count lines (newline characters). The progress
 bar will only move when a new line is found, and the value passed to the
 .B \-s
-option will be interpreted as a line count.  Note that file sizes are not
-automatically calculated when this option is used, to avoid having to read
-all files twice.
+option will be interpreted as a line count.
+
+If this option is used without
+.BR \-s ,
+the "total size" (in this case, total line count) is calculated by reading
+through all input files once before transfer starts.  If any inputs are
+pipes or non-regular files, the total size will not be calculated.
 .TP
 .B \-0, \-\-null
 Count lines as null terminated.  This option implies \-\-line\-mode.

src/main/main.c

@@ -187,10 +187,11 @@ int main(int argc, char **argv)
 
 	if (0 == opts->watch_pid) {
 		/*
-		 * If no size was given, and we're not in line mode, try to
-		 * calculate the total size.
+		 * If no size was given, try to calculate the total size.
 		 */
-		if ((0 == opts->size) && (false == opts->linemode)) {
+		if (0 == opts->size) {
+			pv_state_linemode_set(state, opts->linemode);
+			pv_state_null_set(state, opts->null);
 			opts->size = pv_calc_total_size(state);
 			debug("%s: %llu", "no size given - calculated", opts->size);
 		}

src/main/options.c

@@ -131,7 +131,8 @@ bool opts_add_file(opts_t opts, const char *filename)
  * 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@ */
+/*@null@ */
+/*@only@ */
 opts_t opts_parse(unsigned int argc, char **argv)
 {
 #ifdef HAVE_GETOPT_LONG

src/pv/file.c

@@ -21,24 +21,20 @@
 
 
 /*
- * Try to work out the total size of all data by adding up the sizes of all
- * input files. If any of the input files are of indeterminate size (i.e.
- * they are a pipe), the total size is set to zero.
+ * Calculate the total number of bytes to be transferred by adding up the
+ * sizes of all input files.  If any of the input files are of indeterminate
+ * size (such as if they are a pipe), the total size is set to zero.
  *
  * Any files that cannot be stat()ed or that access() says we can't read
  * will cause a warning to be output and will be removed from the list.
  *
- * In line mode, any files that pass the above checks will then be read to
- * determine how many lines they contain, and the total size will be set to
- * the total line count. Only regular files will be read.
- *
  * Returns the total size, or 0 if it is unknown.
  */
-unsigned long long pv_calc_total_size(pvstate_t state)
+static unsigned long long pv_calc_total_bytes(pvstate_t state)
 {
 	unsigned long long total;
 	struct stat sb;
-	int rc, i, j, fd;
+	int rc, file_idx, move_idx, fd;
 
 	total = 0;
 	rc = 0;
@@ -53,26 +49,26 @@ unsigned long long pv_calc_total_size(pvstate_t state)
 		return total;
 	}
 
-	for (i = 0; i < state->input_file_count; i++) {
-		if (0 == strcmp(state->input_files[i], "-")) {
+	for (file_idx = 0; file_idx < state->input_file_count; file_idx++) {
+		if (0 == strcmp(state->input_files[file_idx], "-")) {
 			rc = fstat(STDIN_FILENO, &sb);
 			if (rc != 0) {
 				total = 0;
 				return total;
 			}
 		} else {
-			rc = stat(state->input_files[i], &sb);
+			rc = stat(state->input_files[file_idx], &sb);
 			if (0 == rc)
-				rc = access(state->input_files[i], R_OK);
+				rc = access(state->input_files[file_idx], R_OK);
 		}
 
 		if (rc != 0) {
-			pv_error(state, "%s: %s", state->input_files[i], strerror(errno));
-			for (j = i; j < state->input_file_count - 1; j++) {
-				state->input_files[j] = state->input_files[j + 1];
+			pv_error(state, "%s: %s", state->input_files[file_idx], strerror(errno));
+			for (move_idx = file_idx; move_idx < state->input_file_count - 1; move_idx++) {
+				state->input_files[move_idx] = state->input_files[move_idx + 1];
 			}
 			state->input_file_count--;
-			i--;
+			file_idx--;
 			state->exit_status |= 2;
 			continue;
 		}
@@ -82,16 +78,16 @@ unsigned long long pv_calc_total_size(pvstate_t state)
 			 * Get the size of block devices by opening
 			 * them and seeking to the end.
 			 */
-			if (0 == strcmp(state->input_files[i], "-")) {
+			if (0 == strcmp(state->input_files[file_idx], "-")) {
 				fd = open("/dev/stdin", O_RDONLY);
 			} else {
-				fd = open(state->input_files[i], O_RDONLY);
+				fd = open(state->input_files[file_idx], O_RDONLY);
 			}
 			if (fd >= 0) {
 				total += lseek(fd, 0, SEEK_END);
 				close(fd);
 			} else {
-				pv_error(state, "%s: %s", state->input_files[i], strerror(errno));
+				pv_error(state, "%s: %s", state->input_files[file_idx], strerror(errno));
 				state->exit_status |= 2;
 			}
 		} else if (S_ISREG(sb.st_mode)) {
@@ -131,18 +127,35 @@ unsigned long long pv_calc_total_size(pvstate_t state)
 		}
 	}
 
-	if (!state->linemode)
-		return total;
+	return total;
+}
+
+
+/*
+ * Count the total number of lines to be transferred by reading through all
+ * input files.  If any of the inputs are not regular files (such as if they
+ * are a pipe or a block device), the total size is set to zero.
+ *
+ * Any files that cannot be stat()ed or that access() says we can't read
+ * will cause a warning to be output and will be removed from the list.
+ *
+ * Returns the total size, or 0 if it is unknown.
+ */
+static unsigned long long pv_calc_total_lines(pvstate_t state)
+{
+	unsigned long long total;
+	struct stat sb;
+	int rc, file_idx, move_idx, fd;
 
 	/*
 	 * In line mode, we count input lines to work out the total size.
 	 */
 	total = 0;
 
-	for (i = 0; i < state->input_file_count; i++) {
+	for (file_idx = 0; file_idx < state->input_file_count; file_idx++) {
 		fd = -1;
 
-		if (0 == strcmp(state->input_files[i], "-")) {
+		if (0 == strcmp(state->input_files[file_idx], "-")) {
 			rc = fstat(STDIN_FILENO, &sb);
 			if ((rc != 0) || (!S_ISREG(sb.st_mode))) {
 				total = 0;
@@ -150,21 +163,24 @@ unsigned long long pv_calc_total_size(pvstate_t state)
 			}
 			fd = dup(STDIN_FILENO);
 		} else {
-			rc = stat(state->input_files[i], &sb);
+			rc = stat(state->input_files[file_idx], &sb);
 			if ((rc != 0) || (!S_ISREG(sb.st_mode))) {
 				total = 0;
 				return total;
 			}
-			fd = open(state->input_files[i], O_RDONLY);
+			fd = open(state->input_files[file_idx], O_RDONLY);
 		}
 
 		if (fd < 0) {
-			pv_error(state, "%s: %s", state->input_files[i], strerror(errno));
-			total = 0;
+			pv_error(state, "%s: %s", state->input_files[file_idx], strerror(errno));
+			for (move_idx = file_idx; move_idx < state->input_file_count - 1; move_idx++) {
+				state->input_files[move_idx] = state->input_files[move_idx + 1];
+			}
+			state->input_file_count--;
+			file_idx--;
 			state->exit_status |= 2;
-			return total;
+			continue;
 		}
-
 #if HAVE_POSIX_FADVISE
 		/* Advise the OS that we will only be reading sequentially. */
 		(void) posix_fadvise(fd, 0, 0, POSIX_FADV_SEQUENTIAL);
@@ -172,30 +188,56 @@ unsigned long long pv_calc_total_size(pvstate_t state)
 
 		while (1) {
 			unsigned char scanbuf[1024];
-			int numread, j;
+			int numread, buf_idx;
 
 			numread = read(fd, scanbuf, sizeof(scanbuf));
 			if (numread < 0) {
-				pv_error(state, "%s: %s", state->input_files[i], strerror(errno));
+				pv_error(state, "%s: %s", state->input_files[file_idx], strerror(errno));
 				state->exit_status |= 2;
 				break;
 			} else if (0 == numread) {
 				break;
 			}
-			for (j = 0; j < numread; j++) {
-				if ('\n' == scanbuf[j])
-					total++;
+			for (buf_idx = 0; buf_idx < numread; buf_idx++) {
+				if (state->null) {
+					if ('\0' == scanbuf[buf_idx])
+						total++;
+				} else {
+					if ('\n' == scanbuf[buf_idx])
+						total++;
+				}
 			}
 		}
 
-		lseek(fd, 0, SEEK_SET);
-		close(fd);
+		if (0 != lseek(fd, 0, SEEK_SET)) {
+			pv_error(state, "%s: %s", state->input_files[file_idx], strerror(errno));
+			state->exit_status |= 2;
+		}
+
+		(void) close(fd);
 	}
 
 	return total;
 }
 
 
+/*
+ * Work out the total size of all data by adding up the sizes of all input
+ * files, using either pv_calc_total_bytes() or pv_calc_total_lines()
+ * depending on whether state->linemode is true.
+ *
+ * Returns the total size, or 0 if it is unknown.
+ */
+unsigned long long pv_calc_total_size(pvstate_t state)
+{
+	if (state->linemode) {
+		return pv_calc_total_lines(state);
+	} else {
+		return pv_calc_total_bytes(state);
+	}
+}
+
+
 /*
  * Close the given file descriptor and open the next one, whose number in
  * the list is "filenum", returning the new file descriptor (or negative on
@@ -280,7 +322,6 @@ int pv_next_file(pvstate_t state, int filenum, int oldfd)
 	if (0 == strcmp(state->input_files[filenum], "-")) {
 		state->current_file = "(stdin)";
 	}
-
 #ifdef O_DIRECT
 	/*
 	 * Set or clear O_DIRECT on the file descriptor.

src/pv/loop.c

@@ -114,7 +114,6 @@ int pv_main_loop(pvstate_t state)
 			pv_crs_fini(state);
 		return state->exit_status;
 	}
-
 #if HAVE_POSIX_FADVISE
 	/* Advise the OS that we will only be reading sequentially. */
 	(void) posix_fadvise(fd, 0, 0, POSIX_FADV_SEQUENTIAL);