From e4776bd936aa162b7f00cb26260dc4a6ca444abb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Mon, 12 Dec 2011 18:20:29 +0700 Subject: revert: convert resolve_ref() to read_ref_full() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is the follow up of c689332 (Convert many resolve_ref() calls to read_ref*() and ref_exists() - 2011-11-13). See the said commit for rationale. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- builtin/revert.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'builtin') diff --git a/builtin/revert.c b/builtin/revert.c index 1ea525c10..0c52a8339 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -901,7 +901,7 @@ static int rollback_single_pick(void) if (!file_exists(git_path("CHERRY_PICK_HEAD")) && !file_exists(git_path("REVERT_HEAD"))) return error(_("no cherry-pick or revert in progress")); - if (!resolve_ref("HEAD", head_sha1, 0, NULL)) + if (read_ref_full("HEAD", head_sha1, 0, NULL)) return error(_("cannot resolve HEAD")); if (is_null_sha1(head_sha1)) return error(_("cannot abort from a branch yet to be born")); -- cgit v1.2.1 From 96ec7b1e708863d0cd6b8e72a986d6f0a1bb64db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Tue, 13 Dec 2011 21:17:48 +0700 Subject: Convert resolve_ref+xstrdup to new resolve_refdup function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- builtin/branch.c | 12 +++++------- builtin/checkout.c | 15 +++++++-------- builtin/fmt-merge-msg.c | 7 ++++--- builtin/for-each-ref.c | 7 ++----- builtin/merge.c | 12 +++++------- builtin/notes.c | 7 ++++--- builtin/receive-pack.c | 7 +++---- builtin/show-branch.c | 4 +--- 8 files changed, 31 insertions(+), 40 deletions(-) (limited to 'builtin') diff --git a/builtin/branch.c b/builtin/branch.c index e1e486e4c..c459f0bab 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -104,6 +104,7 @@ static int branch_merged(int kind, const char *name, */ struct commit *reference_rev = NULL; const char *reference_name = NULL; + void *reference_name_to_free = NULL; int merged; if (kind == REF_LOCAL_BRANCH) { @@ -114,11 +115,9 @@ static int branch_merged(int kind, const char *name, branch->merge && branch->merge[0] && branch->merge[0]->dst && - (reference_name = - resolve_ref(branch->merge[0]->dst, sha1, 1, NULL)) != NULL) { - reference_name = xstrdup(reference_name); + (reference_name = reference_name_to_free = + resolve_refdup(branch->merge[0]->dst, sha1, 1, NULL)) != NULL) reference_rev = lookup_commit_reference(sha1); - } } if (!reference_rev) reference_rev = head_rev; @@ -143,7 +142,7 @@ static int branch_merged(int kind, const char *name, " '%s', even though it is merged to HEAD."), name, reference_name); } - free((char *)reference_name); + free(reference_name_to_free); return merged; } @@ -731,10 +730,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix) track = git_branch_track; - head = resolve_ref("HEAD", head_sha1, 0, NULL); + head = resolve_refdup("HEAD", head_sha1, 0, NULL); if (!head) die(_("Failed to resolve HEAD as a valid ref.")); - head = xstrdup(head); if (!strcmp(head, "HEAD")) { detached = 1; } else { diff --git a/builtin/checkout.c b/builtin/checkout.c index b7c630287..00740bd67 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -696,17 +696,14 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new) { int ret = 0; struct branch_info old; + void *path_to_free; unsigned char rev[20]; int flag; memset(&old, 0, sizeof(old)); - old.path = resolve_ref("HEAD", rev, 0, &flag); - if (old.path) - old.path = xstrdup(old.path); + old.path = path_to_free = resolve_refdup("HEAD", rev, 0, &flag); old.commit = lookup_commit_reference_gently(rev, 1); - if (!(flag & REF_ISSYMREF)) { - free((char *)old.path); + if (!(flag & REF_ISSYMREF)) old.path = NULL; - } if (old.path && !prefixcmp(old.path, "refs/heads/")) old.name = old.path + strlen("refs/heads/"); @@ -720,8 +717,10 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new) } ret = merge_working_tree(opts, &old, new); - if (ret) + if (ret) { + free(path_to_free); return ret; + } if (!opts->quiet && !old.path && old.commit && new->commit != old.commit) orphaned_commit_warning(old.commit); @@ -729,7 +728,7 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new) update_refs_for_switch(opts, &old, new); ret = post_checkout_hook(old.commit, new->commit, 1); - free((char *)old.path); + free(path_to_free); return ret || opts->writeout_error; } diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c index bdfa0ea05..c81a7fef2 100644 --- a/builtin/fmt-merge-msg.c +++ b/builtin/fmt-merge-msg.c @@ -372,14 +372,15 @@ int fmt_merge_msg(struct strbuf *in, struct strbuf *out, int i = 0, pos = 0; unsigned char head_sha1[20]; const char *current_branch; + void *current_branch_to_free; /* get current branch */ - current_branch = resolve_ref("HEAD", head_sha1, 1, NULL); + current_branch = current_branch_to_free = + resolve_refdup("HEAD", head_sha1, 1, NULL); if (!current_branch) 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) { @@ -421,7 +422,7 @@ int fmt_merge_msg(struct strbuf *in, struct strbuf *out, } strbuf_complete_line(out); - free((char *)current_branch); + free(current_branch_to_free); return 0; } diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c index d90e5d2b2..b01d76a24 100644 --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -628,11 +628,8 @@ static void populate_value(struct refinfo *ref) if (need_symref && (ref->flag & REF_ISSYMREF) && !ref->symref) { unsigned char unused1[20]; - const char *symref; - symref = resolve_ref(ref->refname, unused1, 1, NULL); - if (symref) - ref->symref = xstrdup(symref); - else + ref->symref = resolve_refdup(ref->refname, unused1, 1, NULL); + if (!ref->symref) ref->symref = ""; } diff --git a/builtin/merge.c b/builtin/merge.c index a1c85344b..a3cd5e856 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -1096,6 +1096,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix) struct commit_list *common = NULL; const char *best_strategy = NULL, *wt_strategy = NULL; struct commit_list **remotes = &remoteheads; + void *branch_to_free; if (argc == 2 && !strcmp(argv[1], "-h")) usage_with_options(builtin_merge_usage, builtin_merge_options); @@ -1104,12 +1105,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix) * Check if we are _not_ on a detached HEAD, i.e. if there is a * current branch. */ - branch = resolve_ref("HEAD", head_sha1, 0, &flag); - if (branch) { - if (!prefixcmp(branch, "refs/heads/")) - branch += 11; - branch = xstrdup(branch); - } + branch = branch_to_free = resolve_refdup("HEAD", head_sha1, 0, &flag); + if (branch && !prefixcmp(branch, "refs/heads/")) + branch += 11; if (!branch || is_null_sha1(head_sha1)) head_commit = NULL; else @@ -1520,6 +1518,6 @@ int cmd_merge(int argc, const char **argv, const char *prefix) ret = suggest_conflicts(option_renormalize); done: - free((char *)branch); + free(branch_to_free); return ret; } diff --git a/builtin/notes.c b/builtin/notes.c index 10b8bc7ad..667e20a1e 100644 --- a/builtin/notes.c +++ b/builtin/notes.c @@ -804,6 +804,7 @@ static int merge_commit(struct notes_merge_options *o) struct notes_tree *t; struct commit *partial; struct pretty_print_context pretty_ctx; + void *local_ref_to_free; int ret; /* @@ -826,10 +827,10 @@ static int merge_commit(struct notes_merge_options *o) t = xcalloc(1, sizeof(struct notes_tree)); init_notes(t, "NOTES_MERGE_PARTIAL", combine_notes_overwrite, 0); - o->local_ref = resolve_ref("NOTES_MERGE_REF", sha1, 0, NULL); + o->local_ref = local_ref_to_free = + resolve_refdup("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"); @@ -846,7 +847,7 @@ static int merge_commit(struct notes_merge_options *o) free_notes(t); strbuf_release(&msg); ret = merge_abort(o); - free((char *)o->local_ref); + free(local_ref_to_free); return ret; } diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index b6d957cb0..62afac3ec 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -37,6 +37,7 @@ static int prefer_ofs_delta = 1; static int auto_update_server_info; static int auto_gc = 1; static const char *head_name; +static void *head_name_to_free; static int sent_capabilities; static enum deny_action parse_deny_action(const char *var, const char *value) @@ -695,10 +696,8 @@ static void execute_commands(struct command *commands, const char *unpacker_erro check_aliased_updates(commands); - free((char *)head_name); - head_name = resolve_ref("HEAD", sha1, 0, NULL); - if (head_name) - head_name = xstrdup(head_name); + free(head_name_to_free); + head_name = head_name_to_free = resolve_refdup("HEAD", sha1, 0, NULL); for (cmd = commands; cmd; cmd = cmd->next) if (!cmd->skip_update) diff --git a/builtin/show-branch.c b/builtin/show-branch.c index 4b480d7c7..a1f148e48 100644 --- a/builtin/show-branch.c +++ b/builtin/show-branch.c @@ -726,10 +726,8 @@ int cmd_show_branch(int ac, const char **av, const char *prefix) if (ac == 0) { static const char *fake_av[2]; - const char *refname; - refname = resolve_ref("HEAD", sha1, 1, NULL); - fake_av[0] = xstrdup(refname); + fake_av[0] = resolve_refdup("HEAD", sha1, 1, NULL); fake_av[1] = NULL; av = fake_av; ac = 1; -- cgit v1.2.1 From 8cad4744ee37ebec1d9491a1381ec1771a1ba795 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Mon, 12 Dec 2011 18:20:32 +0700 Subject: Rename resolve_ref() to resolve_ref_unsafe() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolve_ref() may return a pointer to a shared buffer and can be overwritten by the next resolve_ref() calls. Callers need to pay attention, not to keep the pointer when the next call happens. Rename with "_unsafe" suffix to warn developers (or reviewers) before introducing new call sites. This patch is generated using the following command git grep -l 'resolve_ref(' -- '*.[ch]'|xargs sed -i 's/resolve_ref(/resolve_ref_unsafe(/g' Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- builtin/branch.c | 2 +- builtin/commit.c | 2 +- builtin/fsck.c | 2 +- builtin/log.c | 4 ++-- builtin/receive-pack.c | 2 +- builtin/remote.c | 2 +- builtin/show-branch.c | 2 +- builtin/symbolic-ref.c | 2 +- 8 files changed, 9 insertions(+), 9 deletions(-) (limited to 'builtin') diff --git a/builtin/branch.c b/builtin/branch.c index c459f0bab..642b5bd31 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -252,7 +252,7 @@ static char *resolve_symref(const char *src, const char *prefix) int flag; const char *dst, *cp; - dst = resolve_ref(src, sha1, 0, &flag); + dst = resolve_ref_unsafe(src, sha1, 0, &flag); if (!(dst && (flag & REF_ISSYMREF))) return NULL; if (prefix && (cp = skip_prefix(dst, prefix))) diff --git a/builtin/commit.c b/builtin/commit.c index e36e9adf8..4d39d2510 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1304,7 +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); + head = resolve_ref_unsafe("HEAD", junk_sha1, 0, NULL); printf("[%s%s ", !prefixcmp(head, "refs/heads/") ? head + 11 : diff --git a/builtin/fsck.c b/builtin/fsck.c index 30d0dc82f..8c479a791 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -563,7 +563,7 @@ static int fsck_head_link(void) if (verbose) fprintf(stderr, "Checking HEAD link\n"); - head_points_at = resolve_ref("HEAD", head_sha1, 0, &flag); + head_points_at = resolve_ref_unsafe("HEAD", head_sha1, 0, &flag); if (!head_points_at) return error("Invalid HEAD"); if (!strcmp(head_points_at, "HEAD")) diff --git a/builtin/log.c b/builtin/log.c index 4395f3e47..89d0cc013 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1040,7 +1040,7 @@ static char *find_branch_name(struct rev_info *rev) if (positive < 0) return NULL; strbuf_addf(&buf, "refs/heads/%s", rev->cmdline.rev[positive].name); - branch = resolve_ref(buf.buf, branch_sha1, 1, NULL); + branch = resolve_ref_unsafe(buf.buf, branch_sha1, 1, NULL); if (!branch || prefixcmp(branch, "refs/heads/") || hashcmp(rev->cmdline.rev[positive].item->sha1, branch_sha1)) @@ -1268,7 +1268,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) rev.pending.objects[0].item->flags |= UNINTERESTING; add_head_to_pending(&rev); - ref = resolve_ref("HEAD", sha1, 1, NULL); + ref = resolve_ref_unsafe("HEAD", sha1, 1, NULL); if (ref && !prefixcmp(ref, "refs/heads/")) branch_name = xstrdup(ref + strlen("refs/heads/")); else diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 62afac3ec..d2dcb7e4a 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -572,7 +572,7 @@ static void check_aliased_update(struct command *cmd, struct string_list *list) int flag; strbuf_addf(&buf, "%s%s", get_git_namespace(), cmd->ref_name); - dst_name = resolve_ref(buf.buf, sha1, 0, &flag); + dst_name = resolve_ref_unsafe(buf.buf, sha1, 0, &flag); strbuf_release(&buf); if (!(flag & REF_ISSYMREF)) diff --git a/builtin/remote.c b/builtin/remote.c index 407abfb0f..583eec90e 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -573,7 +573,7 @@ static int read_remote_branches(const char *refname, strbuf_addf(&buf, "refs/remotes/%s/", rename->old); if (!prefixcmp(refname, buf.buf)) { item = string_list_append(rename->remote_branches, xstrdup(refname)); - symref = resolve_ref(refname, orig_sha1, 1, &flag); + symref = resolve_ref_unsafe(refname, orig_sha1, 1, &flag); if (flag & REF_ISSYMREF) item->util = xstrdup(symref); else diff --git a/builtin/show-branch.c b/builtin/show-branch.c index a1f148e48..a59e088cf 100644 --- a/builtin/show-branch.c +++ b/builtin/show-branch.c @@ -789,7 +789,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix) } } - head_p = resolve_ref("HEAD", head_sha1, 1, NULL); + head_p = resolve_ref_unsafe("HEAD", head_sha1, 1, NULL); if (head_p) { head_len = strlen(head_p); memcpy(head, head_p, head_len + 1); diff --git a/builtin/symbolic-ref.c b/builtin/symbolic-ref.c index dea849c3c..2ef596238 100644 --- a/builtin/symbolic-ref.c +++ b/builtin/symbolic-ref.c @@ -12,7 +12,7 @@ static void check_symref(const char *HEAD, int quiet) { unsigned char sha1[20]; int flag; - const char *refs_heads_master = resolve_ref(HEAD, sha1, 0, &flag); + const char *refs_heads_master = resolve_ref_unsafe(HEAD, sha1, 0, &flag); if (!refs_heads_master) die("No such ref: %s", HEAD); -- cgit v1.2.1