Junio C Hamano [off-list ref] wrote:
Linus Torvalds [off-list ref] writes:
...
quoted
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 took the liberty of munging your two patches to follow your
comments above
That has the disadvantage of ignoring *all* pipe and socket write errors.
IMHO, git would be better served if it didn't do that, since writing to
those can fail with EIO and even a new one: EACCES (though this latter
is only for sockets). Also possible, according to POSIX: ENOBUFS.
Of course, one can probably argue that those are all unlikely.
They may be even less likely than an actual EPIPE, but the point is
that people and tools using git plumbing should be able to rely on
it to report such write failures, no matter how unusual they are.
On Mon, 25 Jun 2007, Jim Meyering wrote:
That has the disadvantage of ignoring *all* pipe and socket write errors.
.. which is the only wayt to do it.
There's *no*way* to tell what the error was for the case of
if (ferror(stdout))
..
because the original errno has long long since been thrown away.
Of course, one can probably argue that those are all unlikely.
They may be even less likely than an actual EPIPE, but the point is
that people and tools using git plumbing should be able to rely on
it to report such write failures, no matter how unusual they are.
..but since what you suggest is physically impossible in a half-way
portable manner, and since all relevant such errors are likely to have
happened long before, I would suggest:
- Use my patch
OR
- Stop using stdio.
You *cannot* make stdio error handling sane. It's simply not possible.
The whole point of stdio is to simplify things, but it does so to the
point where portable and reliable error handling is not an option any
more.
Replace all command output that you care about with some stdio
replacement. We do that for all the paths we *really* care about, where
we use raw unistd IO and do our own buffering (ie things like the
"write_in_full()" stuff and "write_sha1_file()" etc.
Anybody who thinks he can handle errors with stdio is simply barking up
the wrogn tree. It can be done, but it can be done only by essentially
making stdio be a really awkward way to do IO, and you're better off with
the raw unistd.h interfaces.
In particular, you need to make sure you check *each* return value of
every single stdio operation. Even then, you don't actually know if
"errno" is set correctly if an error happens in the middle (ie most stdio
routines are just defined to return EOF or nonzero on error, and it's not
at all clear that errno is reliable).
If you don't, a buffer flush error may have happened at any time, and
whatever error code it had has been thrown away, and turned into one
single bit (the "ferror()" thing).
And yes, some libc's might have extensions that actually guarantee more.
But even then I would be *very* surprised if they actually work and have
been tested to any real degree (glibc does not. The stream error is a
single bit. I checked)
So really: if you care about a particular read or write, use the
"write_in_full()" and "read_in_full()" functions. NOTHING ELSE!
Linus