On Tue, Jun 10, 2014 at 07:12:37AM +0200, Christian Couder wrote: > From: Jeff King <peff@xxxxxxxx> > > > > --- 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html