[PATCH] grep: fix build with no thread support

[PATCH] grep: fix build with no thread support

[Rahul Bedarkar]

Commit 0281e487fd91 ("grep: optionally recurse into submodules")
added functions grep_submodule() and grep_submodule_launch() which
uses "struct work_item" which is defined only when thread support
is available.

When building git with toolchain that don't have thread support, we
get following build errors:

      CC builtin/hash-object.o
  builtin/grep.c: In function 'grep_submodule_launch':
  builtin/grep.c:596:34: error: dereferencing pointer to incomplete type 'struct work_item'
    status = capture_command(&cp, &w->out, 0);
                                    ^
  builtin/grep.c: In function 'grep_submodule':
  builtin/grep.c:644:20: error: storage size of 'w' isn't known
     struct work_item w;
                      ^
  make[2]: *** [builtin/grep.o] Error 1
  make[2]: *** Waiting for unfinished jobs....

This commit fixes above build issue by using "struct grep_source" and
"struct strbuf" instead of "struct work_item" in grep_submodule() and
grep_submodule_launch() when thread support is not available.

This build failure is detected by Buildroot Autobuilder:
http://autobuild.buildroot.net/results/94b/94bce9a99a5ce9894a6918774ab75e23d12c1394/

Cc: Brandon Williams <bmwill@google.com>
Fixes: 0281e487fd91 ("grep: optionally recurse into submodules")
Signed-off-by: Rahul Bedarkar <rahul.bedarkar@imgtec.com>
---

This patch is tested by running t7814-grep-recurse-submodules.sh with
both with/without thread support builds.

 builtin/grep.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 2c727ef..4373d79 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -183,11 +183,13 @@ static void *run(void *arg)
 		if (!w)
 			break;
 
-		opt->output_priv = w;
-		if (w->source.type == GREP_SOURCE_SUBMODULE)
+		if (w->source.type == GREP_SOURCE_SUBMODULE) {
+			opt->output_priv = &w->out;
 			hit |= grep_submodule_launch(opt, &w->source);
-		else
+		} else {
+			opt->output_priv = w;
 			hit |= grep_source(opt, &w->source);
+		}
 		grep_source_clear_data(&w->source);
 		work_done(w);
 	}
