Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> This is trying to implement the strict IO error checks that Jim Meyering
> suggested, but explicitly limits it to just regular files. If a pipe gets
> closed on us, we shouldn't complain about it.
...
> Hmm? I'm not saying this is the only way to do this, but I think this is
> at least likely to be obviously just an improvement, and it leaves room
> for further tweaking of the logic if Jim or others find other cases that
> should be handled.
...
> + /* Check for ENOSPC and EIO errors.. */
> + if (!fstat(fileno(stdout), &st) && S_ISREG(st.st_mode)) {
> + if (ferror(stdout))
> + die("write failure on standard output");
> + if (fflush(stdout) || fclose(stdout))
> + die("write failure on standard output: %s", strerror(errno));
> + }
> + exit(0);
Here is a patch relative to "next", to restore some of the
functionality that was provided by my initially-proposed change.
>From 379aee16596d29b83c95068964c349399b9b9f47 Mon Sep 17 00:00:00 2001
From: Jim Meyering <jim@xxxxxxxxxxxx>
Date: Mon, 25 Jun 2007 18:54:12 +0200
Subject: [PATCH] When detecting write failure, print strerror when possible.
Do not call "die" solely on the basis of ferror.
Instead, call both ferror and fclose, and *then* decide whether/how to die.
Without this patch, some commands unnecessarily omit strerror(errno)
when they fail:
$ ./git-ls-tree HEAD > /dev/full
fatal: write failure on standard output
With the patch, git reports the desired ENOSPC diagnostic:
fatal: write failure on standard output: No space left on device
FWIW, this version of close_stream is similar to the one
I included in another recent patch, but, is slightly cleaner.
Also, rather than returning zero or EOF, this one simply returns
zero or nonzero.
* git.c (close_stream): New function.
(run_command): Don't die solely because of ferror. Use close_stream.
Signed-off-by: Jim Meyering <jim@xxxxxxxxxxxx>
---
git.c | 26 ++++++++++++++++++++++----
1 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/git.c b/git.c
index b949cbb..b00bb1c 100644
--- a/git.c
+++ b/git.c
@@ -246,6 +246,21 @@ struct cmd_struct {
int option;
};
+static int
+close_stream(FILE *stream)
+{
+ int prev_fail = ferror(stream);
+ int fclose_fail = fclose(stream);
+
+ /* If there was a previous failure, but fclose succeeded,
+ clear errno, since ferror does not set it, and its value
+ may be unrelated to the ferror-reported failure. */
+ if (prev_fail && !fclose_fail)
+ errno = 0;
+
+ return prev_fail || fclose_fail;
+}
+
static int run_command(struct cmd_struct *p, int argc, const char **argv)
{
int status;
@@ -274,10 +289,13 @@ static int run_command(struct cmd_struct *p, int argc, const char **argv)
return 0;
/* Check for ENOSPC and EIO errors.. */
- if (ferror(stdout))
- die("write failure on standard output");
- if (fflush(stdout) || fclose(stdout))
- die("write failure on standard output: %s", strerror(errno));
+ if (close_stream(stdout)) {
+ if (errno == 0)
+ die("write failure on standard output");
+ else
+ die("write failure on standard output: %s",
+ 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
