@matheusbsbs / matheus-bagliano / commits / e6dfcd6767

replace dangerous uses of strbuf_attach

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>

Jeff King committed Jun 10, 2014 at 17:38 UTC e6dfcd6767a58816dacec0df39515803b267fbe6

2 files changed +3 -8

notes-cache.c

+2 -4

@@ -48,7 +48,6 @@ int notes_cache_write(struct notes_cache *c)

48 {

49 unsigned char tree_sha1[20];

50 unsigned char commit_sha1[20];

51 - struct strbuf msg = STRBUF_INIT;

52 if (!c || !c->tree.initialized || !c->tree.ref || !*c->tree.ref)

53 return -1;

@@ -57,9 +56,8 @@ int notes_cache_write(struct notes_cache *c)

57 if (write_notes_tree(&c->tree, tree_sha1))

58 return -1;

60 - strbuf_attach(&msg, c->validity,

61 - strlen(c->validity), strlen(c->validity) + 1);

62 - if (commit_tree(msg.buf, msg.len, tree_sha1, NULL, commit_sha1, NULL, NULL) < 0)

59 + if (commit_tree(c->validity, strlen(c->validity), tree_sha1, NULL,

60 + commit_sha1, NULL, NULL) < 0)

61 return -1;

62 if (update_ref("update notes cache", c->tree.ref, commit_sha1, NULL,

63 0, QUIET_ON_ERR) < 0)

notes-merge.c

+1 -4

@@ -673,7 +673,6 @@ int notes_merge_commit(struct notes_merge_options *o,

673 struct dirent *e;

674 struct strbuf path = STRBUF_INIT;

675 char *msg = strstr(partial_commit->buffer, "\n\n");

676 - struct strbuf sb_msg = STRBUF_INIT;

676 int baselen;

677

678 strbuf_addstr(&path, git_path(NOTES_MERGE_WORKTREE));

@@ -720,10 +719,8 @@ int notes_merge_commit(struct notes_merge_options *o,

719 strbuf_setlen(&path, baselen);

720 }

721

723 - strbuf_attach(&sb_msg, msg, strlen(msg), strlen(msg) + 1);

722 create_notes_commit(partial_tree, partial_commit->parents,

725 - sb_msg.buf, sb_msg.len,

726 - result_sha1);

723 + msg, strlen(msg), result_sha1);

724 if (o->verbosity >= 4)

725 printf("Finalized notes merge commit: %s\n",

726 sha1_to_hex(result_sha1));