On Mon, Dec 5, 2022 at 2:45 PM Lars Kellogg-Stedman <lars@xxxxxxxxxx> wrote: > line-range: Fix infinite loop bug with degenerate regex s/Fix/fix/ > 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). s/points/point/ > 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 I've never really used `-L:funcname:`, so it took me by surprise that this would be reported as a fatal error. However, now that I've read through the code and put a bit of thought into degenerate cases, I can't come up with any better way to report it. Okay. > Originally reported in <https://stackoverflow.com/q/74690545/147356>. > > Signed-off-by: Lars Kellogg-Stedman <lars@xxxxxxxxxx> > --- > diff --git 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; > } I'm not particularly familiar with this code (even though I touched it years ago), so I can't say whether this is the best fix, but it seems reasonable now that I've studied the code and put some thought into degenerate cases. I had wondered if it would be possible to catch these degenerate cases in some general way, but whatever ideas I had seemed too special-purpose; your solution here is straightforward and probably more robust than special-purpose code. Style-wise, I'd probably drop the blank line before `return NULL`. Please do re-roll once more with a test which verifies that this bug has been fixed. Here's a test you can add to the bottom of t/t4211-line-log.sh for that purpose (be sure to re-indent it with TABs which Gmail mushes out): test_expect_success 'degenerate -L:$: does not hang' ' >content-immaterial && git add content-immaterial && git commit -m immaterial && test_must_fail git log -L:$:content-immaterial ' And here's my sign-off which you can insert above your own sign-off if you include the above test. Signed-off-by: Eric Sunshine <sunshine@xxxxxxxxxxxxxx>
