On Tue, Jun 10, 2014 at 07:12:37AM +0200, Christian Couder wrote:

> From: Jeff King <p...@peff.net>
> >
> > --- a/builtin/blame.c
> > +++ b/builtin/blame.c
> > @@ -2313,7 +2313,7 @@ static struct commit *fake_working_tree_commit(struct 
> > diff_options *opt,
> >                 ident, ident, path,
> >                 (!contents_from ? path :
> >                  (!strcmp(contents_from, "-") ? "standard input" : 
> > contents_from)));
> > -   set_commit_buffer(commit, strbuf_detach(&msg, NULL));
> > +   set_commit_buffer(commit, msg.buf, msg.len);
> 
> I find the above strange. I would have done something like:
> 
> -     set_commit_buffer(commit, strbuf_detach(&msg, NULL));
> +     size_t size;
> +     char *buf = strbuf_detach(&msg, &size);
> +     set_commit_buffer(commit, buf, size);

It is a little strange. You can't do:

  set_commit_buffer(commit, strbuf_detach(&msg, NULL), msg.len);

because the second argument resets msg.len as a side effect. Doing it
your way is longer, but perhaps is a bit more canonical. In general, one
should call strbuf_detach to ensure that the buffer is allocated (and
does not point to slopbuf). That's guaranteed here, because we just put
contents into the buffer, but it's probably more hygienic to use the
more verbose form.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to