diff options
| -rw-r--r-- | ctx.c | 72 | ||||
| -rw-r--r-- | ctx.h | 13 | ||||
| -rw-r--r-- | parse.c | 21 | ||||
| -rwxr-xr-x | tests.sh | 55 | ||||
| -rw-r--r-- | tests/test_fprint_duplicate_stdout.out | 38 | ||||
| -rw-r--r-- | tests/test_stderr_fails_silently.out | 19 |
6 files changed, 177 insertions, 41 deletions
@@ -161,19 +161,10 @@ struct bfs_ctx_file { const char *path; }; -CFILE *bfs_ctx_open(struct bfs_ctx *ctx, const char *path, bool use_color) { - int error = 0; - - CFILE *cfile = cfopen(path, use_color ? ctx->colors : NULL); - if (!cfile) { - error = errno; - goto out; - } - +struct CFILE *bfs_ctx_dedup(struct bfs_ctx *ctx, CFILE *cfile, const char *path) { struct bfs_stat sb; if (bfs_stat(fileno(cfile->file), NULL, 0, &sb) != 0) { - error = errno; - goto out_close; + return NULL; } bfs_file_id id; @@ -181,37 +172,60 @@ CFILE *bfs_ctx_open(struct bfs_ctx *ctx, const char *path, bool use_color) { struct trie_leaf *leaf = trie_insert_mem(&ctx->files, id, sizeof(id)); if (!leaf) { - error = errno; - goto out_close; + return NULL; } - if (leaf->value) { - struct bfs_ctx_file *ctx_file = leaf->value; - cfclose(cfile); - cfile = ctx_file->cfile; - goto out; + struct bfs_ctx_file *ctx_file = leaf->value; + if (ctx_file) { + ctx_file->path = path; + return ctx_file->cfile; } - struct bfs_ctx_file *ctx_file = malloc(sizeof(*ctx_file)); + leaf->value = ctx_file = malloc(sizeof(*ctx_file)); if (!ctx_file) { - error = errno; trie_remove(&ctx->files, leaf); - goto out_close; + return NULL; } ctx_file->cfile = cfile; ctx_file->path = path; - leaf->value = ctx_file; ++ctx->nfiles; + return cfile; +} + +/** Close a file tracked by the bfs context. */ +static int bfs_ctx_close(struct bfs_ctx *ctx, struct bfs_ctx_file *ctx_file) { + CFILE *cfile = ctx_file->cfile; + + if (cfile == ctx->cout) { + // Will be checked later + return 0; + } else if (cfile == ctx->cerr && !ctx_file->path) { + // Writes to stderr are allowed to fail silently, unless the same file was used by + // -fprint, -fls, etc. + return 0; + } - goto out; + int ret = 0, error = 0; + if (ferror(cfile->file)) { + ret = -1; + error = EIO; + } + + if (cfile == ctx->cerr) { + if (fflush(cfile->file) != 0) { + ret = -1; + error = errno; + } + } else { + if (cfclose(cfile) != 0) { + ret = -1; + error = errno; + } + } -out_close: - cfclose(cfile); - cfile = NULL; -out: errno = error; - return cfile; + return ret; } int bfs_ctx_free(struct bfs_ctx *ctx) { @@ -233,7 +247,7 @@ int bfs_ctx_free(struct bfs_ctx *ctx) { while ((leaf = trie_first_leaf(&ctx->files))) { struct bfs_ctx_file *ctx_file = leaf->value; - if (cfclose(ctx_file->cfile) != 0) { + if (bfs_ctx_close(ctx, ctx_file) != 0) { if (cerr) { bfs_error(ctx, "'%s': %m.\n", ctx_file->path); } @@ -164,16 +164,19 @@ const struct bfs_groups *bfs_ctx_groups(const struct bfs_ctx *ctx); const struct bfs_mtab *bfs_ctx_mtab(const struct bfs_ctx *ctx); /** - * Open a file for the bfs context. + * Deduplicate an opened file. * * @param ctx * The bfs context. - * @param use_color - * Whether to use colors if the file is a TTY. + * @param cfile + * The opened file. + * @param path + * The path to the opened file (or NULL for standard streams). * @return - * The opened file, or NULL on failure. + * If the same file was opened previously, that file is returned. If cfile is a new file, + * cfile itself is returned. If an error occurs, NULL is returned. */ -struct CFILE *bfs_ctx_open(struct bfs_ctx *ctx, const char *path, bool use_color); +struct CFILE *bfs_ctx_dedup(struct bfs_ctx *ctx, struct CFILE *cfile, const char *path); /** * Dump the parsed command line. @@ -334,12 +334,24 @@ static void init_print_expr(struct parser_state *state, struct expr *expr) { static int expr_open(struct parser_state *state, struct expr *expr, const char *path) { struct bfs_ctx *ctx = state->ctx; - expr->cfile = bfs_ctx_open(ctx, path, state->use_color); - if (!expr->cfile) { + CFILE *cfile = cfopen(path, state->use_color ? ctx->colors : NULL); + if (!cfile) { parse_error(state, "${blu}%s${rs} ${bld}%s${rs}: %m.\n", expr->argv[0], path); return -1; } + CFILE *dedup = bfs_ctx_dedup(ctx, cfile, path); + if (!dedup) { + parse_error(state, "${blu}%s${rs} ${bld}%s${rs}: %m.\n", expr->argv[0], path); + cfclose(cfile); + return -1; + } + + expr->cfile = dedup; + + if (dedup != cfile) { + cfclose(cfile); + } return 0; } @@ -3689,6 +3701,11 @@ struct bfs_ctx *bfs_parse_cmdline(int argc, char *argv[]) { goto fail; } + if (!bfs_ctx_dedup(ctx, ctx->cout, NULL) || !bfs_ctx_dedup(ctx, ctx->cerr, NULL)) { + bfs_perror(ctx, "bfs_ctx_dedup()"); + goto fail; + } + bool stdin_tty = isatty(STDIN_FILENO); bool stdout_tty = isatty(STDOUT_FILENO); bool stderr_tty = isatty(STDERR_FILENO); @@ -723,6 +723,10 @@ bfs_tests=( test_execdir_plus + test_fprint_duplicate_stdout + test_fprint_error_stdout + test_fprint_error_stderr + test_help test_hidden @@ -768,6 +772,10 @@ bfs_tests=( # PATH_MAX handling test_deep_strict + + # Error handling + test_stderr_fails_silently + test_stderr_fails_loudly ) sudo_tests=( @@ -1683,9 +1691,9 @@ function test_fprint() { sort -o scratch/test_fprint.out scratch/test_fprint.out if [ "$UPDATE" ]; then - cp scratch/test_fprint.out "$TESTS/test_fprint.out" + cp {scratch,"$TESTS"}/test_fprint.out else - diff -u scratch/test_fprint.out "$TESTS/test_fprint.out" + diff -u {"$TESTS",scratch}/test_fprint.out fi } @@ -1698,9 +1706,22 @@ function test_fprint_duplicate() { sort -o scratch/test_fprint_duplicate.out scratch/test_fprint_duplicate.out if [ "$UPDATE" ]; then - cp scratch/test_fprint_duplicate.out "$TESTS/test_fprint_duplicate.out" + cp {scratch,"$TESTS"}/test_fprint_duplicate.out else - diff -u scratch/test_fprint_duplicate.out "$TESTS/test_fprint_duplicate.out" + diff -u {"$TESTS",scratch}/test_fprint_duplicate.out + fi +} + +function test_fprint_duplicate_stdout() { + touchp scratch/test_fprint_duplicate_stdout.out + + invoke_bfs basic -fprint scratch/test_fprint_duplicate_stdout.out -print >scratch/test_fprint_duplicate_stdout.out + sort -o scratch/test_fprint_duplicate_stdout.out{,} + + if [ "$UPDATE" ]; then + cp {scratch,"$TESTS"}/test_fprint_duplicate_stdout.out + else + diff -u {"$TESTS",scratch}/test_fprint_duplicate_stdout.out fi } @@ -2227,7 +2248,7 @@ function test_fprintf() { if [ "$UPDATE" ]; then cp scratch/test_fprintf.out "$TESTS/test_fprintf.out" else - diff -u scratch/test_fprintf.out "$TESTS/test_fprintf.out" + diff -u "$TESTS/test_fprintf.out" scratch/test_fprintf.out fi } @@ -2606,6 +2627,18 @@ function test_fprint_error() { fi } +function test_fprint_error_stdout() { + if [ -e /dev/full ]; then + ! quiet invoke_bfs basic -maxdepth 0 -fprint /dev/full >/dev/full + fi +} + +function test_fprint_error_stderr() { + if [ -e /dev/full ]; then + ! invoke_bfs basic -maxdepth 0 -fprint /dev/full 2>/dev/full + fi +} + function test_print0() { invoke_bfs basic/a basic/b -print0 >scratch/test_print0.out @@ -3038,6 +3071,18 @@ function test_files0_from_ok() { ! printf "basic\0" | quiet invoke_bfs -files0-from - -ok echo {} \; } +function test_stderr_fails_silently() { + if [ -e /dev/full ]; then + bfs_diff -D all basic 2>/dev/full + fi +} + +function test_stderr_fails_loudly() { + if [ -e /dev/full ]; then + ! invoke_bfs -D all basic -false -fprint /dev/full 2>/dev/full + fi +} + BOL= EOL='\n' diff --git a/tests/test_fprint_duplicate_stdout.out b/tests/test_fprint_duplicate_stdout.out new file mode 100644 index 0000000..6c21751 --- /dev/null +++ b/tests/test_fprint_duplicate_stdout.out @@ -0,0 +1,38 @@ +basic +basic +basic/a +basic/a +basic/b +basic/b +basic/c +basic/c +basic/c/d +basic/c/d +basic/e +basic/e +basic/e/f +basic/e/f +basic/g +basic/g +basic/g/h +basic/g/h +basic/i +basic/i +basic/j +basic/j +basic/j/foo +basic/j/foo +basic/k +basic/k +basic/k/foo +basic/k/foo +basic/k/foo/bar +basic/k/foo/bar +basic/l +basic/l +basic/l/foo +basic/l/foo +basic/l/foo/bar +basic/l/foo/bar +basic/l/foo/bar/baz +basic/l/foo/bar/baz diff --git a/tests/test_stderr_fails_silently.out b/tests/test_stderr_fails_silently.out new file mode 100644 index 0000000..bb3cd8d --- /dev/null +++ b/tests/test_stderr_fails_silently.out @@ -0,0 +1,19 @@ +basic +basic/a +basic/b +basic/c +basic/e +basic/g +basic/i +basic/j +basic/k +basic/l +basic/c/d +basic/e/f +basic/g/h +basic/j/foo +basic/k/foo +basic/l/foo +basic/k/foo/bar +basic/l/foo/bar +basic/l/foo/bar/baz |