From: Eric Wong <e@80x24.org>
Date: Thu, 13 Apr 2017 19:29:28 GMT
Subject: Re: [PATCH v2 4/6] run-command: don't die in child when duping /dev/null
Message-ID: <20170413192928.GA6129@whir>
In-Reply-To: <20170413183252.4713-5-bmwill@google.com>

Brandon Williams <bmwill@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

> @@ -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.

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