git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v4] line-range: fix infinite loop bug with '$' regex

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Dec 19, 2022, 22:55 UTC
Message-ID
<221219.86edsvxank.gmgdl@evledraar.gmail.com>
In-Reply-To
<20221219224850.2703967-1-lars@oddbit.com>
On Mon, Dec 19 2022, Lars Kellogg-Stedman wrote:
Show 84 quoted lines
> When the -L argument to "git log" is passed the zero-width regular
> expression "$" (as in "-L :$:line-range.c"), this results in an
> infinite loop in find_funcname_matching_regexp().
>
> Modify find_funcname_matching_regexp to correctly match the entire line
> instead of the zero-width match at eol and update the loop condition to
> prevent an infinite loop in the event of other undiscovered corner cases.
>
> The primary change is that we pre-decrement the beginning-of-line marker
> ('bol') before comparing it to '\n'. In the case of '$', where we match the
> '\n' at the end of the line and start the loop with bol == eol, this
> ensures that bol will find the beginning of the line on which the match
> occurred.
>
> Originally reported in <https://stackoverflow.com/q/74690545/147356>.
>
> Signed-off-by: Lars Kellogg-Stedman <lars@oddbit.com>
> ---
>  line-range.c        |  7 ++++---
>  t/t4211-line-log.sh | 22 ++++++++++++++++++++++
>  2 files changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/line-range.c b/line-range.c
> index 955a8a9535..47bf0d6f1a 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)
> @@ -148,8 +148,8 @@ static const char *find_funcname_matching_regexp(xdemitconf_t *xecfg, const char
>  		/* determine extent of line matched */
>  		bol = start+match[0].rm_so;
>  		eol = start+match[0].rm_eo;
> -		while (bol > start && *bol != '\n')
> -			bol--;
> +		while (bol > start && *--bol != '\n')
> +			; /* nothing */
>  		if (*bol == '\n')
>  			bol++;
>  		while (*eol && *eol != '\n')
> @@ -161,6 +161,7 @@ static const char *find_funcname_matching_regexp(xdemitconf_t *xecfg, const char
>  			return bol;
>  		start = eol;
>  	}
> +	return NULL;
>  }
>  
>  static const char *parse_range_funcname(
> diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh
> index ac9e4d0928..c6540e822f 100755
> --- a/t/t4211-line-log.sh
> +++ b/t/t4211-line-log.sh
> @@ -315,4 +315,26 @@ test_expect_success 'line-log with --before' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'setup tests for zero-width regular expressions' '
> +	cat >expect <<-EOF
> +	Modify func1() in file.c
> +	Add func1() and func2() in file.c
> +	EOF
> +'
> +
> +test_expect_success 'zero-width regex $ matches any function name' '
> +	git log --format="%s" --no-patch "-L:$:file.c" >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'zero-width regex ^ matches any function name' '
> +	git log --format="%s" --no-patch "-L:^:file.c" >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'zero-width regex .* matches any function name' '
> +	git log --format="%s" --no-patch "-L:.*:file.c" >actual &&
> +	test_cmp expect actual
> +'
> +
Untested, but as most of this is repeated & would fail if the "setup"
test is skipped, a one-off function would be better, e.g.:
	
	test_zerowidth_regex () {
		local rx="$1" &&
		
		test_expect_success "zero-width regex '$rx' matches any function name" '
			cat >expect <<-\EOF &&
			Modify func1() in file.c
			Add func1() and func2() in file.c
			EOF
			git log --format="%s" --no-patch -L:"$rx":file.c >actual &&
			test_cmp expect actual
		'
	}
	
	test_zerowidth_regex '$'
	test_zerowidth_regex '^'
	test_zerowidth_regex '.*'

The change of direction for the fix itself looks good to me (re my earlier feedback on a previous round), i.e. that we should fix our own code, not forbid '$' in regexes.

Previous: Lars Kellogg-StedmanNext: Eric Sunshine
Message 13 of 15 in “line-range: Fix infinite loop bug with degenerate regex”
  1. line-range: Fix infinite loop bug with degenerate regexLars Kellogg-Stedman, Dec 5, 2022
  2. Eric SunshineDec 7, 2022
  3. Ævar Arnfjörð BjarmasonDec 7, 2022
  4. Junio C HamanoDec 7, 2022
  5. SZEDER GáborDec 7, 2022
  6. Lars Kellogg-StedmanDec 9, 2022
  7. line-range: Fix infinite loop bug with degenerate '$' regexLars Kellogg-Stedman, Dec 11, 2022
  8. Junio C HamanoDec 11, 2022
  9. Junio C HamanoDec 11, 2022
  10. Lars Kellogg-StedmanDec 14, 2022
  11. Junio C HamanoDec 18, 2022
  12. line-range: fix infinite loop bug with '$' regexLars Kellogg-Stedman, Dec 19, 2022
  13. Ævar Arnfjörð BjarmasonDec 19, 2022
  14. Eric SunshineDec 20, 2022
  15. Junio C HamanoDec 20, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.