From: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
"Tony Wang" <wwwjfy@gmail.com>,
"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: [PATCH] Copy resolve_ref() return value for longer use
Date: Tue, 8 Nov 2011 09:30:28 +0700 [thread overview]
Message-ID: <1320719428-1802-1-git-send-email-pclouds@gmail.com> (raw)
In-Reply-To: <CACsJy8Aw4_O_GMiK_D=HTENfxGUXTTzCHqM7FkriqU+mQtVWtA@mail.gmail.com>
resolve_ref() may return a pointer to a static buffer. Callers that
use this value outside of a block 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
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/checkout.c | 4 +++-
builtin/commit.c | 3 ++-
builtin/fmt-merge-msg.c | 1 +
builtin/merge.c | 7 +++++--
builtin/notes.c | 1 +
builtin/receive-pack.c | 2 ++
6 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 2a80772..98ddbcd 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -699,7 +699,9 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
unsigned char rev[20];
int flag;
memset(&old, 0, sizeof(old));
- old.path = xstrdup(resolve_ref("HEAD", rev, 0, &flag));
+ old.path = resolve_ref("HEAD", rev, 0, &flag);
+ if (old.path)
+ old.path = xstrdup(old.path);
old.commit = lookup_commit_reference_gently(rev, 1);
if (!(flag & REF_ISSYMREF)) {
free((char *)old.path);
diff --git a/builtin/commit.c b/builtin/commit.c
index c46f2d1..f3a6ed2 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1259,7 +1259,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1,
struct commit *commit;
struct strbuf format = STRBUF_INIT;
unsigned char junk_sha1[20];
- const char *head = resolve_ref("HEAD", junk_sha1, 0, NULL);
+ const char *head;
struct pretty_print_context pctx = {0};
struct strbuf author_ident = STRBUF_INIT;
struct strbuf committer_ident = STRBUF_INIT;
@@ -1304,6 +1304,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1,
rev.diffopt.break_opt = 0;
diff_setup_done(&rev.diffopt);
+ head = resolve_ref("HEAD", junk_sha1, 0, NULL);
printf("[%s%s ",
!prefixcmp(head, "refs/heads/") ?
head + 11 :
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 7e2f225..cab50e0 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -268,6 +268,7 @@ static int do_fmt_merge_msg(int merge_title, struct strbuf *in,
die("No current branch");
if (!prefixcmp(current_branch, "refs/heads/"))
current_branch += 11;
+ current_branch = xstrdup(current_branch);
/* get a line */
while (pos < in->len) {
diff --git a/builtin/merge.c b/builtin/merge.c
index dffd5ec..6865cb7 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1096,8 +1096,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
* current branch.
*/
branch = resolve_ref("HEAD", head_sha1, 0, &flag);
- if (branch && !prefixcmp(branch, "refs/heads/"))
- branch += 11;
+ if (branch) {
+ if (!prefixcmp(branch, "refs/heads/"))
+ branch += 11;
+ branch = xstrdup(branch);
+ }
if (!branch || is_null_sha1(head_sha1))
head_commit = NULL;
else
diff --git a/builtin/notes.c b/builtin/notes.c
index f8e437d..c6e4c86 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -828,6 +828,7 @@ static int merge_commit(struct notes_merge_options *o)
o->local_ref = resolve_ref("NOTES_MERGE_REF", sha1, 0, NULL);
if (!o->local_ref)
die("Failed to resolve NOTES_MERGE_REF");
+ o->local_ref = xstrdup(o->local_ref);
if (notes_merge_commit(o, t, partial, sha1))
die("Failed to finalize notes merge");
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 7ec68a1..6065bf0 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -696,6 +696,8 @@ static void execute_commands(struct command *commands, const char *unpacker_erro
check_aliased_updates(commands);
head_name = resolve_ref("HEAD", sha1, 0, NULL);
+ if (head_name)
+ head_name = xstrdup(head_name);
for (cmd = commands; cmd; cmd = cmd->next)
if (!cmd->skip_update)
--
1.7.3.1.256.g2539c.dirty
next prev parent reply other threads:[~2011-11-08 2:30 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <BC404302028E4B6F8F2C27DC8E63545F@gmail.com>
2011-11-07 9:30 ` git bug(?) for commit baf18fc261ca475343fe3cb9cd2c0dded4bc1bb7 Nguyen Thai Ngoc Duy
2011-11-07 9:48 ` Tony Wang
2011-11-07 10:41 ` Nguyen Thai Ngoc Duy
2011-11-07 11:02 ` Tony Wang
2011-11-07 11:21 ` Nguyen Thai Ngoc Duy
2011-11-08 2:30 ` Nguyễn Thái Ngọc Duy [this message]
2011-11-13 5:57 ` [PATCH] Copy resolve_ref() return value for longer use Junio C Hamano
2011-11-13 7:09 ` Nguyen Thai Ngoc Duy
2011-11-13 7:59 ` Junio C Hamano
2011-11-13 10:22 ` [PATCH 1/2] Convert many resolve_ref() calls to read_ref*() and ref_exists() Nguyễn Thái Ngọc Duy
2011-11-13 10:22 ` [PATCH 2/2] Copy resolve_ref() return value for longer use Nguyễn Thái Ngọc Duy
2011-11-13 20:41 ` Junio C Hamano
2011-11-14 3:32 ` Nguyen Thai Ngoc Duy
2011-11-14 4:03 ` Junio C Hamano
2011-11-14 11:24 ` Jeff King
2011-11-15 6:06 ` Nguyen Thai Ngoc Duy
2011-11-15 6:07 ` [PATCH 01/10] Allow resolve_ref() caller to decide whether to receive static buffer Nguyễn Thái Ngọc Duy
2011-11-15 6:07 ` [PATCH 02/10] cmd_merge: convert to single exit point Nguyễn Thái Ngọc Duy
2011-11-15 6:07 ` [PATCH 03/10] merge: do not point "branch" to a resolve_ref()'s static buffer Nguyễn Thái Ngọc Duy
2011-11-15 6:07 ` [PATCH 04/10] commit: move resolve_ref() closer to where the return value is used Nguyễn Thái Ngọc Duy
2011-11-15 6:07 ` [PATCH 05/10] checkout: do not try xstrdup() on NULL Nguyễn Thái Ngọc Duy
2011-11-15 6:07 ` [PATCH 06/10] reflog-walk.c: request allocated buffer from resolve_ref() Nguyễn Thái Ngọc Duy
2011-11-15 6:07 ` [PATCH 07/10] receive-pack: request resolve_ref() to allocate new buffer Nguyễn Thái Ngọc Duy
2011-11-15 6:07 ` [PATCH 08/10] notes: " Nguyễn Thái Ngọc Duy
2011-11-15 6:07 ` [PATCH 09/10] fmt-merge-msg: " Nguyễn Thái Ngọc Duy
2011-11-15 6:07 ` [PATCH 10/10] branch: " Nguyễn Thái Ngọc Duy
2011-11-15 7:09 ` [PATCH 01/10] Allow resolve_ref() caller to decide whether to receive static buffer Junio C Hamano
2011-11-13 20:30 ` [PATCH 1/2] Convert many resolve_ref() calls to read_ref*() and ref_exists() Junio C Hamano
2011-12-10 3:43 ` [PATCH] Copy resolve_ref() return value for longer use Tony Wang
2011-12-10 4:48 ` Nguyen Thai Ngoc Duy
2011-12-11 2:28 ` Tony Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1320719428-1802-1-git-send-email-pclouds@gmail.com \
--to=pclouds@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=wwwjfy@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.