Am 28.12.22 um 19:00 schrieb Ævar Arnfjörð Bjarmason:
> When the "header_list" struct member was added in [1] it wasn't made
> to free the list using loop added for the adjacent "pattern_list"
> member, see [2] for when we started freeing it.
>
> Let's start doing o by splitting up the loop in free_grep_patterns()
> into a utility function. This makes e.g. this command leak-free when
> run on git.git:
>
> ./git -P log -1 --color=always --author=A origin/master
>
> 1. 80235ba79ef ("log --author=me --grep=it" should find intersection,
> not union, 2010-01-17)
> 2. b48fb5b6a95 (grep: free expressions and patterns when done.,
> 2006-09-27)
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
> ---
> grep.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/grep.c b/grep.c
> index ca75514f8f6..c908535e0d8 100644
> --- a/grep.c
> +++ b/grep.c
> @@ -769,11 +769,11 @@ static void free_pattern_expr(struct grep_expr *x)
> free(x);
> }
>
> -void free_grep_patterns(struct grep_opt *opt)
> +static void free_grep_pat(struct grep_pat *pattern)
> {
> struct grep_pat *p, *n;
>
> - for (p = opt->pattern_list; p; p = n) {
> + for (p = pattern; p; p = n) {
> n = p->next;
> switch (p->token) {
> case GREP_PATTERN: /* atom */
> @@ -790,6 +790,12 @@ void free_grep_patterns(struct grep_opt *opt)
> }
> free(p);
> }
> +}
> +
> +void free_grep_patterns(struct grep_opt *opt)
> +{
> + free_grep_pat(opt->pattern_list);
> + free_grep_pat(opt->header_list);
OK. Patch 19 didn't help me understand this one, though, contrary to
the foreshadowing in its commit message. It seems quite unrelated, in
fact. It's a stylistic improvement, to be sure, but I expected more
structural relevance.
Factoring out free_grep_pat() first and then using it for header_list
in a separate patch would have made a difference by making the actual
leak fix being a one-liner.
Not necessarily worth a re-roll, though -- the patch is understandable
as-is, but it probably took me a moment longer with refactoring and
fix blended together like this.
>
> if (opt->pattern_expression)
> free_pattern_expr(opt->pattern_expression);
