Jim MeyeringThere 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(-)
Show changes to fast-import.c +48 −6
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;
