Re: [PATCH] diff: fix textconv error zombies

[Johannes Sixt <j6t@kdbg.org>]

On Dienstag, 30. März 2010, Junio C Hamano wrote:
> In "if necessary; see below", I tried to find something to help me judge
> why you said "That's really not _that_ important in this case", but I
> think I failed.  Perhaps we would want a bit more detailed discussion in
> the document there?

The "if necessary; see below" refers to the cases where the caller requests a 
file descriptor from start_command(). It is safe to follow the recipe. But I 
actually would not want to expand on the special case like we have in 
run_textconv, where it is not strictly necessary to close the fd before 
finish_command(), namely when we know that the writer will exit after it has 
closed its end of the pipe and the reader reads until EOF.

This could deadlock in the theoretical case that the read() in the parent 
returns an error even though the child is still writing data; the child would 
hang while waiting for the pipe being drained by the parent; but the parent 
would wait for the child to exit, which never happens. Doing the close() 
before finish_command() avoids this situation because the child dies of 
SIGPIPE.

But since the communication is via a pipe, and read failures are highly 
unlikely (and a read failure in a way that the writer does not fail, too, is 
even more unlikely, I think), the close() before finish_command() is "not 
important".

-- Hannes