Don't truncate files until we know they're not duplicates

8 files changed, 115 insertions, 68 deletions

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 <tavianator@tavianator.com>        *
+ * Copyright (C) 2015-2021 Tavian Barnes <tavianator@tavianator.com>        *
  *                                                                          *
  * 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;
 	}