On Tue, Jun 10, 2014 at 01:27:13AM -0400, Jeff King wrote:
> > 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.
I was trying to avoid cluttering up the function with the extra variable
definitions. I ended up with the change below, which I think is clear,
correct, and pushes the complexity outside of the function.
diff --git a/builtin/blame.c b/builtin/blame.c
index cde19eb..53f43ab 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2266,6 +2266,18 @@ static void append_merge_parents(struct commit_list **tail)
}
/*
+ * This isn't as simple as passing sb->buf and sb->len, because we
+ * want to transfer ownership of the buffer to the commit (so we
+ * must use detach).
+ */
+static void set_commit_buffer_from_strbuf(struct commit *c, struct strbuf *sb)
+{
+ size_t len;
+ void *buf = strbuf_detach(sb, &len);
+ set_commit_buffer(c, buf, len);
+}
+
+/*
* Prepare a dummy commit that represents the work tree (or staged) item.
* Note that annotating work tree item never works in the reverse.
*/
@@ -2313,7 +2325,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_from_strbuf(commit, &msg);
if (!contents_from || strcmp("-", contents_from)) {
struct stat st;
--
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