Re: [PATCH v2] line-range: Fix infinite loop bug with degenerate regex
- From
SZEDER Gábor <szeder.dev@gmail.com>
- Date
- Dec 7, 2022, 20:30 UTC
- Message-ID
- <20221207203047.GA2996@szeder.dev>
- In-Reply-To
- <221207.86tu27aju3.gmgdl@evledraar.gmail.com>
On Wed, Dec 07, 2022 at 05:52:04AM +0100, Ævar Arnfjörð Bjarmason wrote:
Show 52 quoted lines
>
> 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.