Re: [PATCH 1/8] pkt-line: remove buffer arg from write_packetized_from_fd_no_flush()

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 4, 2021, 22:55 UTC
Message-ID: <xmqqh7lqcywi.fsf@gitster.c.googlers.com>
In-Reply-To: <d6ea7688dfb2536312b627f7ed47ff7f42091f60.1614889047.git.gitgitgadget@gmail.com>

"Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 16 quoted lines

> From: Jeff Hostetler <jeffhost@microsoft.com>
>
> Remove the scratch buffer argument from `write_packetized_from_fd_no_flush()`
> and allocate a temporary buffer within the function.
>
> In 3e4447f1ea (pkt-line: eliminate the need for static buffer in
> packet_write_gently(), 2021-02-13) we added the argument in order to
> get rid of a static buffer to make the routine safe in multi-threaded
> contexts and to avoid putting a very large buffer on the stack.  This
> delegated the problem to the caller who has more knowledge of the use
> case and can best decide the most efficient way to provide such a
> buffer.
>
> However, in practice, the (currently, only) caller just created the
> buffer on the stack, so we might as well just allocate it within the
> function and restore the original API.

Hmph, I would have expected, since we already have changed the callchain to pass the buffer down, that we'd keep the structure and update the caller to heap-allocate, but I think I like the result of this patch better. It's not like the caller can allocate and reuse a single buffer with repeated calls to the function; rather, the caller makes a single call that results in relaying many bytes by the helper, all done in a loop, and it is sufficient for the helper to allocate/deallocate outside of its loop to reuse the buffer.