Re: [PATCH v2 2/3] diff: add -I<regex> that ignores matching changes

From: Michał Kępień <michal@isc.org>
Date: Oct 13, 2020, 06:36 UTC
Message-ID: <20201013063646.GB3278@larwa.hq.kempniu.pl>
In-Reply-To: <nycvar.QRO.7.76.6.2010121315170.50@tvgsbejvaqbjf.bet>

Hi Johannes,

Show 12 quoted lines

> > @@ -5491,6 +5511,9 @@ static void prep_parse_options(struct diff_options *options)
> >  		OPT_BIT_F(0, "ignore-blank-lines", &options->xdl_opts,
> >  			  N_("ignore changes whose lines are all blank"),
> >  			  XDF_IGNORE_BLANK_LINES, PARSE_OPT_NONEG),
> > +		OPT_CALLBACK_F('I', NULL, options, N_("<regex>"),
> > +			       N_("ignore changes whose all lines match <regex>"),
> > +			       0, diff_opt_ignore_regex),
> >  		OPT_BIT(0, "indent-heuristic", &options->xdl_opts,
> >  			N_("heuristic to shift diff hunk boundaries for easy reading"),
> >  			XDF_INDENT_HEURISTIC),
> 
> Are we releasing the `ignore_regex` anywhere?

Oops, I tried to mimic what is done for 'anchors', and I failed to notice that apparently the elements of the options->anchors array are only free()'d when --patience is also used and the array pointer itself is never free()'d at all. Given this, I believe I need to fix diff_opt_ignore_regex() in patch 2 and also make sure that the memory allocated in diff_opt_anchored() gets properly released - in another preliminary clean-up patch?

At first glance, diff_flush() - specifically the part below the 'free_queue' label - looks like a sane place to free() things. Am I mistaken?

-- 
Best regards,
Michał Kępień