[PATCH v2 0/6] apply: fix leaking buffer of `struct image`

[Patrick Steinhardt <ps@pks.im>]

Hi,

this is the second version of my patch series that refactors lifecycle
management of `struct image` in "apply.c" to plug a bunch of memory
leaks.

Changes compared to v1:
 
  - Fix two typos.

  - Correct the statement that we don't loop around
    `image_remove_first_line()`. No idea how I missed that loop.

  - Split up an overly complex line into multiple lines.

Thanks!

Patrick

Patrick Steinhardt (6):
  apply: reorder functions to move image-related things together
  apply: rename functions operating on `struct image`
  apply: introduce macro and function to init images
  apply: refactor code to drop `line_allocated`
  apply: rename members that track line count and allocation length
  apply: refactor `struct image` to use a `struct strbuf`

 apply.c                            | 449 +++++++++++++----------------
 t/t3436-rebase-more-options.sh     |   1 +
 t/t4107-apply-ignore-whitespace.sh |   4 +-
 t/t4124-apply-ws-rule.sh           |   1 +
 t/t4125-apply-ws-fuzz.sh           |   1 +
 t/t4138-apply-ws-expansion.sh      |   1 +
 6 files changed, 206 insertions(+), 251 deletions(-)

Range-diff against v1:
1:  7b6903ecdd = 1:  a713a7aef0 apply: reorder functions to move image-related things together
2:  3f188412f6 = 2:  be8f98881f apply: rename functions operating on `struct image`
3:  1b49e39bcd = 3:  506c787d68 apply: introduce macro and function to init images
4:  0427cb7250 ! 4:  6ac37186f2 apply: refactor code to drop `line_allocated`
    @@ Commit message
         apply: refactor code to drop `line_allocated`
     
         The `struct image` has two members `line` and `line_allocated`. The
    -    former member is the one that should be used throughougt the code,
    +    former member is the one that should be used throughout the code,
         whereas the latter one is used to track whether the lines have been
         allocated or not.
     
         In practice, the array of lines is always allocated. The reason why we
         have `line_allocated` is that `remove_first_line()` will advance the
    -    array pointer to drop the first entry, and thus it point into the array
    +    array pointer to drop the first entry, and thus it points into the array
         instead of to the array header.
     
         Refactor the function to use memmove(3P) instead, which allows us to get
    -    rid of this double bookkeeping. We call this function at most once per
    -    image anyway, so this shouldn't cause any performance regressions.
    +    rid of this double bookkeeping. This is less efficient, but I doubt that
    +    this matters much in practice. If this judgement call is found to be
    +    wrong at a later point in time we can likely refactor the surrounding
    +    loop such that we first calculate the number of leading context lines to
    +    remove and then remove them in a single call to memmove(3P).
     
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
5:  e35816ed56 = 5:  5497708428 apply: rename members that track line count and allocation length
6:  6cf45daf84 ! 6:  42880dcf04 apply: refactor `struct image` to use a `struct strbuf`
    @@ apply.c: static void image_remove_first_line(struct image *img)
      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);
    ++	size_t last_line_len = img->line[img->line_nr - 1].len;
    ++	strbuf_setlen(&img->buf, img->buf.len - last_line_len);
    ++	img->line_nr--;
      }
      
      /* fmt must contain _one_ %s and no other substitution */

base-commit: ed155187b429a2a6b6475efe1767053df37ccfe1
-- 
2.46.0.551.gc5ee8f2d1c.dirty