Rahul Bedarkar <rahul.bedarkar@xxxxxxxxxx> 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;
> }
> }