Re: [PATCH] pkt-line: re-'static'-ify buffer in packet_write_fmt_1()

> On 27 Aug 2017, at 09:37, Martin Ågren <martin.agren@gmail.com> wrote:
> 
> The static-ness was silently dropped in commit 70428d1a5 ("pkt-line: add
> packet_write_fmt_gently()", 2016-10-16). As a result, for each call to
> packet_write_fmt_1, we allocate and leak a buffer.

> We could keep the strbuf non-static and instead make sure we always
> release it before returning (but not before we die, so that we don't
> touch errno). That would also prepare us for threaded use. But until
> that needs to happen, let's just restore the static-ness so that we get
> back to a situation where we (eventually) do not continuosly keep
> allocating memory.
> 
> Signed-off-by: Martin Ågren <martin.agren@gmail.com>
> ---
> I waffled between "fixing the memory leak" by releasing the buffer and
> "fixing the static-ness" as in this patch. I can see arguments both ways
> and don't have any strong opinion.
> 
> pkt-line.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/pkt-line.c b/pkt-line.c
> index 7db911957..f364944b9 100644
> --- a/pkt-line.c
> +++ b/pkt-line.c
> @@ -136,9 +136,10 @@ static void format_packet(struct strbuf *out, const char *fmt, va_list args)
> static int packet_write_fmt_1(int fd, int gently,
> 			      const char *fmt, va_list args)
> {
> -	struct strbuf buf = STRBUF_INIT;
> +	static struct strbuf buf = STRBUF_INIT;
> 	ssize_t count;
> 
> +	strbuf_reset(&buf);
> 	format_packet(&buf, fmt, args);
> 	count = write_in_full(fd, buf.buf, buf.len);
> 	if (count == buf.len)
> -- 
> 2.14.1.151.g45c1275a3.dirty