replace dangerous uses of strbuf_attach
authorJeff King <peff@peff.net>
Tue, 10 Jun 2014 21:38:38 +0000 (17:38 -0400)
committerJunio C Hamano <gitster@pobox.com>
Thu, 12 Jun 2014 17:29:42 +0000 (10:29 -0700)
It is not a good idea to strbuf_attach an arbitrary pointer
just because a function you are calling wants a strbuf.
Attaching implies a transfer of memory ownership; if anyone
were to modify or release the resulting strbuf, we would
free() the pointer, leading to possible problems:

1. Other users of the original pointer might access freed
memory.

2. The pointer might not be the start of a malloc'd
area, so calling free() on it in the first place would
be wrong.

In the two cases modified here, we are fortunate that nobody
touches the strbuf once it is attached, but it is an
accident waiting to happen. Since the previous commit,
commit_tree and friends take a pointer/buf pair, so we can
just do away with the strbufs entirely.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
notes-cache.c
notes-merge.c
index 9d27b66ebcc51d24d88c9d1d6352344ebc3b466a..25b20aa21ff3896111e99c356648e934060b1825 100644 (file)
@@ -48,7 +48,6 @@ int notes_cache_write(struct notes_cache *c)
 {
        unsigned char tree_sha1[20];
        unsigned char commit_sha1[20];
-       struct strbuf msg = STRBUF_INIT;
 
        if (!c || !c->tree.initialized || !c->tree.ref || !*c->tree.ref)
                return -1;
@@ -57,9 +56,8 @@ int notes_cache_write(struct notes_cache *c)
 
        if (write_notes_tree(&c->tree, tree_sha1))
                return -1;
-       strbuf_attach(&msg, c->validity,
-                     strlen(c->validity), strlen(c->validity) + 1);
-       if (commit_tree(msg.buf, msg.len, tree_sha1, NULL, commit_sha1, NULL, NULL) < 0)
+       if (commit_tree(c->validity, strlen(c->validity), tree_sha1, NULL,
+                       commit_sha1, NULL, NULL) < 0)
                return -1;
        if (update_ref("update notes cache", c->tree.ref, commit_sha1, NULL,
                       0, QUIET_ON_ERR) < 0)
index 9d942104d2156b1f05554d92c43a5a136fbac125..697cec349a2ee3ece9cedd735a4da4828f6621cd 100644 (file)
@@ -673,7 +673,6 @@ int notes_merge_commit(struct notes_merge_options *o,
        struct dirent *e;
        struct strbuf path = STRBUF_INIT;
        char *msg = strstr(partial_commit->buffer, "\n\n");
-       struct strbuf sb_msg = STRBUF_INIT;
        int baselen;
 
        strbuf_addstr(&path, git_path(NOTES_MERGE_WORKTREE));
@@ -720,10 +719,8 @@ int notes_merge_commit(struct notes_merge_options *o,
                strbuf_setlen(&path, baselen);
        }
 
-       strbuf_attach(&sb_msg, msg, strlen(msg), strlen(msg) + 1);
        create_notes_commit(partial_tree, partial_commit->parents,
-                           sb_msg.buf, sb_msg.len,
-                           result_sha1);
+                           msg, strlen(msg), result_sha1);
        if (o->verbosity >= 4)
                printf("Finalized notes merge commit: %s\n",
                        sha1_to_hex(result_sha1));