@@ -538,7 +540,7 @@ static int grep_submodule_launch(struct grep_opt *opt,
 	int status, i;
 	const char *end_of_base;
 	const char *name;
-	struct work_item *w = opt->output_priv;
+	struct strbuf *w = opt->output_priv;
 
 	end_of_base = strchr(gs->name, ':');
 	if (gs->identifier && end_of_base)
@@ -593,10 +595,10 @@ static int grep_submodule_launch(struct grep_opt *opt,
 	 * child process.  A '0' indicates a hit, a '1' indicates no hit and
 	 * anything else is an error.
 	 */
-	status = capture_command(&cp, &w->out, 0);
+	status = capture_command(&cp, w, 0);
 	if (status && (status != 1)) {
 		/* flush the buffer */
-		write_or_die(1, w->out.buf, w->out.len);
+		write_or_die(1, w->buf, w->len);
 		die("process for submodule '%s' failed with exit code: %d",
 		    gs->name, status);
 	}
@@ -641,19 +643,19 @@ static int grep_submodule(struct grep_opt *opt, const unsigned char *sha1,
 	} else
 #endif
 	{
-		struct work_item w;
+		struct grep_source gs;
 		int hit;
+		struct strbuf outbuf = STRBUF_INIT;
 
-		grep_source_init(&w.source, GREP_SOURCE_SUBMODULE,
+		grep_source_init(&gs, GREP_SOURCE_SUBMODULE,
 				 filename, path, sha1);
-		strbuf_init(&w.out, 0);
-		opt->output_priv = &w;
-		hit = grep_submodule_launch(opt, &w.source);
+		opt->output_priv = &outbuf;
+		hit = grep_submodule_launch(opt, &gs);
 
-		write_or_die(1, w.out.buf, w.out.len);
+		write_or_die(1, outbuf.buf, outbuf.len);
 
-		grep_source_clear(&w.source);
-		strbuf_release(&w.out);
+		grep_source_clear(&gs);
+		strbuf_release(&outbuf);
 		return hit;
 	}
 }
-- 
2.6.2

Re: [PATCH] grep: fix build with no thread support

[Junio C Hamano]

Rahul Bedarkar <rahul.bedarkar@imgtec.com> writes:

> diff --git a/builtin/grep.c b/builtin/grep.c
> index 2c727ef..4373d79 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -183,11 +183,13 @@ static void *run(void *arg)
>  		if (!w)
>  			break;
>  
> -		opt->output_priv = w;
> -		if (w->source.type == GREP_SOURCE_SUBMODULE)
> +		if (w->source.type == GREP_SOURCE_SUBMODULE) {
> +			opt->output_priv = &w->out;
>  			hit |= grep_submodule_launch(opt, &w->source);
> -		else
> +		} else {
> +			opt->output_priv = w;
>  			hit |= grep_source(opt, &w->source);
> +		}
>  		grep_source_clear_data(&w->source);
>  		work_done(w);
>  	}

This is not a part of the "fix" but merely a code clean-up, right?
Just double-checking.

> @@ -538,7 +540,7 @@ static int grep_submodule_launch(struct grep_opt *opt,
>  	int status, i;
>  	const char *end_of_base;
>  	const char *name;
> -	struct work_item *w = opt->output_priv;
> +	struct strbuf *w = opt->output_priv;
>  
>  	end_of_base = strchr(gs->name, ':');
>  	if (gs->identifier && end_of_base)

OK, so the new code points output_priv at a strbuf; work_item
contains an "out" strbuf, and that was why the original code was
passing one, but this codepath does not need a full work_item to
work with.  Is that what is going on?

> @@ -593,10 +595,10 @@ static int grep_submodule_launch(struct grep_opt *opt,
>  	 * child process.  A '0' indicates a hit, a '1' indicates no hit and
>  	 * anything else is an error.
>  	 */
> -	status = capture_command(&cp, &w->out, 0);
> +	status = capture_command(&cp, w, 0);

And this is consistent with the above change.

>  	if (status && (status != 1)) {
>  		/* flush the buffer */
> -		write_or_die(1, w->out.buf, w->out.len);
> +		write_or_die(1, w->buf, w->len);

So is this.

>  		die("process for submodule '%s' failed with exit code: %d",
>  		    gs->name, status);
>  	}
> @@ -641,19 +643,19 @@ static int grep_submodule(struct grep_opt *opt, const unsigned char *sha1,
>  	} else
>  #endif
>  	{
> -		struct work_item w;
> +		struct grep_source gs;
>  		int hit;
> +		struct strbuf outbuf = STRBUF_INIT;
>  
> -		grep_source_init(&w.source, GREP_SOURCE_SUBMODULE,
> +		grep_source_init(&gs, GREP_SOURCE_SUBMODULE,
>  				 filename, path, sha1);

Likewise for w.source that happened to have grep_source, but passing
a bare grep_source is sufficient for the purpose of this codepath,
without giving it to w.source?  

I didn't look at code that this patch does not touch, but if
anything still looks at w.out and w.source and expect these to
contain the string accumulated in the strbuf and the grep source the
work item is working on, they will get broken by this change, no?

The first hunk that had a pure clean-up shows that w->source being
the correct grep source seems to matter.

> -		strbuf_init(&w.out, 0);
> -		opt->output_priv = &w;
> -		hit = grep_submodule_launch(opt, &w.source);
> +		opt->output_priv = &outbuf;
> +		hit = grep_submodule_launch(opt, &gs);
>  
> -		write_or_die(1, w.out.buf, w.out.len);
> +		write_or_die(1, outbuf.buf, outbuf.len);
>  
> -		grep_source_clear(&w.source);
> -		strbuf_release(&w.out);
> +		grep_source_clear(&gs);
> +		strbuf_release(&outbuf);
>  		return hit;
>  	}
>  }

Re: [PATCH] grep: fix build with no thread support

[Brandon Williams]

On 03/17, Junio C Hamano wrote:
> Rahul Bedarkar <rahul.bedarkar@imgtec.com> writes:
> 
> > diff --git a/builtin/grep.c b/builtin/grep.c
> > index 2c727ef..4373d79 100644
> > --- a/builtin/grep.c
> > +++ b/builtin/grep.c
> > @@ -183,11 +183,13 @@ static void *run(void *arg)
> >  		if (!w)
> >  			break;
> >  
> > -		opt->output_priv = w;
> > -		if (w->source.type == GREP_SOURCE_SUBMODULE)
> > +		if (w->source.type == GREP_SOURCE_SUBMODULE) {
> > +			opt->output_priv = &w->out;
> >  			hit |= grep_submodule_launch(opt, &w->source);
> > -		else
> > +		} else {
> > +			opt->output_priv = w;
> >  			hit |= grep_source(opt, &w->source);
> > +		}
> >  		grep_source_clear_data(&w->source);
> >  		work_done(w);
> >  	}
> 
> This is not a part of the "fix" but merely a code clean-up, right?
> Just double-checking.
> 
> > @@ -538,7 +540,7 @@ static int grep_submodule_launch(struct grep_opt *opt,
> >  	int status, i;
> >  	const char *end_of_base;
> >  	const char *name;
> > -	struct work_item *w = opt->output_priv;
> > +	struct strbuf *w = opt->output_priv;
> >  
> >  	end_of_base = strchr(gs->name, ':');
> >  	if (gs->identifier && end_of_base)
> 
> OK, so the new code points output_priv at a strbuf; work_item
> contains an "out" strbuf, and that was why the original code was
> passing one, but this codepath does not need a full work_item to
> work with.  Is that what is going on?
> 
> > @@ -593,10 +595,10 @@ static int grep_submodule_launch(struct grep_opt *opt,
> >  	 * child process.  A '0' indicates a hit, a '1' indicates no hit and
> >  	 * anything else is an error.
> >  	 */
> > -	status = capture_command(&cp, &w->out, 0);
> > +	status = capture_command(&cp, w, 0);
> 
> And this is consistent with the above change.
> 
> >  	if (status && (status != 1)) {
> >  		/* flush the buffer */
> > -		write_or_die(1, w->out.buf, w->out.len);
> > +		write_or_die(1, w->buf, w->len);
> 
> So is this.
> 
> >  		die("process for submodule '%s' failed with exit code: %d",
> >  		    gs->name, status);
> >  	}
> > @@ -641,19 +643,19 @@ static int grep_submodule(struct grep_opt *opt, const unsigned char *sha1,
> >  	} else
> >  #endif
> >  	{
> > -		struct work_item w;
> > +		struct grep_source gs;
> >  		int hit;
> > +		struct strbuf outbuf = STRBUF_INIT;
> >  
> > -		grep_source_init(&w.source, GREP_SOURCE_SUBMODULE,
> > +		grep_source_init(&gs, GREP_SOURCE_SUBMODULE,
> >  				 filename, path, sha1);
> 
> Likewise for w.source that happened to have grep_source, but passing
> a bare grep_source is sufficient for the purpose of this codepath,
> without giving it to w.source?  
> 
> I didn't look at code that this patch does not touch, but if
> anything still looks at w.out and w.source and expect these to
> contain the string accumulated in the strbuf and the grep source the
> work item is working on, they will get broken by this change, no?
> 
> The first hunk that had a pure clean-up shows that w->source being
> the correct grep source seems to matter.
> 
> > -		strbuf_init(&w.out, 0);
> > -		opt->output_priv = &w;
> > -		hit = grep_submodule_launch(opt, &w.source);
> > +		opt->output_priv = &outbuf;
> > +		hit = grep_submodule_launch(opt, &gs);
> >  
> > -		write_or_die(1, w.out.buf, w.out.len);
> > +		write_or_die(1, outbuf.buf, outbuf.len);
> >  
> > -		grep_source_clear(&w.source);
> > -		strbuf_release(&w.out);
> > +		grep_source_clear(&gs);
> > +		strbuf_release(&outbuf);
> >  		return hit;
> >  	}
> >  }

Thanks for pointing out the bug (there seem to just be more and more in
this grep code...but what else can you expect from my first
contribution! :D) but I think it would be better to make the way the
recursive code output's more consistent with the way the rest of the
grep handles output.  That way we can get rid of some of the special
casing that I had done here.

I'll send out what I have locally in just a second.

-- 
Brandon Williams

[PATCH 1/2] grep: set default output method

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 grep.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/grep.c b/grep.c
index 0dbdc1d00..56ef0ecbf 100644
--- a/grep.c
+++ b/grep.c
@@ -12,6 +12,11 @@ static int grep_source_is_binary(struct grep_source *gs);
 
 static struct grep_opt grep_defaults;
 
+static void std_output(struct grep_opt *opt, const void *buf, size_t size)
+{
+	fwrite(buf, size, 1, stdout);
+}
+
 /*
  * Initialize the grep_defaults template with hardcoded defaults.
  * We could let the compiler do this, but without C99 initializers
@@ -42,6 +47,7 @@ void init_grep_defaults(void)
 	color_set(opt->color_selected, "");
 	color_set(opt->color_sep, GIT_COLOR_CYAN);
 	opt->color = -1;
+	opt->output = std_output;
 }
 
 static int parse_pattern_type_arg(const char *opt, const char *arg)
@@ -152,6 +158,7 @@ void grep_init(struct grep_opt *opt, const char *prefix)
 	opt->pathname = def->pathname;
 	opt->regflags = def->regflags;
 	opt->relative = def->relative;
+	opt->output = def->output;
 
 	color_set(opt->color_context, def->color_context);
 	color_set(opt->color_filename, def->color_filename);
@@ -1379,11 +1386,6 @@ static int look_ahead(struct grep_opt *opt,
 	return 0;
 }
 
-static void std_output(struct grep_opt *opt, const void *buf, size_t size)
-{
-	fwrite(buf, size, 1, stdout);
-}
-
 static int fill_textconv_grep(struct userdiff_driver *driver,
 			      struct grep_source *gs)
 {
-- 
2.12.0.367.g23dc2f6d3c-goog

[PATCH 2/2] grep: fix builds with with no thread support

[Brandon Williams]

Commit 0281e487fd91 ("grep: optionally recurse into submodules")
added functions grep_submodule() and grep_submodule_launch() which
uses "struct work_item" which is defined only when thread support
is available.

The original implementation of grep_submodule() used the "struct
work_item" in order to gain access to a strbuf to store its output which
was to be printed at a later point in time.  This differs from how both
grep_file() and grep_sha1() handle their output.  This patch eliminates
the reliance on the "struct work_item" and instead opts to use the
output function stored in the output field of the "struct grep_opt"
object directly, making it behave similarly to both grep_file() and
grep_sha1().

Reported-by: Rahul Bedarkar <rahul.bedarkar@imgtec.com>
Signed-off-by: Brandon Williams <bmwill@google.com>
---
 builtin/grep.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 9304c33e7..3f1251bab 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -538,7 +538,7 @@ static int grep_submodule_launch(struct grep_opt *opt,
 	int status, i;
 	const char *end_of_base;
 	const char *name;
-	struct work_item *w = opt->output_priv;
+	struct strbuf child_output = STRBUF_INIT;
 
 	end_of_base = strchr(gs->name, ':');
 	if (gs->identifier && end_of_base)
@@ -593,14 +593,16 @@ static int grep_submodule_launch(struct grep_opt *opt,
 	 * child process.  A '0' indicates a hit, a '1' indicates no hit and
 	 * anything else is an error.
 	 */
-	status = capture_command(&cp, &w->out, 0);
+	status = capture_command(&cp, &child_output, 0);
 	if (status && (status != 1)) {
 		/* flush the buffer */
-		write_or_die(1, w->out.buf, w->out.len);
+		write_or_die(1, child_output.buf, child_output.len);
 		die("process for submodule '%s' failed with exit code: %d",
 		    gs->name, status);
 	}
 
+	opt->output(opt, child_output.buf, child_output.len);
+	strbuf_release(&child_output);
 	/* invert the return code to make a hit equal to 1 */
 	return !status;
 }
@@ -641,19 +643,14 @@ static int grep_submodule(struct grep_opt *opt, const unsigned char *sha1,
 	} else
 #endif
 	{
-		struct work_item w;
+		struct grep_source gs;
 		int hit;
 
-		grep_source_init(&w.source, GREP_SOURCE_SUBMODULE,
+		grep_source_init(&gs, GREP_SOURCE_SUBMODULE,
 				 filename, path, sha1);
-		strbuf_init(&w.out, 0);
-		opt->output_priv = &w;
-		hit = grep_submodule_launch(opt, &w.source);
+		hit = grep_submodule_launch(opt, &gs);
 
-		write_or_die(1, w.out.buf, w.out.len);
-
-		grep_source_clear(&w.source);
-		strbuf_release(&w.out);
+		grep_source_clear(&gs);
 		return hit;
 	}
 }
-- 
2.12.0.367.g23dc2f6d3c-goog

Re: [PATCH] grep: fix build with no thread support

[Brandon Williams]

While taking a look at this bug I discovered that the test suite doesn't
pass 100% of the test when compiled with the NO_PTHREADS option. The
following tests seem to be failing:

t1060-object-corruption.sh                       (Wstat: 256 Tests: 13 Failed: 3)
  Failed tests:  7-9
  Non-zero exit status: 1
t5306-pack-nobase.sh                             (Wstat: 256 Tests: 4 Failed: 1)
  Failed test:  4
  Non-zero exit status: 1
t5504-fetch-receive-strict.sh                    (Wstat: 256 Tests: 12 Failed: 2)
  Failed tests:  4-5
  Non-zero exit status: 1
t5530-upload-pack-error.sh                       (Wstat: 256 Tests: 10 Failed: 1)
  Failed test:  10
  Non-zero exit status: 1

I didn't take a close look at it but this would seem to indicate that we
don't worry to much about systems without pthreads support.  Just food
for thought.

-- 
Brandon Williams

Re: [PATCH] grep: fix build with no thread support

[Jeff King]

On Fri, Mar 17, 2017 at 11:47:01AM -0700, Brandon Williams wrote:

> While taking a look at this bug I discovered that the test suite doesn't
> pass 100% of the test when compiled with the NO_PTHREADS option. The
> following tests seem to be failing:
> 
> t1060-object-corruption.sh                       (Wstat: 256 Tests: 13 Failed: 3)
>   Failed tests:  7-9
>   Non-zero exit status: 1
> t5306-pack-nobase.sh                             (Wstat: 256 Tests: 4 Failed: 1)
>   Failed test:  4
>   Non-zero exit status: 1
> t5504-fetch-receive-strict.sh                    (Wstat: 256 Tests: 12 Failed: 2)
>   Failed tests:  4-5
>   Non-zero exit status: 1
> t5530-upload-pack-error.sh                       (Wstat: 256 Tests: 10 Failed: 1)
>   Failed test:  10
>   Non-zero exit status: 1
> 
> I didn't take a close look at it but this would seem to indicate that we
> don't worry to much about systems without pthreads support.  Just food
> for thought.

Hmm. We used to. What version did you test? Everything passes for me at
0281e487f^ (after that it fails to build). So AFAICT v2.12.0 is the
first release which does not work with NO_PTHREADS.

-Peff

Re: [PATCH] grep: fix build with no thread support

[Brandon Williams]

On 03/17, Jeff King wrote:
> On Fri, Mar 17, 2017 at 11:47:01AM -0700, Brandon Williams wrote:
> 
> > While taking a look at this bug I discovered that the test suite doesn't
> > pass 100% of the test when compiled with the NO_PTHREADS option. The
> > following tests seem to be failing:
> > 
> > t1060-object-corruption.sh                       (Wstat: 256 Tests: 13 Failed: 3)
> >   Failed tests:  7-9
> >   Non-zero exit status: 1
> > t5306-pack-nobase.sh                             (Wstat: 256 Tests: 4 Failed: 1)
> >   Failed test:  4
> >   Non-zero exit status: 1
> > t5504-fetch-receive-strict.sh                    (Wstat: 256 Tests: 12 Failed: 2)
> >   Failed tests:  4-5
> >   Non-zero exit status: 1
> > t5530-upload-pack-error.sh                       (Wstat: 256 Tests: 10 Failed: 1)
> >   Failed test:  10
> >   Non-zero exit status: 1
> > 
> > I didn't take a close look at it but this would seem to indicate that we
> > don't worry to much about systems without pthreads support.  Just food
> > for thought.
> 
> Hmm. We used to. What version did you test? Everything passes for me at
> 0281e487f^ (after that it fails to build). So AFAICT v2.12.0 is the
> first release which does not work with NO_PTHREADS.
> 
> -Peff

The version I ran tests on was what the master branch was pointing to a
day or so ago:

v2.12.0-264-gd6db3f216

-- 
Brandon Williams

Re: [PATCH] grep: fix build with no thread support

[Jeff King]

On Fri, Mar 17, 2017 at 03:42:32PM -0700, Brandon Williams wrote:

> > > I didn't take a close look at it but this would seem to indicate that we
> > > don't worry to much about systems without pthreads support.  Just food
> > > for thought.
> > 
> > Hmm. We used to. What version did you test? Everything passes for me at
> > 0281e487f^ (after that it fails to build). So AFAICT v2.12.0 is the
> > first release which does not work with NO_PTHREADS.
> > 
> > -Peff
> 
> The version I ran tests on was what the master branch was pointing to a
> day or so ago:
> 
> v2.12.0-264-gd6db3f216

Ah, OK. I couldn't get such a recent version to build with NO_PTHREADS,
but I assume you mean after applying your two patches.

v2.11.0 is fine, but v2.12.0 with your patches shows the problem.
Bisecting (and applying the patches when necessary) points to my
46df6906f (execv_dashed_external: wait for child on signal death,
2017-01-06).

-Peff

Re: [PATCH 2/2] grep: fix builds with with no thread support

[Jonathan Nieder]

Brandon Williams wrote:

> Commit 0281e487fd91 ("grep: optionally recurse into submodules")
> added functions grep_submodule() and grep_submodule_launch() which
> uses "struct work_item" which is defined only when thread support

nit: which use (s/uses/use/)

> is available.
[...]
> Reported-by: Rahul Bedarkar <rahul.bedarkar@imgtec.com>
> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---
>  builtin/grep.c | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)

With or without that commit message tweak,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Thanks.

Re: [PATCH] grep: fix build with no thread support

[Brandon Williams]

On 03/17, Jeff King wrote:
> On Fri, Mar 17, 2017 at 03:42:32PM -0700, Brandon Williams wrote:
> 
> > > > I didn't take a close look at it but this would seem to indicate that we
> > > > don't worry to much about systems without pthreads support.  Just food
> > > > for thought.
> > > 
> > > Hmm. We used to. What version did you test? Everything passes for me at
> > > 0281e487f^ (after that it fails to build). So AFAICT v2.12.0 is the
> > > first release which does not work with NO_PTHREADS.
> > > 
> > > -Peff
> > 
> > The version I ran tests on was what the master branch was pointing to a
> > day or so ago:
> > 
> > v2.12.0-264-gd6db3f216
> 
> Ah, OK. I couldn't get such a recent version to build with NO_PTHREADS,
> but I assume you mean after applying your two patches.
> 

Oh yes, sorry I should have mentioned that was with my patches applied.
My bad! :D

> v2.11.0 is fine, but v2.12.0 with your patches shows the problem.
> Bisecting (and applying the patches when necessary) points to my
> 46df6906f (execv_dashed_external: wait for child on signal death,
> 2017-01-06).
> 
> -Peff

-- 
Brandon Williams

Re: [PATCH 2/2] grep: fix builds with with no thread support

[Rahul Bedarkar]

Hello,

On Saturday 18 March 2017 12:11 AM, Brandon Williams wrote:
> Commit 0281e487fd91 ("grep: optionally recurse into submodules")
> added functions grep_submodule() and grep_submodule_launch() which
> uses "struct work_item" which is defined only when thread support
> is available.
>
> The original implementation of grep_submodule() used the "struct
> work_item" in order to gain access to a strbuf to store its output which
> was to be printed at a later point in time.  This differs from how both
> grep_file() and grep_sha1() handle their output.  This patch eliminates
> the reliance on the "struct work_item" and instead opts to use the
> output function stored in the output field of the "struct grep_opt"
> object directly, making it behave similarly to both grep_file() and
> grep_sha1().
>
> Reported-by: Rahul Bedarkar <rahul.bedarkar@imgtec.com>
> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---
>  builtin/grep.c | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)
>

It works for me.

Tested-by: Rahul Bedarkar <rahul.bedarkar@imgtec.com>

Thanks,
Rahul