Skip to content

Commit 911ec99

Browse files
peffgitster
authored andcommitted
run-command: introduce capture_command helper
Something as simple as reading the stdout from a command turns out to be rather hard to do right. Doing: cmd.out = -1; run_command(&cmd); strbuf_read(&buf, cmd.out, 0); can result in deadlock if the child process produces a large amount of output. What happens is: 1. The parent spawns the child with its stdout connected to a pipe, of which the parent is the sole reader. 2. The parent calls wait(), blocking until the child exits. 3. The child writes to stdout. If it writes more data than the OS pipe buffer can hold, the write() call will block. This is a deadlock; the parent is waiting for the child to exit, and the child is waiting for the parent to call read(). So we might try instead: start_command(&cmd); strbuf_read(&buf, cmd.out, 0); finish_command(&cmd); But that is not quite right either. We are examining cmd.out and running finish_command whether start_command succeeded or not, which is wrong. Moreover, these snippets do not do any error handling. If our read() fails, we must make sure to still call finish_command (to reap the child process). And both snippets failed to close the cmd.out descriptor, which they must do (provided start_command succeeded). Let's introduce a run-command helper that can make this a bit simpler for callers to get right. Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent d56d966 commit 911ec99

File tree

2 files changed

+29
-0
lines changed

2 files changed

+29
-0
lines changed

run-command.c

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -833,3 +833,19 @@ int run_hook_le(const char *const *env, const char *name, ...)
833833

834834
return ret;
835835
}
836+
837+
int capture_command(struct child_process *cmd, struct strbuf *buf, size_t hint)
838+
{
839+
cmd->out = -1;
840+
if (start_command(cmd) < 0)
841+
return -1;
842+
843+
if (strbuf_read(buf, cmd->out, hint) < 0) {
844+
close(cmd->out);
845+
finish_command(cmd); /* throw away exit code */
846+
return -1;
847+
}
848+
849+
close(cmd->out);
850+
return finish_command(cmd);
851+
}

run-command.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,19 @@ int run_command_v_opt(const char **argv, int opt);
7171
*/
7272
int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const char *const *env);
7373

74+
/**
75+
* Execute the given command, capturing its stdout in the given strbuf.
76+
* Returns -1 if starting the command fails or reading fails, and otherwise
77+
* returns the exit code of the command. The output collected in the
78+
* buffer is kept even if the command returns a non-zero exit. The hint field
79+
* gives a starting size for the strbuf allocation.
80+
*
81+
* The fields of "cmd" should be set up as they would for a normal run_command
82+
* invocation. But note that there is no need to set cmd->out; the function
83+
* sets it up for the caller.
84+
*/
85+
int capture_command(struct child_process *cmd, struct strbuf *buf, size_t hint);
86+
7487
/*
7588
* The purpose of the following functions is to feed a pipe by running
7689
* a function asynchronously and providing output that the caller reads.

0 commit comments

Comments
 (0)