Marco Roeland <marco.roeland@xxxxxxxxx> wrote:
> The if statement with 'fcntl' is missing a brace, it should be:
>
> + if ((fcntl(fileno (stdout), F_GETFD) >= 0)
> + && (ferror(stdout) || fclose(stdout)))
Thank you.
That was a stale patch (from before I compiled/tested, obviously).
I intended this:
if (fcntl(fileno (stdout), F_GETFD) >= 0
&& (ferror(stdout) || fclose(stdout)))
Here's the correct one:
Subject: [PATCH] Don't ignore write failure from git-diff, git-log, etc.
Currently, when git-diff writes to a full device or gets an I/O error,
it fails to detect the write error:
$ git-diff |wc -c
3984
$ git-diff > /dev/full && echo ignored write failure
ignored write failure
git-log does the same thing:
$ git-log -n1 > /dev/full && echo ignored write failure
ignored write failure
Each and every git command should report such a failure.
Some already do, but with the patch below, they all do, and we
won't have to rely on code in each command's implementation to
perform the right incantation.
$ ./git-log -n1 > /dev/full
fatal: write failure on standard output: No space left on device
[Exit 128]
$ ./git-diff > /dev/full
fatal: write failure on standard output: No space left on device
[Exit 128]
You can demonstrate this with git's own --version output, too:
(but git --help detects the failure without this patch)
$ ./git --version > /dev/full
fatal: write failure on standard output: No space left on device
[Exit 128]
Note that the fcntl test (for whether the fileno may be closed) is
required in order to avoid EBADF upon closing an already-closed stdout,
as would happen for each git command that already closes stdout; I think
update-index was the one I noticed in the failure of t5400, before I
added that test.
Also, to be consistent, don't ignore EPIPE write failures.
Signed-off-by: Jim Meyering <jim@xxxxxxxxxxxx>
---
git.c | 11 ++++++++++-
merge-recursive.c | 3 ---
write_or_die.c | 4 ----
3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/git.c b/git.c
index 29b55a1..7b45a73 100644
--- a/git.c
+++ b/git.c
@@ -308,6 +308,7 @@ static void handle_internal_command(int argc, const char **argv, char **envp)
for (i = 0; i < ARRAY_SIZE(commands); i++) {
struct cmd_struct *p = commands+i;
const char *prefix;
+ int status;
if (strcmp(p->cmd, cmd))
continue;
@@ -321,7 +322,15 @@ static void handle_internal_command(int argc, const char **argv, char **envp)
die("%s must be run in a work tree", cmd);
trace_argv_printf(argv, argc, "trace: built-in: git");
- exit(p->fn(argc, argv, prefix));
+ status = p->fn(argc, argv, prefix);
+
+ /* Close stdout if necessary, and diagnose any failure. */
+ if (fcntl(fileno (stdout), F_GETFD) >= 0
+ && (ferror(stdout) || fclose(stdout)))
+ die("write failure on standard output: %s",
+ strerror(errno));
+
+ exit(status);
}
}
diff --git a/merge-recursive.c b/merge-recursive.c
index 8f72b2c..bfa4335 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -523,9 +523,6 @@ static void flush_buffer(int fd, const char *buf, unsigned long size)
while (size > 0) {
long ret = write_in_full(fd, buf, size);
if (ret < 0) {
- /* Ignore epipe */
- if (errno == EPIPE)
- break;
die("merge-recursive: %s", strerror(errno));
} else if (!ret) {
die("merge-recursive: disk full?");
diff --git a/write_or_die.c b/write_or_die.c
index 5c4bc85..fadfcaa 100644
--- a/write_or_die.c
+++ b/write_or_die.c
@@ -41,8 +41,6 @@ int write_in_full(int fd, const void *buf, size_t count)
void write_or_die(int fd, const void *buf, size_t count)
{
if (write_in_full(fd, buf, count) < 0) {
- if (errno == EPIPE)
- exit(0);
die("write error (%s)", strerror(errno));
}
}
@@ -50,8 +48,6 @@ void write_or_die(int fd, const void *buf, size_t count)
int write_or_whine_pipe(int fd, const void *buf, size_t count, const char *msg)
{
if (write_in_full(fd, buf, count) < 0) {
- if (errno == EPIPE)
- exit(0);
fprintf(stderr, "%s: write error (%s)\n",
msg, strerror(errno));
return 0;
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
