threads / patch / 19629

patchgrep: fix colouring of matches with zero length

Subject: [PATCH] grep: fix colouring of matches with zero length

## tl;dr

3 messages between Jun 1, 2009 and Jun 1, 2009. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

René Scharfe· Jun 1, 2009, 16:46 UTC · lore

The first hunk fixes match colouring if the patterns is the empty string. Such a pattern matches the whole line, but fixmatch() sets the end offset to the same value as the start offset. Special case the empty pattern in match_one_pattern() and thus avoid calling fixmatch().

The second hunk of the patch makes sure that grep match colouring doesn't get stuck in an infinite loop if the length of a match is zero. The rest of the line is simply printed without colouring. Even though the empty pattern string doesn't result in empty matches any more, this defensive measure catches unknown cases.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 grep.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)
Show changes to grep.c +7 −1
diff --git a/grep.c b/grep.c
index cc6d5b0..745b3c0 100644
--- a/grep.c
+++ b/grep.c
@@ -324,7 +324,11 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,
 	}
 
  again:
-	if (p->fixed)
+	if (p->pattern[0] == '\0') {
+		hit = 1;
+		pmatch[0].rm_so = 0;
+		pmatch[0].rm_eo = eol - bol;
+	} else if (p->fixed)
 		hit = !fixmatch(p->pattern, bol, pmatch);
 	else
 		hit = !regexec(&p->regexp, bol, 1, pmatch, eflags);
@@ -500,6 +504,8 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 
 		*eol = '\0';
 		while (next_match(opt, bol, eol, ctx, &match, eflags)) {
+			if (match.rm_so == match.rm_eo)
+				break;
 			printf("%.*s%s%.*s%s",
 			       (int)match.rm_so, bol,
 			       opt->color_match,

-- 
1.6.3.1
Junio C Hamano· Jun 1, 2009, 21:17 UTC · re: René Scharfe · lore

Re: [PATCH] grep: fix colouring of matches with zero length

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> The first hunk fixes match colouring if the patterns is the empty string.
> Such a pattern matches the whole line,...
Hmmm...

An empty pattern may have to produce a match with any line, but I do not think it should match the whole line. GNU seems to agree with me.

	echo foo | grep --color -e ''
        echo foo | grep --color -F -e ''
        echo foo | grep --color -e '.*'
        echo foo | grep --color -F -e 'foo'
The first two are uncoloured (but still show matches).
> The second hunk of the patch makes sure that grep match colouring doesn't
> get stuck in an infinite loop if the length of a match is zero.

Certainly we need to advance the match pointer, and this issue needs to be addressed. I think it is a good change.

Show 37 quoted lines
> of the line is simply printed without colouring.  Even though the empty
> pattern string doesn't result in empty matches any more, this defensive
> measure catches unknown cases.
>
> Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
> ---
>  grep.c |    8 +++++++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
>
> diff --git a/grep.c b/grep.c
> index cc6d5b0..745b3c0 100644
> --- a/grep.c
> +++ b/grep.c
> @@ -324,7 +324,11 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,
>  	}
>  
>   again:
> -	if (p->fixed)
> +	if (p->pattern[0] == '\0') {
> +		hit = 1;
> +		pmatch[0].rm_so = 0;
> +		pmatch[0].rm_eo = eol - bol;
> +	} else if (p->fixed)
>  		hit = !fixmatch(p->pattern, bol, pmatch);
>  	else
>  		hit = !regexec(&p->regexp, bol, 1, pmatch, eflags);
> @@ -500,6 +504,8 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
>  
>  		*eol = '\0';
>  		while (next_match(opt, bol, eol, ctx, &match, eflags)) {
> +			if (match.rm_so == match.rm_eo)
> +				break;
>  			printf("%.*s%s%.*s%s",
>  			       (int)match.rm_so, bol,
>  			       opt->color_match,
> -- 
> 1.6.3.1
René Scharfe· Jun 1, 2009, 21:53 UTC · re: Junio C Hamano · lore

Re: [PATCH] grep: fix colouring of matches with zero length

Junio C Hamano schrieb:
Show 16 quoted lines
> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> 
>> The first hunk fixes match colouring if the patterns is the empty string.
>> Such a pattern matches the whole line,...
> 
> Hmmm...
> 
> An empty pattern may have to produce a match with any line, but I do not
> think it should match the whole line.  GNU seems to agree with me.
> 
> 	echo foo | grep --color -e ''
>         echo foo | grep --color -F -e ''
>         echo foo | grep --color -e '.*'
>         echo foo | grep --color -F -e 'foo'
> 
> The first two are uncoloured (but still show matches).

You're right, and matching the zero-length string at the beginning of the line actually makes sense. Corrected patch below.

If a zero-length match is encountered, break out of loop and show the rest of the line uncoloured. Otherwise we'd be looping forever, trying to make progress by advancing the pointer by zero characters.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 grep.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
Show changes to grep.c +2 −0
diff --git a/grep.c b/grep.c
index cc6d5b0..7bf4a60 100644
--- a/grep.c
+++ b/grep.c
@@ -500,6 +500,8 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 
 		*eol = '\0';
 		while (next_match(opt, bol, eol, ctx, &match, eflags)) {
+			if (match.rm_so == match.rm_eo)
+				break;
 			printf("%.*s%s%.*s%s",
 			       (int)match.rm_so, bol,
 			       opt->color_match,

-- 
1.6.3.1

← back to recent threads