On Fri, Feb 08, 2013 at 04:29:11PM -0800, Junio C Hamano wrote:
> Perhaps something along this line...
>
> -- >8 --
> Subject: "log --grep": commit's buffer may already have been discarded
>
> Following up on be5c9fb9049e (logmsg_reencode: lazily load missing
> commit buffers, 2013-01-26), extract the part that reads the commit
> buffer data into a separate helper function, and use it when we
> apply the grep filter on the commit during the log walk.
This obviously makes sense if we don't want to get the route of
re-encoding for grep. Re-encoding would be a user-visible change, but I
wonder if it is the right thing to be doing.
> diff --git a/revision.c b/revision.c
> index d7562ee..caf8ef3 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -2279,9 +2279,16 @@ static int commit_match(struct commit *commit, struct rev_info *opt)
> strbuf_addch(&buf, '\n');
> }
>
> - /* Copy the commit to temporary if we are using "fake" headers */
> - if (buf.len)
> + if (!commit->buffer) {
> + /* we may not have commit->buffer */
> + unsigned long size;
> + char *msg = read_commit_object_data(commit, &size);
> + strbuf_add(&buf, msg, size);
> + free(msg);
> + } else if (buf.len) {
> + /* Copy the commit to temporary if we are using "fake" headers */
> strbuf_addstr(&buf, commit->buffer);
> + }
Hmm. It would be nice to avoid the extra copy when we do not otherwise
need to use the strbuf. I would have expected something more like:
const char *msg = commit->buffer;
if (!msg)
msg = read_commit_object_data(commit, NULL);
[...]
if (buf.len)
retval = grep_buffer(&opt->grep_filter, buf.buf, buf.len);
else
retval = grep_buffer(&opt->grep_filter, msg, strlen(msg));
strbuf_release(&buf);
if (msg != commit->buffer)
free(msg);
return retval;
You would also need to adjust the other uses of commit->buffer
throughout the function to refer to msg.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html