diff options
| author | Ævar Arnfjörð Bjarmason <avarab@gmail.com> | 2022-11-08 19:17:51 +0100 |
|---|---|---|
| committer | Junio C Hamano <gitster@pobox.com> | 2022-11-21 12:32:48 +0900 |
| commit | ac95f5d36adb42980064587fd7910fa275ff853e (patch) | |
| tree | 9971f5e482dea36993d32224666996ee514c2a71 /builtin/bugreport.c | |
| parent | 603f2f5719f8a85996d5f20cf0e56186c149b923 (diff) | |
| download | git-ac95f5d36adb42980064587fd7910fa275ff853e.tar.gz | |
built-ins: use free() not UNLEAK() if trivial, rm dead code
For a lot of uses of UNLEAK() it would be quite tricky to release the
memory involved, or we're missing the relevant *_(release|clear)()
functions. But in these cases we have them already, and can just
invoke them on the variable(s) involved, instead of UNLEAK().
For "builtin/worktree.c" the UNLEAK() was also added in [1], but the
struct member it's unleaking was removed in [2]. The only non-"int"
member of that structure is "const char *keep_locked", which comes to
us via "argv" or a string literal[3].
We have good visibility via the compiler and
tooling (e.g. SANITIZE=address) on bad free()-ing, but none on
UNLEAK() we don't need anymore. So let's prefer releasing the memory
when it's easy.
For "bugreport", "worktree" and "config" we need to start using a "ret
= ..." return pattern. For "builtin/bugreport.c" these UNLEAK() were
added in [4], and for "builtin/config.c" in [1].
For "config" the code seen here was the only user of the "value"
variable. For "ACTION_{RENAME,REMOVE}_SECTION" we need to be sure to
return the right exit code in the cases where we were relying on
falling through to the top-level.
I think there's still a use-case for UNLEAK(), but hat it's changed
since then. Using it so that "we can see the real leaks" is
counter-productive in these cases.
It's more useful to have UNLEAK() be a marker of the remaining odd
cases where it's hard to free() the memory for whatever reason. With
this change less than 20 of them remain in-tree.
1. 0e5bba53af7 (add UNLEAK annotation for reducing leak false
positives, 2017-09-08)
2. d861d34a6ed (worktree: remove extra members from struct add_opts,
2018-04-24)
3. 0db4961c49b (worktree: teach `add` to accept --reason <string> with
--lock, 2021-07-15)
4. 0e5bba53af7 and 00d8c311050 (commit: fix "author_ident" leak,
2022-05-12).
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Diffstat (limited to 'builtin/bugreport.c')
| -rw-r--r-- | builtin/bugreport.c | 9 |
1 files changed, 6 insertions, 3 deletions
diff --git a/builtin/bugreport.c b/builtin/bugreport.c index 96052541cb..5bc254be80 100644 --- a/builtin/bugreport.c +++ b/builtin/bugreport.c @@ -106,6 +106,7 @@ int cmd_bugreport(int argc, const char **argv, const char *prefix) const char *user_relative_path = NULL; char *prefixed_filename; size_t output_path_len; + int ret; const struct option bugreport_options[] = { OPT_CALLBACK_F(0, "diagnose", &diagnose, N_("mode"), @@ -182,7 +183,9 @@ int cmd_bugreport(int argc, const char **argv, const char *prefix) user_relative_path); free(prefixed_filename); - UNLEAK(buffer); - UNLEAK(report_path); - return !!launch_editor(report_path.buf, NULL, NULL); + strbuf_release(&buffer); + + ret = !!launch_editor(report_path.buf, NULL, NULL); + strbuf_release(&report_path); + return ret; } |