Re: [PATCH 4/7] run-command: add pipe_command helper

[Eric Sunshine <sunshine@sunshineco.com>]

On Thu, Jun 16, 2016 at 5:37 AM, Jeff King <peff@peff.net> wrote:
> We already have capture_command(), which captures the stdout
> of a command in a way that avoids deadlocks. But sometimes
> we need to do more I/O, like capturing stderr as well, or
> sending data to stdin. It's easy to write code that
> deadlocks racily in these situations depending on how fast
> the command reads its input, or in which order it writes its
> output.
>
> Let's give callers an easy interface for doing this the
> right way, similar to what capture_command() did for the
> simple case.
>
> The whole thing is backed by a generic poll() loop that can
> feed an arbitrary number of buffers to descriptors, and fill
> an arbitrary number of strbufs from other descriptors. This
> seems like overkill, but the resulting code is actually a
> bit cleaner than just handling the three descriptors
> (because the output code for stdout/stderr is effectively
> duplicated, so being able to loop is a benefit).
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> diff --git a/run-command.h b/run-command.h
> @@ -79,17 +79,34 @@ int run_command_v_opt(const char **argv, int opt);
>  /**
> - * Execute the given command, capturing its stdout in the given strbuf.
> + * Execute the given command, sending "in" to its stdin, and capturing its
> + * stdout and stderr in the "out" and "err" strbufs. Any of the three may
> + * be NULL to skip processing.
> + *
>   * Returns -1 if starting the command fails or reading fails, and otherwise
> - * returns the exit code of the command. The output collected in the
> - * buffer is kept even if the command returns a non-zero exit. The hint field
> - * gives a starting size for the strbuf allocation.
> + * returns the exit code of the command. Any output collected in the

Did you mean s/returns/Returns/ ?

> + * buffers is kept even if the command returns a non-zero exit. The hint fields
> + * gives starting sizes for the strbuf allocations.
>   *
>   * The fields of "cmd" should be set up as they would for a normal run_command
> - * invocation. But note that there is no need to set cmd->out; the function
> - * sets it up for the caller.
> + * invocation. But note that there is no need to set the in, out, or err
> + * fields; capture_command handles that automatically.

s/capture_command/pipe_command/

> + */
> +int pipe_command(struct child_process *cmd,
> +                const char *in, size_t in_len,
> +                struct strbuf *out, size_t out_hint,
> +                struct strbuf *err, size_t err_hint);
> +