From: Junio C Hamano <gitster@pobox.com>
Date: Sun, 22 Mar 2015 22:54:29 GMT
Subject: Re: [PATCH 3/7] strbuf: introduce strbuf_read_cmd helper
Message-ID: <xmqqwq28bq3e.fsf@gitster.dls.corp.google.com>
In-Reply-To: <CAPig+cR5Ur4xOKZ6K=bOwOVM8bHHjJJXHxzCbvYBhqOTtD6dXg@mail.gmail.com>

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Sun, Mar 22, 2015 at 6:07 AM, Jeff King <peff@peff.net> wrote:
>> 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()

Sound like a good suggestion (but I haven't read the users of the
proposed function, after doing which I might change my mind---I'll
see).

Thanks.