Re: [PATCH v2] line-range: Fix infinite loop bug with degenerate regex

["SZEDER Gábor" <szeder.dev@gmail.com>]

On Wed, Dec 07, 2022 at 05:52:04AM +0100, Ævar Arnfjörð Bjarmason wrote:
> 
> On Mon, Dec 05 2022, Lars Kellogg-Stedman wrote:
> 
> > When the -L argument to "git log" is passed the degenerate regular
> > expression "$" (as in "-L :$:line-range.c"), this results in an
> > infinite loop in find_funcname_matching_regexp() (the function
> > iterates through the file correctly, but when it reaches the end of
> > the file it matches $ against the empty string, "", and at that points
> > loops forever).
> >
> > Modify the loop condition from while (1) to while (*start) so that the
> > loop exits when start is the empty string. In this case, "git log" exits
> > with the error:
> >
> >     fatal: -L parameter '$' starting at line 1: no match
> >
> > Originally reported in <https://stackoverflow.com/q/74690545/147356>.
> >
> > Signed-off-by: Lars Kellogg-Stedman <lars@oddbit.com>
> > ---
> >  line-range.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/line-range.c b/line-range.c
> > index 955a8a9535..bdcb810485 100644
> > --- a/line-range.c
> > +++ b/line-range.c
> > @@ -135,7 +135,7 @@ static const char *find_funcname_matching_regexp(xdemitconf_t *xecfg, const char
> >  {
> >  	int reg_error;
> >  	regmatch_t match[1];
> > -	while (1) {
> > +	while (*start) {
> >  		const char *bol, *eol;
> >  		reg_error = regexec(regexp, start, 1, match, 0);
> >  		if (reg_error == REG_NOMATCH)
> > @@ -161,6 +161,8 @@ static const char *find_funcname_matching_regexp(xdemitconf_t *xecfg, const char
> >  			return bol;
> >  		start = eol;
> >  	}
> > +
> > +    return NULL;
> >  }
> >  
> >  static const char *parse_range_funcname(
> 
> We really should fix this, but why not just count this as a match,
> rather than erroring out?
> 
> That we're mixing up whether '$' always matches here with our iteration
> loop is our own internal bug, we shouldn't error out on a '$'. It's just
> a regex that happens to match everything.

Indeed.  The description of '-L...' in the manpage of 'git log' (and
'blame') states:

    If :<funcname> is given in place of <start> and <end>, it is a
    regular expression that denotes the range from the first funcname
    line that matches <funcname>, up to the next funcname line.

So, if we use '-L' with a funcname regex that matches every line, be
it '$', '^' or '.*', then this would mean that it should denote the
range from the first funcname line in that file to the second.