On 04/13, Eric Wong wrote:
> Brandon Williams <bmw...@google.com> 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

Reply via email to