Re: [PATCH] start_command(), if .in/.out > 0, closes file descriptors, not the callers

From: Shawn O. Pearce <spearce@spearce.org>
Date: Feb 17, 2008, 07:42 UTC
Message-ID: <20080217074252.GO24004@spearce.org>
In-Reply-To: <7vmyq07bqe.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:

> Johannes Sixt <johannes.sixt@telecom.at> writes:
> 
> These two patches make the calling convention more uniform and
> it feels like a good clean-up of the semantics.

...

Show 16 quoted lines

> > diff --git a/run-command.h b/run-command.h
> > index e9c84d0..0e67472 100644
> > --- a/run-command.h
> > +++ b/run-command.h
> > @@ -14,6 +14,23 @@ enum {
> >  struct child_process {
> >  	const char **argv;
> >  	pid_t pid;
> > +	/*
> > +	 * Using .in, .out, .err:
> > +	 * - Specify 0 to inherit stdin, stdout, stderr from parent.
> 
> Wouldn't this be better if 0, 1, or 2 specify inheriting
> these respectively?
> 
> I am confused...

Using 0 to inherit stdin/stdout/stderr makes it easy for callers of run_command() to setup something like this:

	struct child_process p;
	memset(&p, 0, sizeof(p));
	p.git_cmd = 1;
	p.argv = args;
	run_command(&p);

and have the child inherit stdin/stdout/stderr automatically, much as a fork() automatically inherits those into the child process.

So I agree with keeping 0 for inheriting, rather than needing to set "p.in = 0; p.out = 1; p.err = 2". We don't always need (or want) the redirection service.

I think its OK (although maybe somewhat crazy) for the caller to set "p.in = 1" and thus ask start_command() to close(1) before it returns.

What happens if the parent has already closed fds 1 and 2 but then opens a network socket somewhere and wants that socket to be stdin of the child? Its at fd 1. But the parent doesn't want to track that fd either, she wants it to go into the child and only the child. So it seems reasonable to make this the "give away" case, and that's what it always had been.

Of course the same argument can be said for fd 0; if the parent does close(0) before some other open/socket/pipe call then its obviously possible for the parent to get a fd that it wants the child to take over and close.

We're basically assuming that the parent will always keep its
own stdin open if it will be spawning children.  We all know what
happens when we assume (we double close file descriptors) but I
think its a reasonable safe assumption to be making.

-- 
Shawn.