diff --git a/doc/NEWS.md b/doc/NEWS.md index 679abd8..af6af5e 100644 --- a/doc/NEWS.md +++ b/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 diff --git a/doc/pv.1 b/doc/pv.1 index 4d3065f..efc93fc 100644 --- a/doc/pv.1 +++ b/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. diff --git a/src/main/main.c b/src/main/main.c index 356b0f0..26d7959 100644 --- a/src/main/main.c +++ b/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); } diff --git a/src/main/options.c b/src/main/options.c index e0d6dbc..73f468d 100644 --- a/src/main/options.c +++ b/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 diff --git a/src/pv/file.c b/src/pv/file.c index d8d18d4..b78b66f 100644 --- a/src/pv/file.c +++ b/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. diff --git a/src/pv/loop.c b/src/pv/loop.c index 0f38ea3..7caa445 100644 --- a/src/pv/loop.c +++ b/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);