Subject: Re: [PATCH 6/6] apply: refactor `struct image` to use a `struct strbuf`

From: Junio C Hamano <[email-masked]>
Date: Mon, 16 Sep 2024 12:30:37 -0700

Patrick Steinhardt <[email-masked]> writes:

> Refactor the code to use a `struct strbuf` instead, addressing all of
> the above. Like this we can easily perform in-place updates in all
> situations, the logic to perform those updates becomes way simpler and
> the lifetime of the buffer becomes a ton easier to track.
>
> This refactoring also plugs some leaking buffers as a side effect.

Nice.

In short, the leaks were in the original code where it was making
direct assignment to image->buf, and we now use strbuf_attach(),
which releases the current buffer before replacing it with a piece
of memory allocated outside the control of strbuf API?

>  static void image_remove_last_line(struct image *img)
>  {
> -	img->len -= img->line[--img->line_nr].len;
> +	strbuf_setlen(&img->buf, img->buf.len - img->line[--img->line_nr].len);
>  }

I feel that this, while technically is faithful to the original, got
a bit too complex to understand what is going on.  Perhaps split it
into two statements with an intermediate variable?  I dunno.

Thanks.