Date: Mon, 23 Aug 2010 19:13:01 +0200 From: Dimitry Andric <dimitry@andric.com> To: bf1783@gmail.com Cc: =?ISO-8859-1?Q?av?= <des@des.no>, =?ISO-8859-1?Q?Dag-Erling_Sm=F8rgr?=, freebsd-current@freebsd.org, "b. f." <bf1783@googlemail.com>, =?ISO-8859-1?Q?G=E1bor_K=F6vesd=E1n?= <gabor@FreeBSD.org> Subject: Re: Official request: Please make GNU grep the default Message-ID: <4C72AC1D.20100@andric.com> In-Reply-To: <AANLkTinZtyz=L32Pqch_m76zuSMK8FWaM=VWM70YZF%2B_@mail.gmail.com> References: <AANLkTinPj0x=ZZ6w_hdkt1_DdZ19AvUoRzcZyCKY%2BndB@mail.gmail.com> <86mxshdqgh.fsf@ds4.des.no> <AANLkTinZtyz=L32Pqch_m76zuSMK8FWaM=VWM70YZF%2B_@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------090708010302010401080909
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
On 2010-08-20 23:07, b. f. wrote:
> The lisp category is the only category that causes a problem with the
> new bsdgrep, and I didn't take the time to analyze why ( which is why
> I was not more specific in my original message). 'lisp' is preceded by
> 'elisp', which would normally be a match for the 'lisp' in a port
> Makefile, were it not for the -w flag. 'x11' succeeds, but it
> precedes all of the x11-* categories. I suspect that there is an
> error in the logic of either the -w or the -q flag implementation in
> bsdgrep, which causes problems when the two options are used together.
> The target succeeds as expected with GNU grep.
Can you please try the following patch? I think this solves more than
one problem in bsdgrep's logic, but it needs to be reviewed by Gabor and
others.
In usr.bin/grep/util.c, in the function procline(), there is the
following fragment:
290 /* Loop to process the whole line */
291 while (st <= l->len) {
[...]
295 /* Loop to compare with all the patterns */
296 for (i = 0; i < patterns; i++) {
[... sets r to 0 if a match was found ...]
336 if (r == 0) {
[...]
341 /* matches - skip further patterns */
342 if ((color != NULL && !oflag) || qflag || lflag)
343 break;
344 }
345 }
[...]
351 /* One pass if we are not recording matches */
352 if ((color != NULL && !oflag) || qflag || lflag)
353 break;
[...]
357 }
If during the first iteration of the "loop to process the whole line"
no match was found (for example, if doing a word search for "lisp" and
the string "elisp" is found instead), AND the -q option was used, the
test in line 352 aborts the whole loop, without searching any further!
Thus it will miss the "lisp" string later in the line.
It looks like line 352 should only be evaluated if the for() loop just
above it resulted in one or or more matches, so it is probably easiest
to just replace line 343 with a goto that jumps out of the two enclosing
loops (it is still a pity C does not have a "break 2" statement), and
delete lines 351..353 entirely.
However, if there are unintended side effects, for example with weird
combinations of the --color, -o and/or -l flags, please let me know. :)
--------------090708010302010401080909
Content-Type: text/plain;
name="bsdgrep-wq-fix.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="bsdgrep-wq-fix.diff"
diff --git a/usr.bin/grep/util.c b/usr.bin/grep/util.c
index c65d8f2..97a14f3 100644
--- a/usr.bin/grep/util.c
+++ b/usr.bin/grep/util.c
@@ -340,7 +340,7 @@ procline(struct str *l, int nottext)
matches[m++] = pmatch;
/* matches - skip further patterns */
if ((color != NULL && !oflag) || qflag || lflag)
- break;
+ goto skip;
}
}
@@ -348,9 +348,6 @@ procline(struct str *l, int nottext)
c = !c;
break;
}
- /* One pass if we are not recording matches */
- if ((color != NULL && !oflag) || qflag || lflag)
- break;
if (st == (size_t)pmatch.rm_so)
break; /* No matches */
@@ -358,6 +355,7 @@ procline(struct str *l, int nottext)
} else
c = !vflag;
+skip:
if (c && binbehave == BINFILE_BIN && nottext)
return (c); /* Binary file */
--------------090708010302010401080909--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4C72AC1D.20100>