git grep '(' = segfault

git grep '(' = segfault

[Dmitry Potapov]

Hi,

I have tried a few version of Git and got "Segmentation fault" when
run:

$ git grep '('

I am not very familiar with grep code and don't have time to dig
into it right now. So, maybe someone else can take a look at it.

Thanks,
Dmitry

Re: git grep '(' = segfault

[Linus Torvalds]

On Mon, 27 Apr 2009, Dmitry Potapov wrote:
> 
> I have tried a few version of Git and got "Segmentation fault" when
> run:
> 
> $ git grep '('
> 
> I am not very familiar with grep code and don't have time to dig
> into it right now. So, maybe someone else can take a look at it.

Good job.

Something like this should fix it.

		Linus

---
 grep.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/grep.c b/grep.c
index f3a27d7..04c777a 100644
--- a/grep.c
+++ b/grep.c
@@ -72,6 +72,8 @@ static struct grep_expr *compile_pattern_atom(struct grep_pat **list)
 	struct grep_expr *x;
 
 	p = *list;
+	if (!p)
+		return NULL;
 	switch (p->token) {
 	case GREP_PATTERN: /* atom */
 	case GREP_PATTERN_HEAD:
@@ -84,8 +86,6 @@ static struct grep_expr *compile_pattern_atom(struct grep_pat **list)
 	case GREP_OPEN_PAREN:
 		*list = p->next;
 		x = compile_pattern_or(list);
-		if (!x)
-			return NULL;
 		if (!*list || (*list)->token != GREP_CLOSE_PAREN)
 			die("unmatched parenthesis");
 		*list = (*list)->next;
@@ -101,6 +101,8 @@ static struct grep_expr *compile_pattern_not(struct grep_pat **list)
 	struct grep_expr *x;
 
 	p = *list;
+	if (!p)
+		return NULL;
 	switch (p->token) {
 	case GREP_NOT:
 		if (!p->next)
@@ -372,6 +374,8 @@ static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,
 	int h = 0;
 	regmatch_t match;
 
+	if (!x)
+		die("Not a valid grep expression");
 	switch (x->node) {
 	case GREP_NODE_ATOM:
 		h = match_one_pattern(x->u.atom, bol, eol, ctx, &match, 0);

Re: git grep '(' = segfault

[Erik Faye-Lund]

On Mon, Apr 27, 2009 at 8:10 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> Something like this should fix it.
>
>                Linus

(Sorry that you're getting this mail twice, Linus - I forgot to reply all)

This does indeed fix the crash, but I find the resulting operation
quite bizarre. I'm getting a "fatal: unmatched paranthesis", however
"git grep '(('" is happy to simply grep for the double parenthesis.
Now, if I had done "git grep '\('", I'd expect an unmatched
parenthesis-error... Am I misunderstanding something here?

$ git grep '('
fatal: unmatched parenthesis

$ git grep '(('
Documentation/CodingGuidelines: - We use Arithmetic Expansion $(( ... )).
[...]

$ git grep '\('
fatal: command line, '\(': Unmatched ( or \(

-- 
Erik "kusma" Faye-Lund
kusmabite@gmail.com
(+47) 986 59 656

Re: git grep '(' = segfault

[Linus Torvalds]

On Mon, 27 Apr 2009, Erik Faye-Lund wrote:
> On Mon, Apr 27, 2009 at 8:10 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> > Something like this should fix it.
> >
> >                Linus
> 
> (Sorry that you're getting this mail twice, Linus - I forgot to reply all)

Payback time. Now _you'll_ get it twice. Mmmwwhahahahaahaaa!

> $ git grep '('
> fatal: unmatched parenthesis

Try this:

        git grep -e '('

because 'git grep' actually allows multiple patterns, so you can do things 
like

        git grep '(' -e hello --or -e 'hi there' ')' --and -e world

Practical? Maybe.

                Linus

Re: git grep '(' = segfault

[Erik Faye-Lund]

> Practical? Maybe.

Confusing? Yes. ;)

-- 
Erik "kusma" Faye-Lund
kusmabite@gmail.com
(+47) 986 59 656

Re: git grep '(' = segfault

[Matthieu Moy]

Erik Faye-Lund <kusmabite@googlemail.com> writes:

> $ git grep '('
> fatal: unmatched parenthesis

try this: git grep -e '('

The idea is that you can do things like

git grep -e foo --and '(' -e bar --or -e boz ')'

to build a boolean expression of expressions.

-- 
Matthieu

Re: git grep '(' = segfault

[Erik Faye-Lund]

Yeah, thanks for explaining. But isn't this functionality kind of
redundant since we have the -E switch?
As far as I can see, "git grep -E "(bar|boz)" should do the same thing
as "git grep -e foo --and '(' -e bar --or -e boz ')'"...

On Mon, Apr 27, 2009 at 8:54 PM, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
> Erik Faye-Lund <kusmabite@googlemail.com> writes:
>
>> $ git grep '('
>> fatal: unmatched parenthesis
>
> try this: git grep -e '('
>
> The idea is that you can do things like
>
> git grep -e foo --and '(' -e bar --or -e boz ')'
>
> to build a boolean expression of expressions.
>
> --
> Matthieu
>



-- 
Erik "kusma" Faye-Lund
kusmabite@gmail.com
(+47) 986 59 656

Re: git grep '(' = segfault

[Junio C Hamano]

Erik Faye-Lund <kusmabite@googlemail.com> writes:

> Yeah, thanks for explaining. But isn't this functionality kind of
> redundant since we have the -E switch?
> As far as I can see, "git grep -E "(bar|boz)" should do the same thing
> as "git grep -e foo --and '(' -e bar --or -e boz ')'"...

With -E, --or is not strictly necessary.

With only -E, however, it is cumbersome to express what --and can.  Once
you learn the handiness of --and, naturally you would start wanting to
be able to say --or (especially if you do not even know about -E).

Re: git grep '(' = segfault

[Michał Kiedrowicz]

Dmitry Potapov <dpotapov@gmail.com> wrote:

> Hi,
> 
> I have tried a few version of Git and got "Segmentation fault" when
> run:
> 
> $ git grep '('
> 
> I am not very familiar with grep code and don't have time to dig
> into it right now. So, maybe someone else can take a look at it.
> 
> Thanks,
> Dmitry
> --

Try this patch, however I'm not sure if it is correct.

---

diff --git a/grep.c b/grep.c
index f3a27d7..5748355 100644
--- a/grep.c
+++ b/grep.c
@@ -84,10 +84,10 @@ static struct grep_expr *compile_pattern_atom(struct grep_pat **list)
 	case GREP_OPEN_PAREN:
 		*list = p->next;
 		x = compile_pattern_or(list);
-		if (!x)
-			return NULL;
 		if (!*list || (*list)->token != GREP_CLOSE_PAREN)
 			die("unmatched parenthesis");
+		if (!x)
+			return NULL;
 		*list = (*list)->next;
 		return x;
 	default: