Junio C Hamano <junkio@xxxxxxx> wrote:
>> - exit(p->fn(argc, argv, prefix));
>> + status = p->fn(argc, argv, prefix);
>> +
>> + /* Close stdout if necessary, and diagnose any failure
>> + other than EPIPE. */
>> + if (fcntl(fileno (stdout), F_GETFD) >= 0) {
>> + errno = 0;
>> + if ((ferror(stdout) || fclose(stdout))
>> + && errno != EPIPE) {
>> + if (errno == 0)
>> + die("write failure on standard output");
>> + else
>> + die("write failure on standard output"
>> + ": %s", strerror(errno));
>> + }
>
> This makes the final write failure trump the breakage p->fn()
> already diagnosed, doesn't it?
Yes. Are there circumstances in which a nonzero status from
some cmd_* function would mean something so grave that you
wouldn't also want to know that standard output is incomplete
or corrupt (and possibly use a different exit status)?
So far, after a quick and incomplete survey, I haven't found any.
However, if some git command is documented to exit with
status N for some listed values of N, e.g.,
1 A happened
2 B happened
3 any other failure
then the above choice of dying with "die" would be wrong.
E.g. git-diff's --exit-code comes close:
--exit-code
Make the program exit with codes similar to diff(1). That is, it
exits with 1 if there were differences and 0 means no differences.
but doesn't say how it handles errors.
[ OT: Perhaps that documentation should be changed to look more like diff's,
so that it says there is a different exit code for the third
case (some failure):
$ diff --help|tail -3|head -1
Exit status is 0 if inputs are the same, 1 if different, 2 if trouble.
]
> Maybe if (fcntrl(...) >=0 )
> should read if (!status && fcntrl(...) >= 0).
No, because then something like git-diff's --exit-code could hide
a write error.
If you want to preserve the exit status, then it should be enough
to call set_die_routine with a function that will work just like
"die" but exit with a specified (status) value.
-
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
