Skip to content

Commit d5a35c1

Browse files
pcloudsgitster
pclouds
authored and
gitster
committed
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>
1 parent c689332 commit d5a35c1

File tree

8 files changed

+66
-28
lines changed

8 files changed

+66
-28
lines changed

‎builtin/branch.c‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,10 @@ static int branch_merged(int kind, const char *name,
115115
branch->merge[0] &&
116116
branch->merge[0]->dst &&
117117
(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);
119120
reference_rev = lookup_commit_reference(sha1);
121+
}
120122
}
121123
if (!reference_rev)
122124
reference_rev = head_rev;
@@ -141,6 +143,7 @@ static int branch_merged(int kind, const char *name,
141143
" '%s', even though it is merged to HEAD."),
142144
name, reference_name);
143145
}
146+
free((char *)reference_name);
144147
return merged;
145148
}
146149

‎builtin/checkout.c‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -699,7 +699,9 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
699699
unsigned char rev[20];
700700
int flag;
701701
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);
703705
old.commit = lookup_commit_reference_gently(rev, 1);
704706
if (!(flag & REF_ISSYMREF)) {
705707
free((char *)old.path);

‎builtin/commit.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1259,7 +1259,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1,
12591259
struct commit *commit;
12601260
struct strbuf format = STRBUF_INIT;
12611261
unsigned char junk_sha1[20];
1262-
const char *head = resolve_ref("HEAD", junk_sha1, 0, NULL);
1262+
const char *head;
12631263
struct pretty_print_context pctx = {0};
12641264
struct strbuf author_ident = STRBUF_INIT;
12651265
struct strbuf committer_ident = STRBUF_INIT;
@@ -1304,6 +1304,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1,
13041304
rev.diffopt.break_opt = 0;
13051305
diff_setup_done(&rev.diffopt);
13061306

1307+
head = resolve_ref("HEAD", junk_sha1, 0, NULL);
13071308
printf("[%s%s ",
13081309
!prefixcmp(head, "refs/heads/") ?
13091310
head + 11 :

‎builtin/fmt-merge-msg.c‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,7 @@ static int do_fmt_merge_msg(int merge_title, struct strbuf *in,
268268
die("No current branch");
269269
if (!prefixcmp(current_branch, "refs/heads/"))
270270
current_branch += 11;
271+
current_branch = xstrdup(current_branch);
271272

272273
/* get a line */
273274
while (pos < in->len) {
@@ -283,8 +284,10 @@ static int do_fmt_merge_msg(int merge_title, struct strbuf *in,
283284
die ("Error in line %d: %.*s", i, len, p);
284285
}
285286

286-
if (!srcs.nr)
287+
if (!srcs.nr) {
288+
free((char*)current_branch);
287289
return 0;
290+
}
288291

289292
if (merge_title)
290293
do_fmt_merge_msg_title(out, current_branch);
@@ -306,6 +309,7 @@ static int do_fmt_merge_msg(int merge_title, struct strbuf *in,
306309
shortlog(origins.items[i].string, origins.items[i].util,
307310
head, &rev, shortlog_len, out);
308311
}
312+
free((char *)current_branch);
309313
return 0;
310314
}
311315

‎builtin/merge.c‎

Lines changed: 40 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1082,7 +1082,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
10821082
struct commit *head_commit;
10831083
struct strbuf buf = STRBUF_INIT;
10841084
const char *head_arg;
1085-
int flag, i;
1085+
int flag, i, ret = 0;
10861086
int best_cnt = -1, merge_was_ok = 0, automerge_was_ok = 0;
10871087
struct commit_list *common = NULL;
10881088
const char *best_strategy = NULL, *wt_strategy = NULL;
@@ -1096,8 +1096,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
10961096
* current branch.
10971097
*/
10981098
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+
}
11011104
if (!branch || is_null_sha1(head_sha1))
11021105
head_commit = NULL;
11031106
else
@@ -1121,7 +1124,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
11211124
die(_("There is no merge to abort (MERGE_HEAD missing)."));
11221125

11231126
/* Invoke 'git reset --merge' */
1124-
return cmd_reset(nargc, nargv, prefix);
1127+
ret = cmd_reset(nargc, nargv, prefix);
1128+
goto done;
11251129
}
11261130

11271131
if (read_cache_unmerged())
@@ -1205,7 +1209,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
12051209
read_empty(remote_head->sha1, 0);
12061210
update_ref("initial pull", "HEAD", remote_head->sha1, NULL, 0,
12071211
DIE_ON_ERR);
1208-
return 0;
1212+
goto done;
12091213
} else {
12101214
struct strbuf merge_names = STRBUF_INIT;
12111215

@@ -1292,7 +1296,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
12921296
* but first the most common case of merging one remote.
12931297
*/
12941298
finish_up_to_date("Already up-to-date.");
1295-
return 0;
1299+
goto done;
12961300
} else if (allow_fast_forward && !remoteheads->next &&
12971301
!common->next &&
12981302
!hashcmp(common->item->object.sha1, head_commit->object.sha1)) {
@@ -1313,15 +1317,20 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
13131317
strbuf_addstr(&msg,
13141318
" (no commit created; -m option ignored)");
13151319
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+
}
13181324

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+
}
13211330

