Subject: Re: [PATCH v2 4/6] run-command: don't die in child when duping /dev/null

From: Brandon Williams <[email-masked]>
Date: Thu, 13 Apr 2017 12:43:17 -0700

On 04/13, Eric Wong wrote:
> Brandon Williams <[email-masked]> wrote:
> > @@ -487,7 +483,7 @@ int start_command(struct child_process *cmd)
> >  		atexit(notify_parent);
> >  
> >  		if (cmd->no_stdin)
> > -			dup_devnull(0);
> > +			dup2(null_fd, 0);
> 
> I prefer we keep error checking for dup2 failures,
> and also add more error checking for unchecked dup2 calls.
> Can be a separate patch, I suppose.
> 
> Ditto for other dup2 changes

I simply figured that since we weren't doing the checks on the other
dup2 calls that I would keep it consistent.  But if we wanted to add in
more error checking then we can can in another patch.

> 
> > @@ -558,6 +554,8 @@ int start_command(struct child_process *cmd)
> >  	}
> >  	close(notify_pipe[0]);
> >  
> > +	if (null_fd > 0)
> > +		close(null_fd);
> 
> I would prefer:
> 
> 	if (null_fd >= 0)
> 
> here, even if we currently do not release stdin.

K will do.

> >  	argv_array_clear(&argv);
> >  	free(childenv);

-- 
Brandon Williams