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.
[ Side note: feel free to change the S_ISREG() to a !S_ISFIFO(). That
would allow easier testing with /dev/full, which tends to be a character
special device. That said, I think sockets and pipes are generally
interchangeable, which is why I limited it to just regular files. ]
If the subcommand already returned an error, that takes precedence (and we
assume that the subcommand already printed out any relevant messages
relating to it)
Signed-off-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
---
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.
Side note: I think I made a mistake in making the run_command() a NORETURN
function and putting the exit() into it. It's probably better to instead
just make it return "int", and make the caller do
exit(run_command(...));
and that makes it much prettier to have "run_command()" just return early
if an error happens (or doesn't happen).
For example, then we could just do
status = p->fn(...);
if (status)
return status;
/* Somebody closed stdout? */
if (fstat(fileno(stdout), &st))
return 0;
/* Ignore write errors for pipes and sockets.. */
if (S_ISFIFO(st.st_mode) || S_ISSOCK(st.st_mode))
return 0;
which makes it easy to explain what's going on, and avoids having any deep
indentation at all.
I dunno. This passes all the tests, but it's not like we currently test
for ENOSPC/EIO anyway, or even can do that. If we _just_ disable the thing
for pipes/sockets, we could add a test using /dev/full.
I did check that changing it to !S_ISFIFO() gets the right behaviour, and
"git log | head" doesn't complain, while "git log > /dev/full" does. So
this has gotten some very rudimentary testing, but not in the exact form
I'm actually sending it out.
git.c | 15 ++++++++++++++-
1 files changed, 14 insertions(+), 1 deletions(-)
diff --git a/git.c b/git.c
index 6c728e4..db0118a 100644
--- a/git.c
+++ b/git.c
@@ -224,6 +224,8 @@ struct cmd_struct {
static NORETURN void run_command(struct cmd_struct *p, int argc, const char **argv)
{
+ int status;
+ struct stat st;
const char *prefix;
prefix = NULL;
@@ -237,7 +239,18 @@ static NORETURN void run_command(struct cmd_struct *p, int argc, const char **ar
}
trace_argv_printf(argv, argc, "trace: built-in: git");
- exit(p->fn(argc, argv, prefix));
+ status = p->fn(argc, argv, prefix);
+ if (status)
+ exit(status);
+
+ /* 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);
}
static void handle_internal_command(int argc, const char **argv)
-
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
