From 78944c815a5d8d1c93771ca1c343b9406bc262c4 Mon Sep 17 00:00:00 2001 From: Tavian Barnes Date: Sun, 26 Sep 2021 15:23:26 -0400 Subject: Don't truncate files until we know they're not duplicates --- color.c | 39 ++---------------------------------- color.h | 28 +++++++++++--------------- ctx.c | 2 +- parse.c | 45 ++++++++++++++++++++++++++++++------------ tests.sh | 29 +++++++++++++++++++++++++++ tests/test_fprint_append.out | 38 +++++++++++++++++++++++++++++++++++ tests/test_fprint_truncate.out | 1 + util.c | 1 + 8 files changed, 115 insertions(+), 68 deletions(-) create mode 100644 tests/test_fprint_append.out create mode 100644 tests/test_fprint_truncate.out diff --git a/color.c b/color.c index 09070f2..7064044 100644 --- a/color.c +++ b/color.c @@ -501,7 +501,7 @@ void free_colors(struct colors *colors) { } } -static CFILE *cfalloc(void) { +CFILE *cfwrap(FILE *file, const struct colors *colors, bool close) { CFILE *cfile = malloc(sizeof(*cfile)); if (!cfile) { return NULL; @@ -513,43 +513,8 @@ static CFILE *cfalloc(void) { return NULL; } - cfile->file = NULL; - cfile->colors = NULL; - cfile->close = false; - - return cfile; -} - -CFILE *cfopen(const char *path, const struct colors *colors) { - CFILE *cfile = cfalloc(); - if (!cfile) { - return NULL; - } - - cfile->file = xfopen(path, O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC); - if (!cfile->file) { - cfclose(cfile); - return NULL; - } - cfile->close = true; - - if (isatty(fileno(cfile->file))) { - cfile->colors = colors; - } else { - cfile->colors = NULL; - } - - return cfile; -} - -CFILE *cfdup(FILE *file, const struct colors *colors) { - CFILE *cfile = cfalloc(); - if (!cfile) { - return NULL; - } - - cfile->close = false; cfile->file = file; + cfile->close = close; if (isatty(fileno(file))) { cfile->colors = colors; diff --git a/color.h b/color.h index 55e89ff..9b83ea4 100644 --- a/color.h +++ b/color.h @@ -1,6 +1,6 @@ /**************************************************************************** * bfs * - * Copyright (C) 2015-2020 Tavian Barnes * + * Copyright (C) 2015-2021 Tavian Barnes * * * * Permission to use, copy, modify, and/or distribute this software for any * * purpose with or without fee is hereby granted. * @@ -63,33 +63,26 @@ typedef struct CFILE { } CFILE; /** - * Open a file for colored output. - * - * @param path - * The path to the file to open. - * @param colors - * The color table to use if file is a TTY. - * @return A colored file stream. - */ -CFILE *cfopen(const char *path, const struct colors *colors); - -/** - * Make a colored copy of an open file. + * Wrap an existing file into a colored stream. * * @param file * The underlying file. * @param colors * The color table to use if file is a TTY. - * @return A colored wrapper around file. + * @param close + * Whether to close the underlying stream when this stream is closed. + * @return + * A colored wrapper around file. */ -CFILE *cfdup(FILE *file, const struct colors *colors); +CFILE *cfwrap(FILE *file, const struct colors *colors, bool close); /** * Close a colored file. * * @param cfile * The colored file to close. - * @return 0 on success, -1 on failure. + * @return + * 0 on success, -1 on failure. */ int cfclose(CFILE *cfile); @@ -114,7 +107,8 @@ int cfclose(CFILE *cfile); * %%: A literal '%' * ${cc}: Change the color to 'cc' * $$: A literal '$' - * @return 0 on success, -1 on failure. + * @return + * 0 on success, -1 on failure. */ BFS_FORMATTER(2, 3) int cfprintf(CFILE *cfile, const char *format, ...); diff --git a/ctx.c b/ctx.c index 450d87e..c86e41d 100644 --- a/ctx.c +++ b/ctx.c @@ -161,7 +161,7 @@ struct bfs_ctx_file { const char *path; }; -struct CFILE *bfs_ctx_dedup(struct bfs_ctx *ctx, CFILE *cfile, const char *path) { +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) { return NULL; diff --git a/parse.c b/parse.c index 98e7dbb..22c2473 100644 --- a/parse.c +++ b/parse.c @@ -334,25 +334,45 @@ 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; - CFILE *cfile = cfopen(path, state->use_color ? ctx->colors : NULL); + FILE *file = NULL; + CFILE *cfile = NULL; + + file = xfopen(path, O_WRONLY | O_CREAT | O_CLOEXEC); + if (!file) { + goto fail; + } + + cfile = cfwrap(file, state->use_color ? ctx->colors : NULL, true); if (!cfile) { - parse_error(state, "${blu}%s${rs} ${bld}%s${rs}: %m.\n", expr->argv[0], path); - return -1; + goto fail; } 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); + goto fail; + } + + if (dedup == cfile) { + // O_TRUNC was omitted above to avoid repeatedly truncating the same file, so do it + // manually here + if (ftruncate(fileno(file), 0) != 0) { + goto fail; + } + } else { cfclose(cfile); - return -1; } expr->cfile = dedup; + return 0; - if (dedup != cfile) { +fail: + parse_error(state, "${blu}%s${rs} ${bld}%s${rs}: %m.\n", expr->argv[0], path); + if (cfile) { cfclose(cfile); + } else if (file) { + fclose(file); } - return 0; + return -1; } /** @@ -2675,12 +2695,11 @@ static CFILE *launch_pager(pid_t *pid, CFILE *cout) { } pipefd[1] = -1; - CFILE *ret = cfdup(file, NULL); + CFILE *ret = cfwrap(file, NULL, true); if (!ret) { goto fail_file; } file = NULL; - ret->close = true; struct bfs_spawn ctx; if (bfs_spawn_init(&ctx) != 0) { @@ -3689,15 +3708,15 @@ struct bfs_ctx *bfs_parse_cmdline(int argc, char *argv[]) { ctx->colors_error = errno; } - ctx->cerr = cfdup(stderr, use_color ? ctx->colors : NULL); + ctx->cerr = cfwrap(stderr, use_color ? ctx->colors : NULL, false); if (!ctx->cerr) { - perror("cfdup()"); + perror("cfwrap()"); goto fail; } - ctx->cout = cfdup(stdout, use_color ? ctx->colors : NULL); + ctx->cout = cfwrap(stdout, use_color ? ctx->colors : NULL, false); if (!ctx->cout) { - bfs_perror(ctx, "cfdup()"); + bfs_perror(ctx, "cfwrap()"); goto fail; } diff --git a/tests.sh b/tests.sh index 0b2d931..01ff9c0 100755 --- a/tests.sh +++ b/tests.sh @@ -518,6 +518,7 @@ gnu_tests=( test_fprint test_fprint_duplicate test_fprint_error + test_fprint_truncate test_fprint0 @@ -723,6 +724,7 @@ bfs_tests=( test_execdir_plus + test_fprint_append test_fprint_duplicate_stdout test_fprint_error_stdout test_fprint_error_stderr @@ -1725,6 +1727,33 @@ function test_fprint_duplicate_stdout() { fi } +function test_fprint_truncate() { + printf "basic\nbasic\n" >scratch/test_fprint_truncate.out + + invoke_bfs basic -maxdepth 0 -fprint scratch/test_fprint_truncate.out + sort -o scratch/test_fprint_truncate.out scratch/test_fprint_truncate.out + + if [ "$UPDATE" ]; then + cp {scratch,"$TESTS"}/test_fprint_truncate.out + else + diff -u {"$TESTS",scratch}/test_fprint_truncate.out + fi +} + +function test_fprint_append() { + rm -f scratch/test_fprint_append.out + + invoke_bfs basic -fprint scratch/test_fprint_append.out >>scratch/test_fprint_append.out + invoke_bfs basic -fprint scratch/test_fprint_append.out >>scratch/test_fprint_append.out + sort -o scratch/test_fprint_append.out scratch/test_fprint_append.out + + if [ "$UPDATE" ]; then + cp {scratch,"$TESTS"}/test_fprint_append.out + else + diff -u {"$TESTS",scratch}/test_fprint_append.out + fi +} + function test_double_dash() { cd basic bfs_diff -- . -type f diff --git a/tests/test_fprint_append.out b/tests/test_fprint_append.out new file mode 100644 index 0000000..6c21751 --- /dev/null +++ b/tests/test_fprint_append.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_fprint_truncate.out b/tests/test_fprint_truncate.out new file mode 100644 index 0000000..15a13db --- /dev/null +++ b/tests/test_fprint_truncate.out @@ -0,0 +1 @@ +basic diff --git a/util.c b/util.c index 728f962..2780447 100644 --- a/util.c +++ b/util.c @@ -443,6 +443,7 @@ FILE *xfopen(const char *path, int flags) { break; default: assert(!"Invalid access mode"); + errno = EINVAL; return NULL; } -- cgit v1.2.3