On Fri, 29 Jun 2007, Junio C Hamano wrote:
>
> Thanks for bringing it up, as I had the same "Huh?" moment.
> I would probably call that simply "do_not_flush". Or name the
> variable "flush_stdout" and swap all the logic.
I think that patch looks fine, but I also think that there is a more
fundamental problem with this approach:
- all these patches basically break the whole _point_ of Jim's original
reason for wanting this!
So remember, we had two different reasons for flushing:
- interactivity over a pipe.
Tools like "gitk" and "git gui blame" all run waiting for input, and we
want to give them the commit data as soon as possible.
- error checking on a filesystem.
Yes, "ferror()" _after_ the write shows an error happened, but doesn't
show what error it was. Doing a fflush() is a lot more likely to
actually give the right errors.
So non-files want flushing due to latency issues, and files want flushing
due to error handling. And the patch under discussion basically broke the
error handling.
(It turns out that it doesn't break it for the trivial "/dev/full"
testcase, since that doesn't show up as a file, but it would break it for
the *real* case of a filesystem that becomes full).
One option is to change the git.c thing to do
if (fflush(stdout))
die("write failure on standard output: %s", strerror(errno));
if (ferror(stdout))
die("unknown write failure on standard output");
if (fclose(stdout))
die("close failure on standard output: %s", strerror(errno));
where the "fflush()" is done first (regardless of ferror), just in the
(probably futile) hope of getting the right errno.
Linus
-
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
