From 0879126bb44aa9761a920946f35f5aa697717445 Mon Sep 17 00:00:00 2001
From: Tavian Barnes <tavianator@tavianator.com>
Date: Thu, 10 Nov 2022 12:53:45 -0500
Subject: Try to report I/O errors earlier and only once

---
 src/ctx.c   | 32 +++++++++++++++++++++++++++-----
 src/eval.c  | 24 +++++++++++++++++++-----
 src/expr.h  |  2 ++
 src/parse.c |  2 ++
 4 files changed, 50 insertions(+), 10 deletions(-)

diff --git a/src/ctx.c b/src/ctx.c
index 4b373a1..b9d15bb 100644
--- a/src/ctx.c
+++ b/src/ctx.c
@@ -139,6 +139,8 @@ struct bfs_ctx_file {
 	CFILE *cfile;
 	/** The path to the file (for diagnostics). */
 	const char *path;
+	/** Remembers I/O errors, to propagate them to the exit status. */
+	int error;
 };
 
 CFILE *bfs_ctx_dedup(struct bfs_ctx *ctx, CFILE *cfile, const char *path) {
@@ -169,6 +171,7 @@ CFILE *bfs_ctx_dedup(struct bfs_ctx *ctx, CFILE *cfile, const char *path) {
 
 	ctx_file->cfile = cfile;
 	ctx_file->path = path;
+	ctx_file->error = 0;
 
 	if (cfile != ctx->cout && cfile != ctx->cerr) {
 		++ctx->nfiles;
@@ -182,10 +185,24 @@ void bfs_ctx_flush(const struct bfs_ctx *ctx) {
 	// - the user sees everything relevant before an -ok[dir] prompt
 	// - output from commands is interleaved consistently with bfs
 	// - executed commands can rely on I/O from other bfs actions
-	//
-	// We do not check errors here, but they will be caught at cleanup time
-	// with ferror().
-	fflush(NULL);
+	TRIE_FOR_EACH(&ctx->files, leaf) {
+		struct bfs_ctx_file *ctx_file = leaf->value;
+		CFILE *cfile = ctx_file->cfile;
+		if (fflush(cfile->file) == 0) {
+			continue;
+		}
+
+		ctx_file->error = errno;
+		clearerr(cfile->file);
+
+		const char *path = ctx_file->path;
+		if (path) {
+			bfs_error(ctx, "'%s': %m.\n", path);
+		} else if (cfile == ctx->cout) {
+			bfs_error(ctx, "(standard output): %m.\n");
+		}
+
+	}
 
 	// Flush the user/group caches, in case the executed command edits the
 	// user/group tables
@@ -258,6 +275,11 @@ int bfs_ctx_free(struct bfs_ctx *ctx) {
 		TRIE_FOR_EACH(&ctx->files, leaf) {
 			struct bfs_ctx_file *ctx_file = leaf->value;
 
+			if (ctx_file->error) {
+				// An error was previously reported during bfs_ctx_flush()
+				ret = -1;
+			}
+
 			if (bfs_ctx_fclose(ctx, ctx_file) != 0) {
 				if (cerr) {
 					bfs_error(ctx, "'%s': %m.\n", ctx_file->path);
@@ -271,7 +293,7 @@ int bfs_ctx_free(struct bfs_ctx *ctx) {
 
 		if (cout && bfs_ctx_fflush(cout) != 0) {
 			if (cerr) {
-				bfs_error(ctx, "standard output: %m.\n");
+				bfs_error(ctx, "(standard output): %m.\n");
 			}
 			ret = -1;
 		}
diff --git a/src/eval.c b/src/eval.c
index f756617..dd147c9 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -108,6 +108,20 @@ static void eval_report_error(struct bfs_eval *state) {
 	}
 }
 
+/**
+ * Report an I/O error that occurs during evaluation.
+ */
+static void eval_io_error(const struct bfs_expr *expr, struct bfs_eval *state) {
+	if (expr->path) {
+		eval_error(state, "'%s': %m.\n", expr->path);
+	} else {
+		eval_error(state, "(standard output): %m.\n");
+	}
+
+	// Don't report the error again in bfs_ctx_free()
+	clearerr(expr->cfile->file);
+}
+
 /**
  * Perform a bfs_stat() call if necessary.
  */
@@ -732,7 +746,7 @@ done:
 	return true;
 
 error:
-	eval_report_error(state);
+	eval_io_error(expr, state);
 	return true;
 }
 
@@ -741,7 +755,7 @@ error:
  */
 bool eval_fprint(const struct bfs_expr *expr, struct bfs_eval *state) {
 	if (cfprintf(expr->cfile, "%pP\n", state->ftwbuf) < 0) {
-		eval_report_error(state);
+		eval_io_error(expr, state);
 	}
 	return true;
 }
@@ -753,7 +767,7 @@ bool eval_fprint0(const struct bfs_expr *expr, struct bfs_eval *state) {
 	const char *path = state->ftwbuf->path;
 	size_t length = strlen(path) + 1;
 	if (fwrite(path, 1, length, expr->cfile->file) != length) {
-		eval_report_error(state);
+		eval_io_error(expr, state);
 	}
 	return true;
 }
@@ -763,7 +777,7 @@ bool eval_fprint0(const struct bfs_expr *expr, struct bfs_eval *state) {
  */
 bool eval_fprintf(const struct bfs_expr *expr, struct bfs_eval *state) {
 	if (bfs_printf(expr->cfile, expr->printf, state->ftwbuf) != 0) {
-		eval_report_error(state);
+		eval_io_error(expr, state);
 	}
 
 	return true;
@@ -803,7 +817,7 @@ bool eval_fprintx(const struct bfs_expr *expr, struct bfs_eval *state) {
 	return true;
 
 error:
-	eval_report_error(state);
+	eval_io_error(expr, state);
 	return true;
 }
 
diff --git a/src/expr.h b/src/expr.h
index 1f1ece6..22f569e 100644
--- a/src/expr.h
+++ b/src/expr.h
@@ -162,6 +162,8 @@ struct bfs_expr {
 		struct {
 			/** The output stream. */
 			CFILE *cfile;
+			/** Optional file path. */
+			const char *path;
 			/** Optional -printf format. */
 			struct bfs_printf *printf;
 		};
diff --git a/src/parse.c b/src/parse.c
index 74f160c..47f4bec 100644
--- a/src/parse.c
+++ b/src/parse.c
@@ -481,6 +481,7 @@ static void init_print_expr(struct parser_state *state, struct bfs_expr *expr) {
 	expr_set_always_true(expr);
 	expr->cost = PRINT_COST;
 	expr->cfile = state->ctx->cout;
+	expr->path = NULL;
 }
 
 /**
@@ -512,6 +513,7 @@ static int expr_open(struct parser_state *state, struct bfs_expr *expr, const ch
 	}
 
 	expr->cfile = dedup;
+	expr->path = path;
 	return 0;
 
 fail:
-- 
cgit v1.2.3