threads / patch / 7178

patch, 8 partsTeach run_command how to setup a stdin pipe

Subject: [PATCH 5/8] Teach run_command how to setup a stdin pipe

## tl;dr

4 messages between Mar 10, 2007 and Mar 11, 2007. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Shawn O. Pearce· Mar 10, 2007, 08:28 UTC · lore

Sometimes callers trying to use run_command to execute a child process will want to setup a pipe or file descriptor to redirect into the child's stdin.

This idea is completely stolen from builtin-bundle's fork_with_pipe, written by Johannes Schindelin. All credit (and blame) should lie with Dscho. ;-)

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 run-command.c |   35 ++++++++++++++++++++++++++++++++++-
 run-command.h |    3 +++
 2 files changed, 37 insertions(+), 1 deletions(-)

Show changes to 2 files +37 −1

run-command.c, run-command.h

diff --git a/run-command.c b/run-command.c
index a866a06..03ff7bc 100644
--- a/run-command.c
+++ b/run-command.c
@@ -4,15 +4,39 @@
 
 int start_command(struct child_process *cmd)
 {
+	int need_in = !cmd->no_stdin && cmd->in < 0;
+	int fdin[2];
+
+	if (need_in) {
+		if (pipe(fdin) < 0)
+			return -ERR_RUN_COMMAND_PIPE;
+		cmd->in = fdin[1];
+		cmd->close_in = 1;
+	}
+
 	cmd->pid = fork();
-	if (cmd->pid < 0)
+	if (cmd->pid < 0) {
+		if (need_in) {
+			close(fdin[0]);
+			close(fdin[1]);
+		}
 		return -ERR_RUN_COMMAND_FORK;
+	}
+
 	if (!cmd->pid) {
 		if (cmd->no_stdin) {
 			int fd = open("/dev/null", O_RDWR);
 			dup2(fd, 0);
 			close(fd);
+		} else if (need_in) {
+			dup2(fdin[0], 0);
+			close(fdin[0]);
+			close(fdin[1]);
+		} else if (cmd->in) {
+			dup2(cmd->in, 0);
+			close(cmd->in);
 		}
+
 		if (cmd->stdout_to_stderr)
 			dup2(2, 1);
 		if (cmd->git_cmd) {
@@ -22,11 +46,20 @@ int start_command(struct child_process *cmd)
 		}
 		die("exec %s failed.", cmd->argv[0]);
 	}
+
+	if (need_in)
+		close(fdin[0]);
+	else if (cmd->in)
+		close(cmd->in);
+
 	return 0;
 }
 
 int finish_command(struct child_process *cmd)
 {
+	if (cmd->close_in)
+		close(cmd->in);
+
 	for (;;) {
 		int status, code;
 		pid_t waiting = waitpid(cmd->pid, &status, 0);
diff --git a/run-command.h b/run-command.h
index 24cdb4e..ff09067 100644
--- a/run-command.h
+++ b/run-command.h
@@ -4,6 +4,7 @@
 enum {
 	ERR_RUN_COMMAND_FORK = 10000,
 	ERR_RUN_COMMAND_EXEC,
+	ERR_RUN_COMMAND_PIPE,
 	ERR_RUN_COMMAND_WAITPID,
 	ERR_RUN_COMMAND_WAITPID_WRONG_PID,
 	ERR_RUN_COMMAND_WAITPID_SIGNAL,
@@ -13,6 +14,8 @@ enum {
 struct child_process {
 	const char **argv;
 	pid_t pid;
+	int in;
+	unsigned close_in:1;
 	unsigned no_stdin:1;
 	unsigned git_cmd:1; /* if this is to be git sub-command */
 	unsigned stdout_to_stderr:1;

-- 
1.5.0.3.942.g299f

Johannes Schindelin· Mar 10, 2007, 16:54 UTC · re: Shawn O. Pearce · lore

Re: [PATCH 5/8] Teach run_command how to setup a stdin pipe

Hi,

On Sat, 10 Mar 2007, Shawn O. Pearce wrote:

Show 7 quoted lines

> Sometimes callers trying to use run_command to execute a child process 
> will want to setup a pipe or file descriptor to redirect into the 
> child's stdin.
> 
> This idea is completely stolen from builtin-bundle's fork_with_pipe, 
> written by Johannes Schindelin.  All credit (and blame) should lie with 
> Dscho.  ;-)

;-)

Thank you for starting this thread. I think it makes a lot of sense in the face of the MinGW port. (I am not interested in the hook stuff personally, so I'll not comment on that.)

Ciao, Dscho

Shawn O. Pearce· Mar 11, 2007, 01:45 UTC · re: Johannes Schindelin · lore

Re: [PATCH 5/8] Teach run_command how to setup a stdin pipe

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 15 quoted lines

> On Sat, 10 Mar 2007, Shawn O. Pearce wrote:
> 
> > Sometimes callers trying to use run_command to execute a child process 
> > will want to setup a pipe or file descriptor to redirect into the 
> > child's stdin.
> > 
> > This idea is completely stolen from builtin-bundle's fork_with_pipe, 
> > written by Johannes Schindelin.  All credit (and blame) should lie with 
> > Dscho.  ;-)
> 
> ;-)
> 
> Thank you for starting this thread. I think it makes a lot of sense in the 
> face of the MinGW port. (I am not interested in the hook stuff personally, 
> so I'll not comment on that.)

I'd like to see run_command learn how to also redirect stdout, then replace fork_with_pipe in builtin-bundle with run_command. We should be able to also improve some of our other more direct uses of fork to use run_command at that point too.

I don't know squat about MinGW, so I don't know if having the run_command abstraction really helps there or not, but I know we tried to make it with the good intentions of being able to use it to wrap that asinine CreateProcess() thing that Win32 has...

-- 
Shawn.

Johannes Schindelin· Mar 11, 2007, 02:07 UTC · re: Shawn O. Pearce · lore

Re: [PATCH 5/8] Teach run_command how to setup a stdin pipe

Hi,

On Sat, 10 Mar 2007, Shawn O. Pearce wrote:

Show 21 quoted lines

> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > On Sat, 10 Mar 2007, Shawn O. Pearce wrote:
> > 
> > > Sometimes callers trying to use run_command to execute a child 
> > > process will want to setup a pipe or file descriptor to redirect 
> > > into the child's stdin.
> > > 
> > > This idea is completely stolen from builtin-bundle's fork_with_pipe, 
> > > written by Johannes Schindelin.  All credit (and blame) should lie 
> > > with Dscho.  ;-)
> > 
> > ;-)
> > 
> > Thank you for starting this thread. I think it makes a lot of sense in 
> > the face of the MinGW port. (I am not interested in the hook stuff 
> > personally, so I'll not comment on that.)
> 
> I'd like to see run_command learn how to also redirect stdout, then 
> replace fork_with_pipe in builtin-bundle with run_command. We should be 
> able to also improve some of our other more direct uses of fork to use 
> run_command at that point too.

I concur.

> I don't know squat about MinGW, so I don't know if having the 
> run_command abstraction really helps there or not, but I know we tried 
> to make it with the good intentions of being able to use it to wrap that 
> asinine CreateProcess() thing that Win32 has...

There is only one place that I am aware of, where this is not enough: For the shallow thing, upload-pack fork()s but does not execute another program, but lists all the objects to be uploaded. (For shallow, this _cannot_ be an external process, since it mucks with the commits which are to be treated as shallow).

Ciao, Dscho