Re: [PATCH 1/2] log: UNLEAK rev to silence a large number of leaks
- From
Carlo Marcelo Arenas Belón <carenas@gmail.com>
- Date
- Sep 18, 2021, 20:06 UTC
- Message-ID
- <YUZG0D5ayEWd7MLP@carlos-mbp.lan>
- In-Reply-To
- <6d54bc264e2f9ce519f32c0673167a00bab55573.1631972978.git.gitgitgadget@gmail.com>
My equivalent version for these fixes is obviously more verbose but IMHO not that ugly (and as safe)
It avoids the need to UNLEAK early by changing the program flow also for the early return so the cleanup could be centralized in one single function.
Both, the cmdline and mailmap arrays (and the objects they accumulate) are cleaned in a "reusable" way.
Note that the cleaning of the "name" in the cmdline item throws a warning as shown below which I intentionally didn't fix, as it would seem that either the use of const there or the need to strdup is wrong. So hope someone that knows this code better could chime in.
Carlo ------ >8 ------ Subject: [PATCH] builtin/log: leaks from `git show` in t0000
obviously not ready, since the following will need to be corrected:
revision.c:1496:8: warning: passing 'const char *' to parameter of type 'void *' discards qualifiers [-Wincompatible-pointer-types-discards-qualifiers]
free(info->rev[i].name);
^~~~~~~~~~~~~~~~~Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> --- builtin/log.c | 8 ++++++-- revision.c | 20 ++++++++++++++++++++ revision.h | 5 +++++ 3 files changed, 31 insertions(+), 2 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c index f75d87e8d7..1b1c1f53f4 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -645,8 +645,10 @@ int cmd_show(int argc, const char **argv, const char *prefix) opt.tweak = show_setup_revisions_tweak; cmd_log_init(argc, argv, prefix, &rev, &opt); - if (!rev.no_walk) - return cmd_log_walk(&rev); + if (!rev.no_walk) { + ret = cmd_log_walk(&rev); + goto done; + } count = rev.pending.nr; objects = rev.pending.objects; @@ -702,6 +704,8 @@ int cmd_show(int argc, const char **argv, const char *prefix) } } free(objects); +done: + repo_clear_revisions(&rev); return ret; } diff --git a/revision.c b/revision.c index 0dabb5a0bc..ce62192dd8 100644 --- a/revision.c +++ b/revision.c @@ -1487,6 +1487,18 @@ static void add_rev_cmdline(struct rev_info *revs, info->nr++; } +static void clear_rev_cmdline(struct rev_info *revs) +{ + struct rev_cmdline_info *info = &revs->cmdline; + size_t i, nr = info->nr; + + for (i = 0; i < nr; i++) + free(info->rev[i].name); + + FREE_AND_NULL(info->rev); + info->nr = info->alloc = 0; +} + static void add_rev_cmdline_list(struct rev_info *revs, struct commit_list *commit_list, int whence, @@ -1845,6 +1857,14 @@ void repo_init_revisions(struct repository *r, init_display_notes(&revs->notes_opt); } +void repo_clear_revisions(struct rev_info *revs) +{ + if (revs->mailmap) + clear_mailmap(revs->mailmap); + FREE_AND_NULL(revs->mailmap); + clear_rev_cmdline(revs); +} + static void add_pending_commit_list(struct rev_info *revs, struct commit_list *commit_list, unsigned int flags) diff --git a/revision.h b/revision.h index 0c65a760ee..f695c41cee 100644 --- a/revision.h +++ b/revision.h @@ -358,6 +358,11 @@ void repo_init_revisions(struct repository *r, struct rev_info *revs, const char *prefix); +/* + * Free all structures dynamically allocated for the provided rev_info + */ +void repo_clear_revisions(struct rev_info *revs); + /** * Parse revision information, filling in the `rev_info` structure, and * removing the used arguments from the argument list. Returns the number
-- 2.33.0.911.gbe391d4e11