13221331
finish(head_commit, o->sha1, msg.buf);
13231332
drop_save();
1324-
return 0;
1333+
goto done;
13251334
} else if (!remoteheads->next && common->next)
13261335
;
13271336
/*
@@ -1339,8 +1348,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
13391348
git_committer_info(IDENT_ERROR_ON_NO_NAME);
13401349
printf(_("Trying really trivial in-index merge...\n"));
13411350
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+
}
13441356
printf(_("Nope.\n"));
13451357
}
13461358
} else {
@@ -1368,7 +1380,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
13681380
}
13691381
if (up_to_date) {
13701382
finish_up_to_date("Already up-to-date. Yeeah!");
1371-
return 0;
1383+
goto done;
13721384
}
13731385
}
13741386

@@ -1450,9 +1462,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
14501462
* If we have a resulting tree, that means the strategy module
14511463
* auto resolved the merge cleanly.
14521464
*/
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+
}
14561470

14571471
/*
14581472
* 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)
14661480
else
14671481
fprintf(stderr, _("Merge with strategy %s failed.\n"),
14681482
use_strategies[0]->name);
1469-
return 2;
1483+
ret = 2;
1484+
goto done;
14701485
} else if (best_strategy == wt_strategy)
14711486
; /* We already have its result in the working tree. */
14721487
else {
@@ -1482,10 +1497,13 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
14821497
else
14831498
write_merge_state();
14841499

1485-
if (merge_was_ok) {
1500+
if (merge_was_ok)
14861501
fprintf(stderr, _("Automatic merge went well; "
14871502
"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;
14911509
}

‎builtin/notes.c‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -804,6 +804,7 @@ static int merge_commit(struct notes_merge_options *o)
804804
struct notes_tree *t;
805805
struct commit *partial;
806806
struct pretty_print_context pretty_ctx;
807+
int ret;
807808

808809
/*
809810
* Read partial merge result from .git/NOTES_MERGE_PARTIAL,
@@ -828,6 +829,7 @@ static int merge_commit(struct notes_merge_options *o)
828829
o->local_ref = resolve_ref("NOTES_MERGE_REF", sha1, 0, NULL);
829830
if (!o->local_ref)
830831
die("Failed to resolve NOTES_MERGE_REF");
832+
o->local_ref = xstrdup(o->local_ref);
831833

832834
if (notes_merge_commit(o, t, partial, sha1))
833835
die("Failed to finalize notes merge");
@@ -843,7 +845,9 @@ static int merge_commit(struct notes_merge_options *o)
843845

844846
free_notes(t);
845847
strbuf_release(&msg);
846-
return merge_abort(o);
848+
ret = merge_abort(o);
849+
free((char *)o->local_ref);
850+
return ret;
847851
}
848852

849853
static int merge(int argc, const char **argv, const char *prefix)

‎builtin/receive-pack.c‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -695,7 +695,10 @@ static void execute_commands(struct command *commands, const char *unpacker_erro
695695

696696
check_aliased_updates(commands);
697697

698+
free((char *)head_name);
698699
head_name = resolve_ref("HEAD", sha1, 0, NULL);
700+
if (head_name)
701+
head_name = xstrdup(head_name);
699702

700703
for (cmd = commands; cmd; cmd = cmd->next)
701704
if (!cmd->skip_update)

‎reflog-walk.c‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,11 @@ static struct complete_reflogs *read_complete_reflog(const char *ref)
5151
if (reflogs->nr == 0) {
5252
unsigned char sha1[20];
5353
const char *name = resolve_ref(ref, sha1, 1, NULL);
54-
if (name)
54+
if (name) {
55+
name = xstrdup(name);
5556
for_each_reflog_ent(name, read_one_reflog, reflogs);
57+
free((char *)name);
58+
}
5659
}
5760
if (reflogs->nr == 0) {
5861
int len = strlen(ref);

0 commit comments

Comments
 (0)