From 65eac50c21bc7e23e83cbbd7bd3f702ecdcabf99 Mon Sep 17 00:00:00 2001 From: Andrew Wood Date: Mon, 28 Aug 2023 17:15:16 +0100 Subject: [PATCH] Alter file processing to silently skip unreadable input files at the size calculation stage, and report - and skip rather than exiting - unreadable files at the point they are needed during the transfer, more like "cat" does; also, more variable name corrections for bad names (single letters). --- doc/NEWS.md | 1 + doc/pv.1 | 13 +++++++----- src/pv/file.c | 58 +++++++++++++++++++++++++-------------------------- src/pv/loop.c | 36 +++++++++++++++++++++----------- 4 files changed, 61 insertions(+), 47 deletions(-) diff --git a/doc/NEWS.md b/doc/NEWS.md index af6af5e..b7d63fc 100644 --- a/doc/NEWS.md +++ b/doc/NEWS.md @@ -5,6 +5,7 @@ * 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 + * fix: only report errors about missing files when starting to transfer from them, not while calculating size, and behave more like `cat`(1) by skipping them and moving on * 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 efc93fc..6c9cfe6 100644 --- a/doc/pv.1 +++ b/doc/pv.1 @@ -114,8 +114,8 @@ Otherwise, only those display types that are explicitly switched on will be shown. .TP .B \-p, \-\-progress -Turn the progress bar on. If standard input is not a file and no -size was given (with the +Turn the progress bar on. If any inputs are not files, or are +unreadable, and no size was explicitly given (with the .B \-s modifier), the progress bar cannot indicate how close to completion the transfer is, so it will just move left and right to indicate that data is @@ -143,7 +143,8 @@ transfer. .TP .B \-a, \-\-average\-rate Turn the average rate counter on. This will display the current average -rate of data transfer (default: last 30s, see --average-rate-window). +rate of data transfer (default: last 30s, see +.BR \-m ). .TP .B \-b, \-\-bytes Turn the total byte counter on. This will display the total amount of @@ -279,10 +280,12 @@ 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. +pipes or non-regular files, or are unreadable, the total size will not be +calculated. .TP .B \-0, \-\-null -Count lines as null terminated. This option implies \-\-line\-mode. +Count lines as terminated with a zero byte instead of with a newline. +This option implies \-\-line\-mode. .TP .B \-i SEC, \-\-interval SEC Wait diff --git a/src/pv/file.c b/src/pv/file.c index e4f96a9..deeb117 100644 --- a/src/pv/file.c +++ b/src/pv/file.c @@ -26,7 +26,7 @@ * 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. + * will be skipped, and the total size will be set to zero. * * Returns the total size, or 0 if it is unknown. */ @@ -34,10 +34,9 @@ static unsigned long long pv_calc_total_bytes(pvstate_t state) { unsigned long long total; struct stat sb; - int rc, file_idx, move_idx, fd; + int file_idx; total = 0; - rc = 0; memset(&sb, 0, sizeof(sb)); /* @@ -50,6 +49,8 @@ static unsigned long long pv_calc_total_bytes(pvstate_t state) } for (file_idx = 0; file_idx < state->input_file_count; file_idx++) { + int rc; + if (0 == strcmp(state->input_files[file_idx], "-")) { rc = fstat(STDIN_FILENO, &sb); if (rc != 0) { @@ -63,17 +64,14 @@ static unsigned long long pv_calc_total_bytes(pvstate_t state) } if (rc != 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; - continue; + debug("%s: %s", state->input_files[file_idx], strerror(errno)); + total = 0; + return total; } if (S_ISBLK(sb.st_mode)) { + int fd; + /* * Get the size of block devices by opening * them and seeking to the end. @@ -87,8 +85,8 @@ static unsigned long long pv_calc_total_bytes(pvstate_t state) total += lseek(fd, 0, SEEK_END); close(fd); } else { - pv_error(state, "%s: %s", state->input_files[file_idx], strerror(errno)); - state->exit_status |= 2; + total = 0; + return total; } } else if (S_ISREG(sb.st_mode)) { total += sb.st_size; @@ -106,7 +104,10 @@ static unsigned long long pv_calc_total_bytes(pvstate_t state) * and that we can seek back to the start after getting the size. */ if (total <= 0) { + int rc; + rc = fstat(STDOUT_FILENO, &sb); + if ((0 == rc) && S_ISBLK(sb.st_mode) && (0 == (fcntl(STDOUT_FILENO, F_GETFL) & O_APPEND))) { total = lseek(STDOUT_FILENO, 0, SEEK_END); @@ -122,7 +123,7 @@ static unsigned long long pv_calc_total_bytes(pvstate_t state) * device. */ if (total > 0) { - state->stop_at_size = 1; + state->stop_at_size = true; } } } @@ -137,7 +138,7 @@ static unsigned long long pv_calc_total_bytes(pvstate_t state) * 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. + * will be skipped, and the total size will be set to zero. * * Returns the total size, or 0 if it is unknown. */ @@ -145,15 +146,13 @@ 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; + int file_idx; - /* - * In line mode, we count input lines to work out the total size. - */ total = 0; for (file_idx = 0; file_idx < state->input_file_count; file_idx++) { - fd = -1; + int fd = -1; + int rc = 0; if (0 == strcmp(state->input_files[file_idx], "-")) { rc = fstat(STDIN_FILENO, &sb); @@ -172,14 +171,9 @@ static unsigned long long pv_calc_total_lines(pvstate_t state) } if (fd < 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; - continue; + debug("%s: %s", state->input_files[file_idx], strerror(errno)); + total = 0; + return total; } #if HAVE_POSIX_FADVISE /* Advise the OS that we will only be reading sequentially. */ @@ -252,8 +246,8 @@ int pv_next_file(pvstate_t state, int filenum, int oldfd) struct stat osb; int fd, input_file_is_stdout; - if (oldfd > 0) { - if (close(oldfd)) { + if (oldfd >= 0) { + if (0 != close(oldfd)) { pv_error(state, "%s: %s", _("failed to close file"), strerror(errno)); state->exit_status |= 8; return -1; @@ -261,11 +255,13 @@ int pv_next_file(pvstate_t state, int filenum, int oldfd) } if (filenum >= state->input_file_count) { + debug("%s: %d >= %d", "filenum too large", filenum, state->input_file_count); state->exit_status |= 8; return -1; } if (filenum < 0) { + debug("%s: %d < 0", "filenum too small", filenum); state->exit_status |= 8; return -1; } @@ -333,6 +329,8 @@ int pv_next_file(pvstate_t state, int filenum, int oldfd) */ #endif /* O_DIRECT */ + debug("%s: %d: %s: fd=%d", "next file opened", filenum, state->current_file, fd); + return fd; } diff --git a/src/pv/loop.c b/src/pv/loop.c index 7caa445..4e16e41 100644 --- a/src/pv/loop.c +++ b/src/pv/loop.c @@ -60,7 +60,7 @@ int pv_main_loop(pvstate_t state) struct timeval init_time, next_remotecheck; long double elapsed; struct stat sb; - int fd, n; + int fd, file_idx; /* * "written" is ALWAYS bytes written by the last transfer. @@ -106,9 +106,21 @@ int pv_main_loop(pvstate_t state) target = 0; final_update = 0; - n = 0; + file_idx = 0; - fd = pv_next_file(state, n, -1); + /* + * Open the first readable input file. + */ + fd = -1; + while (fd < 0 && file_idx < state->input_file_count) { + fd = pv_next_file(state, file_idx, -1); + if (fd < 0) + file_idx++; + } + + /* + * Exit early if there was no readable input file. + */ if (fd < 0) { if (state->cursor) pv_crs_fini(state); @@ -215,16 +227,16 @@ int pv_main_loop(pvstate_t state) target -= written; } - if (eof_in && eof_out && n < (state->input_file_count - 1)) { - n++; - fd = pv_next_file(state, n, fd); - if (fd < 0) { - if (state->cursor) - pv_crs_fini(state); - return state->exit_status; + /* + * EOF, and files remain - advance to the next file. + */ + while (eof_in && eof_out && file_idx < (state->input_file_count - 1)) { + file_idx++; + fd = pv_next_file(state, file_idx, fd); + if (fd >= 0) { + eof_in = 0; + eof_out = 0; } - eof_in = 0; - eof_out = 0; } gettimeofday(&cur_time, NULL);