[PATCH] fast-import.c: detect fclose- and fflush-induced write failure

[Jim Meyering <jim@meyering.net>]

There are potentially ignored write errors in fast-import.c.
Here's a proof-of-concept patch.
Something like close_stream might be worth making more generally
accessible, but rather than close_wstream_or_die, I'd prefer general
purpose die-like functions that take an errno value, e.g.,

  die_errno (errno, fmt_str, arg1, ...)

that would work just like die, with this change:
- when errno is nonzero,
    append the concatenation of ": " and strerror(errno)
    between the format-string-denoted output and the final newline
- when errno is zero
    work just like "die" currently does.

This functionality is like that provided by the error, warn*,
err, verr, etc. functions that have been in glibc forever.

With such a function (and an error_errno analog), the nearly-identical
uses of "error" added below and the nearly identical uses of "die" that
might end up in git.c could be factored out.

Here's a ChangeLog-style entry:

 (end_packfile): Die upon fflush failure.
 (close_stream, close_wstream_or_die): New functions.
 (dump_marks): Upon fclose failure, rollback the lock and give a diagnostic.
 (main): Die upon fclose failure.  Record pack_edges file name for use in diagnostics.

Signed-off-by: Jim Meyering <jim@meyering.net>
---
 fast-import.c |   54 ++++++++++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index f9bfcc7..7941839 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -793,7 +793,9 @@ static void end_packfile(void)
 					fprintf(pack_edges, " %s", sha1_to_hex(t->sha1));
 			}
 			fputc('\n', pack_edges);
-			fflush(pack_edges);
+			if (fflush(pack_edges))
+				die("failed to write pack-edges file: %s",
+				    strerror(errno));
 		}

 		pack_id++;
@@ -1344,6 +1346,31 @@ static void dump_marks_helper(FILE *f,
 	}
 }

+static int
+close_stream(FILE *stream)
+{
+	int prev_fail = (ferror(stream) != 0);
+	int fclose_fail = (fclose(stream) != 0);
+
+	if (prev_fail || fclose_fail) {
+		if (! fclose_fail)
+			errno = 0;
+		return EOF;
+	}
+	return 0;
+}
+
+static void
+close_wstream_or_die(FILE *stream, const char *file_name)
+{
+	if (close_stream(stream)) {
+		if (errno == 0)
+			die ("%s: write failed: %s", file_name, strerror(errno));
+		else
+			die ("%s: write failed", file_name);
+	}
+}
+
 static void dump_marks(void)
 {
 	static struct lock_file mark_lock;
@@ -1369,7 +1396,18 @@ static void dump_marks(void)
 	}

 	dump_marks_helper(f, 0, marks);
-	fclose(f);
+	if (close_stream(f) != 0) {
+		int close_errno = errno;
+		rollback_lock_file(&mark_lock);
+		failure |=
+		  (close_errno == 0
+		   ? error("Failed to write temporary marks file %s.lock",
+			   mark_file)
+		   : error("Failed to write temporary marks file %s.lock: %s",
+			   mark_file, strerror(close_errno)));
+		return;
+	}
+
 	if (commit_lock_file(&mark_lock))
 		failure |= error("Unable to write marks file %s: %s",
 			mark_file, strerror(errno));
@@ -2015,6 +2053,7 @@ static const char fast_import_usage[] =
 int main(int argc, const char **argv)
 {
 	int i, show_stats = 1;
+	const char *pack_edges_file = NULL;

 	git_config(git_default_config);
 	alloc_objects(object_entry_alloc);
@@ -2052,10 +2091,13 @@ int main(int argc, const char **argv)
 			mark_file = a + 15;
 		else if (!prefixcmp(a, "--export-pack-edges=")) {
 			if (pack_edges)
-				fclose(pack_edges);
-			pack_edges = fopen(a + 20, "a");
+				close_wstream_or_die(pack_edges,
+						     pack_edges_file);
+			pack_edges_file = a + 20;
+			pack_edges = fopen(pack_edges_file, "a");
 			if (!pack_edges)
-				die("Cannot open %s: %s", a + 20, strerror(errno));
+				die("Cannot open %s: %s", pack_edges_file,
+				    strerror(errno));
 		} else if (!strcmp(a, "--force"))
 			force_update = 1;
 		else if (!strcmp(a, "--quiet"))
@@ -2095,7 +2137,7 @@ int main(int argc, const char **argv)
 	dump_marks();

 	if (pack_edges)
-		fclose(pack_edges);
+		close_wstream_or_die(pack_edges, pack_edges_file);

 	if (show_stats) {
 		uintmax_t total_count = 0, duplicate_count = 0;