Re: [PATCH v5 17/19] grep API: plug memory leaks by freeing "header_list"

From: Elijah Newren <newren@gmail.com>
Date: Jan 26, 2023, 02:33 UTC
Message-ID: <CABPp-BFtR81jdjnEMKMaFnD71hdmCfgoQMSRuAEk8oMb3_AYFw@mail.gmail.com>
In-Reply-To: <patch-v5-17.19-6a8f4a567aa-20230118T120334Z-avarab@gmail.com>

I had the same reaction to this patch that Rene did on v1. The commit message of the previous patch should probably be reworked so as to not mislead.

On Wed, Jan 18, 2023 at 5:15 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 13 quoted lines

>
> 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.
>
> 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)

That paragraph is really hard to parse for me. Maybe change the above to something like

""" When the "header_list" struct member was added in [1], freeing this field was neglected. Fix that now, so that commands like

    ./git -P log -1 --color=always --author=A origin/master

will run leak-free.

1. 80235ba79ef ("log --author=me --grep=it" should find intersection,
    not union, 2010-01-17)
"""

Show 19 quoted lines

> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  grep.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/grep.c b/grep.c
> index a4450df4559..c908535e0d8 100644
> --- a/grep.c
> +++ b/grep.c
> @@ -795,6 +795,7 @@ static void free_grep_pat(struct grep_pat *pattern)
>  void free_grep_patterns(struct grep_opt *opt)
>  {
>         free_grep_pat(opt->pattern_list);
> +       free_grep_pat(opt->header_list);
>
>         if (opt->pattern_expression)
>                 free_pattern_expr(opt->pattern_expression);
> --
> 2.39.0.1225.g30a3d88132d

Actual code change looks good.