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

From: "Shawn O. Pearce" <[email-masked]>
Date: Sun, 17 Feb 2008 02:42:52 -0500

Junio C Hamano <[email-masked]> wrote:
> Johannes Sixt <[email-masked]> writes:
> 
> These two patches make the calling convention more uniform and
> it feels like a good clean-up of the semantics.
...
> > 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.