Copy resolve_ref() return value for longer use
resolve_ref() may return a pointer to a static buffer. Callers that use this value longer than a couple of statements should copy the value to avoid some hidden resolve_ref() call that may change the static buffer's value. The bug found by Tony Wang <wwwjfy@gmail.com> in builtin/merge.c demonstrates this. The first call is in cmd_merge() branch = resolve_ref("HEAD", head_sha1, 0, &flag); Then deep in lookup_commit_or_die() a few lines after, resolve_ref() may be called again and destroy "branch". lookup_commit_or_die lookup_commit_reference lookup_commit_reference_gently parse_object lookup_replace_object do_lookup_replace_object prepare_replace_object for_each_replace_ref do_for_each_ref get_loose_refs get_ref_dir get_ref_dir resolve_ref All call sites are checked and made sure that xstrdup() is called if the value should be saved. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Nguyễn Thái Ngọc Duy committed
Nov 13, 2011 at 17:22 UTC
d5a35c114ab6b4337a1c7598bf75c331d94ee092
8 files changed
+66
-28
builtin/branch.c
+4
-1
@@ -115,8 +115,10 @@ static int branch_merged(int kind, const char *name,
115
branch->merge[0] &&
116
branch->merge[0]->dst &&
117
(reference_name =
118
- resolve_ref(branch->merge[0]->dst, sha1, 1, NULL)) != NULL)
118
+ resolve_ref(branch->merge[0]->dst, sha1, 1, NULL)) != NULL) {
119
+ reference_name = xstrdup(reference_name);
120
reference_rev = lookup_commit_reference(sha1);
121
+ }
122
}
123
if (!reference_rev)
124
reference_rev = head_rev;
@@ -141,6 +143,7 @@ static int branch_merged(int kind, const char *name,
143
" '%s', even though it is merged to HEAD."),
144
name, reference_name);
145
}
146
+ free((char *)reference_name);
147
return merged;
148
}
149
builtin/checkout.c
+3
-1
@@ -699,7 +699,9 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
699
unsigned char rev[20];
700
int flag;
701
memset(&old, 0, sizeof(old));
702
- old.path = xstrdup(resolve_ref("HEAD", rev, 0, &flag));
702
+ old.path = resolve_ref("HEAD", rev, 0, &flag);
703
+ if (old.path)
704
+ old.path = xstrdup(old.path);
705
old.commit = lookup_commit_reference_gently(rev, 1);
706
if (!(flag & REF_ISSYMREF)) {
707
free((char *)old.path);
builtin/commit.c
+2
-1
@@ -1259,7 +1259,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1,
1259
struct commit *commit;
1260
struct strbuf format = STRBUF_INIT;
1261
unsigned char junk_sha1[20];
1262
- const char *head = resolve_ref("HEAD", junk_sha1, 0, NULL);
1262
+ const char *head;
1263
struct pretty_print_context pctx = {0};
1264
struct strbuf author_ident = STRBUF_INIT;
1265
struct strbuf committer_ident = STRBUF_INIT;
@@ -1304,6 +1304,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1,
1304
rev.diffopt.break_opt = 0;
1305
diff_setup_done(&rev.diffopt);
1306
1307
+ head = resolve_ref("HEAD", junk_sha1, 0, NULL);
1308
printf("[%s%s ",
1309
!prefixcmp(head, "refs/heads/") ?
1310
head + 11 :
builtin/fmt-merge-msg.c
+5
-1
@@ -268,6 +268,7 @@ static int do_fmt_merge_msg(int merge_title, struct strbuf *in,
268
die("No current branch");
269
if (!prefixcmp(current_branch, "refs/heads/"))
270
current_branch += 11;
271
+ current_branch = xstrdup(current_branch);
272
273
/* get a line */
274
while (pos < in->len) {
@@ -283,8 +284,10 @@ static int do_fmt_merge_msg(int merge_title, struct strbuf *in,
284
die ("Error in line %d: %.*s", i, len, p);
285
}
286
286
- if (!srcs.nr)
287
+ if (!srcs.nr) {
288
+ free((char*)current_branch);
289
return 0;
290
+ }
291
292
if (merge_title)
293
do_fmt_merge_msg_title(out, current_branch);
@@ -306,6 +309,7 @@ static int do_fmt_merge_msg(int merge_title, struct strbuf *in,
309
shortlog(origins.items[i].string, origins.items[i].util,
310
head, &rev, shortlog_len, out);
311
}
312
+ free((char *)current_branch);
313
return 0;
314
}
315
builtin/merge.c
+40
-22
@@ -1082,7 +1082,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
1082
struct commit *head_commit;
1083
struct strbuf buf = STRBUF_INIT;
1084
const char *head_arg;
1085
- int flag, i;
1085
+ int flag, i, ret = 0;
1086
int best_cnt = -1, merge_was_ok = 0, automerge_was_ok = 0;
1087
struct commit_list *common = NULL;
1088
const char *best_strategy = NULL, *wt_strategy = NULL;
@@ -1096,8 +1096,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
1096
* current branch.
1097
*/
1098
branch = resolve_ref("HEAD", head_sha1, 0, &flag);
1099
- if (branch && !prefixcmp(branch, "refs/heads/"))
1100
- branch += 11;
1099
+ if (branch) {
1100
+ if (!prefixcmp(branch, "refs/heads/"))
1101
+ branch += 11;
1102
+ branch = xstrdup(branch);
1103
+ }
1104
if (!branch || is_null_sha1(head_sha1))
1105
head_commit = NULL;
1106
else
@@ -1121,7 +1124,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
1124
die(_("There is no merge to abort (MERGE_HEAD missing)."));
1125
1126
/* Invoke 'git reset --merge' */
1124
- return cmd_reset(nargc, nargv, prefix);
1127
+ ret = cmd_reset(nargc, nargv, prefix);
1128
+ goto done;
1129
}
1130
1131
if (read_cache_unmerged())
@@ -1205,7 +1209,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
1209
read_empty(remote_head->sha1, 0);
1210
update_ref("initial pull", "HEAD", remote_head->sha1, NULL, 0,
1211
DIE_ON_ERR);
1208
- return 0;
1212
+ goto done;
1213
} else {
1214
struct strbuf merge_names = STRBUF_INIT;
1215
@@ -1292,7 +1296,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
1296
* but first the most common case of merging one remote.
1297
*/
1298
finish_up_to_date("Already up-to-date.");
1295
- return 0;
1299
+ goto done;
1300
} else if (allow_fast_forward && !remoteheads->next &&
1301
!common->next &&
1302
!hashcmp(common->item->object.sha1, head_commit->object.sha1)) {
@@ -1313,15 +1317,20 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
1317
strbuf_addstr(&msg,
1318
" (no commit created; -m option ignored)");
1319
o = want_commit(sha1_to_hex(remoteheads->item->object.sha1));
1316
- if (!o)
1317
- return 1;
1320
+ if (!o) {
1321
+ ret = 1;
1322
+ goto done;
1323
+ }
1324
1319
- if (checkout_fast_forward(head_commit->object.sha1, remoteheads->item->object.sha1))
1320
- return 1;
1325
+ if (checkout_fast_forward(head_commit->object.sha1,
1326
+ remoteheads->item->object.sha1)) {
1327
+ ret = 1;
1328
+ goto done;
1329
+ }
1330
1331
finish(head_commit, o->sha1, msg.buf);
1332
drop_save();
1324
- return 0;
1333
+ goto done;
1334
} else if (!remoteheads->next && common->next)
1335
;
1336
/*
@@ -1339,8 +1348,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
1348
git_committer_info(IDENT_ERROR_ON_NO_NAME);
1349
printf(_("Trying really trivial in-index merge...\n"));
1350
if (!read_tree_trivial(common->item->object.sha1,
1342
- head_commit->object.sha1, remoteheads->item->object.sha1))
1343
- return merge_trivial(head_commit);
1351
+ head_commit->object.sha1,
1352
+ remoteheads->item->object.sha1)) {
1353
+ ret = merge_trivial(head_commit);
1354
+ goto done;
1355
+ }
1356
printf(_("Nope.\n"));
1357
}
1358
} else {
@@ -1368,7 +1380,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
1380
}
1381
if (up_to_date) {
1382
finish_up_to_date("Already up-to-date. Yeeah!");
1371
- return 0;
1383
+ goto done;
1384
}
1385
}
1386
@@ -1450,9 +1462,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
1462
* If we have a resulting tree, that means the strategy module
1463
* auto resolved the merge cleanly.
1464
*/
1453
- if (automerge_was_ok)
1454
- return finish_automerge(head_commit, common, result_tree,
1455
- wt_strategy);
1465
+ if (automerge_was_ok) {
1466
+ ret = finish_automerge(head_commit, common, result_tree,
1467
+ wt_strategy);
1468
+ goto done;
1469
+ }
1470
1471
/*
1472
* Pick the result from the best strategy and have the user fix
@@ -1466,7 +1480,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
1480
else
1481
fprintf(stderr, _("Merge with strategy %s failed.\n"),
1482
use_strategies[0]->name);
1469
- return 2;
1483
+ ret = 2;
1484
+ goto done;
1485
} else if (best_strategy == wt_strategy)
1486
; /* We already have its result in the working tree. */
1487
else {
@@ -1482,10 +1497,13 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
1497
else
1498
write_merge_state();
1499
1485
- if (merge_was_ok) {
1500
+ if (merge_was_ok)
1501
fprintf(stderr, _("Automatic merge went well; "
1502
"stopped before committing as requested\n"));
1488
- return 0;
1489
- } else
1490
- return suggest_conflicts(option_renormalize);
1503
+ else
1504
+ ret = suggest_conflicts(option_renormalize);
1505
+
1506
+done:
1507
+ free((char *)branch);
1508
+ return ret;
1509
}
builtin/notes.c
+5
-1
@@ -804,6 +804,7 @@ static int merge_commit(struct notes_merge_options *o)
804
struct notes_tree *t;
805
struct commit *partial;
806
struct pretty_print_context pretty_ctx;
807
+ int ret;
808
809
/*
810
* Read partial merge result from .git/NOTES_MERGE_PARTIAL,
@@ -828,6 +829,7 @@ static int merge_commit(struct notes_merge_options *o)
829
o->local_ref = resolve_ref("NOTES_MERGE_REF", sha1, 0, NULL);
830
if (!o->local_ref)
831
die("Failed to resolve NOTES_MERGE_REF");
832
+ o->local_ref = xstrdup(o->local_ref);
833
834
if (notes_merge_commit(o, t, partial, sha1))
835
die("Failed to finalize notes merge");
@@ -843,7 +845,9 @@ static int merge_commit(struct notes_merge_options *o)
845
846
free_notes(t);
847
strbuf_release(&msg);
846
- return merge_abort(o);
848
+ ret = merge_abort(o);
849
+ free((char *)o->local_ref);
850
+ return ret;
851
}
852
853
static int merge(int argc, const char **argv, const char *prefix)
builtin/receive-pack.c
+3
@@ -695,7 +695,10 @@ static void execute_commands(struct command *commands, const char *unpacker_erro
695
696
check_aliased_updates(commands);
697
698
+ free((char *)head_name);
699
head_name = resolve_ref("HEAD", sha1, 0, NULL);
700
+ if (head_name)
701
+ head_name = xstrdup(head_name);
702
703
for (cmd = commands; cmd; cmd = cmd->next)
704
if (!cmd->skip_update)
reflog-walk.c
+4
-1
@@ -51,8 +51,11 @@ static struct complete_reflogs *read_complete_reflog(const char *ref)
51
if (reflogs->nr == 0) {
52
unsigned char sha1[20];
53
const char *name = resolve_ref(ref, sha1, 1, NULL);
54
- if (name)
54
+ if (name) {
55
+ name = xstrdup(name);
56
for_each_reflog_ent(name, read_one_reflog, reflogs);
57
+ free((char *)name);
58
+ }
59
}
60
if (reflogs->nr == 0) {
61
int len = strlen(ref);