Re: [PATCH v2 properly indented] fix start_command() bug when stdin is closed

Junio C Hamano schrieb:

Paolo Bonzini [off-list ref] writes:

quoted

 int start_command(struct child_process *cmd)
 {
 	int need_in, need_out, need_err;
 	int fdin[2], fdout[2], fderr[2];
 
 	/*
+	 * Make sure that all file descriptors <= 2 are open, otherwise we
+	 * mess them up when dup'ing pipes onto stdin/stdout/stderr.  Since
+	 * we are at it, save a file descriptor on /dev/null to use it later.
+	 */
+	if (devnull_fd == -1) {
+		devnull_fd = open("/dev/null", O_RDWR);
+		while (devnull_fd >= 0 && devnull_fd <= 2)
+			devnull_fd = dup(devnull_fd);
+		if (devnull_fd == -1)
+			die("opening /dev/null failed (%s)", strerror(errno));
+	}
+

I may be misreading the patch but, this logic always opens /dev/null, if
nobody asked for *any* cmd->no_stdXXX and low 3 fds are occupied, and
worse, it keeps fd=3 open.

Making sure low fds 0, 1 and 2 are open is a good thing.  I do not think
clobbering fd=3 is good.

It is sometimes _unnecessary_, but I don't see why it should hurt. The
effect on performance will be in the noise.

Also shouldn't this be done only on the side that dup()s fds around,
i.e. in the child process after fork()?  Why is this done for the parent?

Because it must be done *before* the pipe()s are created so that they
don't occupy fds 0-2.

-- Hannes