| Message ID | patch-20.20-8ecc68c3e93-20221228T175512Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | leak fixes: various simple leak fixes | expand |
On Wed, Dec 28, 2022 at 1:03 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > 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: s/o by/so by/ > ./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@gmail.com>
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@gmail.com> > --- > 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);
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); if (opt->pattern_expression) free_pattern_expr(opt->pattern_expression);
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@gmail.com> --- grep.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)