[RFC PATCH 0/5] strvec: add a "nodup" mode, fix memory leaks

From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "René Scharfe" <l.s.r@web.de>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [RFC PATCH 0/5] strvec: add a "nodup" mode, fix memory leaks
Date: Thu, 15 Dec 2022 10:11:06 +0100	[thread overview]
Message-ID: <RFC-cover-0.5-00000000000-20221215T090226Z-avarab@gmail.com> (raw)
In-Reply-To: <221214.86ilie48cv.gmgdl@evledraar.gmail.com>

This is an alternative to René's [1], his already fixes a leak in "git
am", and this could be done later, so I'm submitting it as RFC, but it
could also replace it.

I think as this series shows extending the "strvec" API to get a
feature that works like the existing "strdup_strings" that the "struct
string_list" has can make memory management much simpler.

The 4/5 here shows cases where we were leaking because our "v" was
clobbered, but where all the strings we were pushing to the "strvec"
were fixed strings, so we could skip xstrdup()-ing them.

The 5/5 then shows more complex cases where we have mixed-use,
i.e. most strings are fixed, but some are not. For those we use a
"struct string_list to_free = STRING_LIST_INIT_DUP", which we then
push to the "to_free" list with "string_list_append_nodup()".

This does make the API slightly more dangerous to use, as it's no
longer guaranteed that it owns all the members it points to. But as
the "struct string_list" has shown this isn't an issue in practice,
and e.g. SANITIZE=address et al are good about finding double-frees,
or frees of fixed strings.

A branch & CI for this are found at [2].

1. https://lore.kernel.org/git/baf93e4a-7f05-857c-e551-09675496c03c@web.de/
2. https://github.com/avar/git/tree/avar/add-and-use-strvec-init-nodup

Ævar Arnfjörð Bjarmason (5):
  builtin/annotate.c: simplify for strvec API
  various: add missing strvec_clear()
  strvec API: add a "STRVEC_INIT_NODUP"
  strvec API users: fix leaks by using "STRVEC_INIT_NODUP"
  strvec API users: fix more leaks by using "STRVEC_INIT_NODUP"

 builtin/am.c                  |  2 +-
 builtin/annotate.c            | 17 ++++++++---------
 builtin/describe.c            | 28 +++++++++++++++++++++-------
 builtin/stash.c               |  8 ++++++--
 builtin/upload-archive.c      | 16 ++++++++++++----
 strvec.c                      | 20 ++++++++++++++++++--
 strvec.h                      | 30 +++++++++++++++++++++++++++++-
 t/t0023-crlf-am.sh            |  1 +
 t/t4152-am-subjects.sh        |  2 ++
 t/t4254-am-corrupt.sh         |  2 ++
 t/t4256-am-format-flowed.sh   |  1 +
 t/t4257-am-interactive.sh     |  2 ++
 t/t5003-archive-zip.sh        |  1 +
 t/t5403-post-checkout-hook.sh |  1 +
 14 files changed, 105 insertions(+), 26 deletions(-)

-- 
2.39.0.rc2.1048.g0e5493b8d5b