git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 3/7] strbuf: introduce strbuf_read_cmd helper

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Mar 22, 2015, 19:36 UTC
Message-ID
<CAPig+cR5Ur4xOKZ6K=bOwOVM8bHHjJJXHxzCbvYBhqOTtD6dXg@mail.gmail.com>
In-Reply-To
<20150322100724.GC11615@peff.net>
On Sun, Mar 22, 2015 at 6:07 AM, Jeff King <peff@peff.net> wrote:
Show 18 quoted lines
> Something as simple as reading the stdout from a command
> turns out to be rather hard to do right. Doing:
>
>   if (!run_command(&cmd))
>         strbuf_read(&buf, cmd.out, 0);
>
> can result in deadlock if the child process produces a large
> amount of output. [...]
>
> Let's introduce a strbuf helper that can make this a bit
> simpler for callers to do right.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> This is really at the intersection of the strbuf and
> run-command APIs, so you could argue for it being part of
> either It is logically quite like the strbuf_read_file()
> function, so I put it there.

It does feel like a layering violation. If moved to the run-command API, it could given one of the following names or something better:

    run_command_capture()
    capture_command()
    command_capture()
    run_command_with_output()
    capture_output()
Show 30 quoted lines
> diff --git a/strbuf.h b/strbuf.h
> index 1883494..93a50da 100644
> --- a/strbuf.h
> +++ b/strbuf.h
> @@ -1,6 +1,8 @@
>  #ifndef STRBUF_H
>  #define STRBUF_H
>
> +struct child_process;
> +
>  /**
>   * strbuf's are meant to be used with all the usual C string and memory
>   * APIs. Given that the length of the buffer is known, it's often better to
> @@ -373,6 +375,14 @@ extern int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint);
>  extern int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint);
>
>  /**
> + * Execute the given command, capturing its stdout in the given strbuf.
> + * 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.
> + */
> +int strbuf_read_cmd(struct strbuf *sb, struct child_process *cmd, size_t hint);
> +
> +/**
>   * Read a line from a FILE *, overwriting the existing contents
>   * of the strbuf. The second argument specifies the line
>   * terminator character, typically `'\n'`.
> --
> 2.3.3.618.ga041503
Previous: Jeff KingNext: Junio C Hamano
Message 8 of 26 in “status hangs trying to get submodule summary”
  1. Wincent ColaiutaMar 22, 2015
  2. status: read submodule process output before calling wait()Jeff King, Mar 22, 2015
  3. Jeff KingMar 22, 2015
  4. 0/7 introduce strbuf_read_cmd to avoid deadlocksJeff King, Mar 22, 2015
  5. 1/7 wt-status: don't flush before running "submodule status"Jeff King, Mar 22, 2015
  6. 2/7 wt_status: fix signedness mismatch in strbuf_read callJeff King, Mar 22, 2015
  7. 3/7 strbuf: introduce strbuf_read_cmd helperJeff King, Mar 22, 2015
  8. Eric SunshineMar 22, 2015
  9. Junio C HamanoMar 22, 2015
  10. Junio C HamanoMar 22, 2015
  11. 0/7 introduce capture_command to avoid deadlocksJeff King, Mar 23, 2015
  12. 1/7 wt-status: don't flush before running "submodule status"Jeff King, Mar 23, 2015
  13. 2/7 wt_status: fix signedness mismatch in strbuf_read callJeff King, Mar 23, 2015
  14. 3/7 run-command: introduce capture_command helperJeff King, Mar 23, 2015
  15. 4/7 wt-status: use capture_commandJeff King, Mar 23, 2015
  16. 5/7 submodule: use capture_commandJeff King, Mar 23, 2015
  17. 6/7 trailer: use capture_commandJeff King, Mar 23, 2015
  18. 7/7 run-command: forbid using run_command with piped outputJeff King, Mar 23, 2015
  19. Junio C HamanoMar 23, 2015
  20. Jeff KingMar 22, 2015
  21. Junio C HamanoMar 22, 2015
  22. Jeff KingMar 22, 2015
  23. 4/7 wt-status: use strbuf_read_cmdJeff King, Mar 22, 2015
  24. 5/7 submodule: use strbuf_read_cmdJeff King, Mar 22, 2015
  25. 6/7 trailer: use strbuf_read_cmdJeff King, Mar 22, 2015
  26. 7/7 run-command: forbid using run_command with piped outputJeff King, Mar 22, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.