Convert many resolve_ref() calls to read_ref*() and ref_exists()

resolve_ref() may return a pointer to a static buffer, which is not safe for long-term use because if another resolve_ref() call happens, the buffer may be changed. Many call sites though do not care about this buffer. They simply check if the return value is NULL or not. Convert all these call sites to new wrappers to reduce resolve_ref() calls from 57 to 34. If we change resolve_ref() prototype later on to avoid passing static buffer out, this helps reduce changes. 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 c6893323917cbf4cb66c29ba2ac03014a44f0f0c
12 files changed +36 -32
builtin/branch.c
+2 -3
@@ -186,7 +186,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)
186 free(name);
187
188 name = xstrdup(mkpath(fmt, bname.buf));
189 - if (!resolve_ref(name, sha1, 1, NULL)) {
189 + if (read_ref(name, sha1)) {
190 error(_("%sbranch '%s' not found."),
191 remote, bname.buf);
192 ret = 1;
@@ -565,7 +565,6 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru
565 static void rename_branch(const char *oldname, const char *newname, int force)
566 {
567 struct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = STRBUF_INIT;
568 - unsigned char sha1[20];
568 struct strbuf oldsection = STRBUF_INIT, newsection = STRBUF_INIT;
569 int recovery = 0;
570
@@ -577,7 +576,7 @@ static void rename_branch(const char *oldname, const char *newname, int force)
576 * Bad name --- this could be an attempt to rename a
577 * ref that we used to allow to be created by accident.
578 */
580 - if (resolve_ref(oldref.buf, sha1, 1, NULL))
579 + if (ref_exists(oldref.buf))
580 recovery = 1;
581 else
582 die(_("Invalid branch name: '%s'"), oldname);
builtin/checkout.c
+2 -2
@@ -288,7 +288,7 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,
288 commit_locked_index(lock_file))
289 die(_("unable to write new index file"));
290
291 - resolve_ref("HEAD", rev, 0, &flag);
291 + read_ref_full("HEAD", rev, 0, &flag);
292 head = lookup_commit_reference_gently(rev, 1);
293
294 errs |= post_checkout_hook(head, head, 0);
@@ -866,7 +866,7 @@ static int parse_branchname_arg(int argc, const char **argv,
866 setup_branch_path(new);
867
868 if (!check_refname_format(new->path, 0) &&
869 - resolve_ref(new->path, branch_rev, 1, NULL))
869 + !read_ref(new->path, branch_rev))
870 hashcpy(rev, branch_rev);
871 else
872 new->path = NULL; /* not an existing branch */
builtin/merge.c
+2 -2
@@ -420,7 +420,7 @@ static struct object *want_commit(const char *name)
420 static void merge_name(const char *remote, struct strbuf *msg)
421 {
422 struct object *remote_head;
423 - unsigned char branch_head[20], buf_sha[20];
423 + unsigned char branch_head[20];
424 struct strbuf buf = STRBUF_INIT;
425 struct strbuf bname = STRBUF_INIT;
426 const char *ptr;
@@ -479,7 +479,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
479 strbuf_addstr(&truname, "refs/heads/");
480 strbuf_addstr(&truname, remote);
481 strbuf_setlen(&truname, truname.len - len);
482 - if (resolve_ref(truname.buf, buf_sha, 1, NULL)) {
482 + if (ref_exists(truname.buf)) {
483 strbuf_addf(msg,
484 "%s\t\tbranch '%s'%s of .\n",
485 sha1_to_hex(remote_head->sha1),
builtin/remote.c
+3 -5
@@ -343,8 +343,7 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat
343 states->tracked.strdup_strings = 1;
344 states->stale.strdup_strings = 1;
345 for (ref = fetch_map; ref; ref = ref->next) {
346 - unsigned char sha1[20];
347 - if (!ref->peer_ref || read_ref(ref->peer_ref->name, sha1))
346 + if (!ref->peer_ref || !ref_exists(ref->peer_ref->name))
347 string_list_append(&states->new, abbrev_branch(ref->name));
348 else
349 string_list_append(&states->tracked, abbrev_branch(ref->name));
@@ -710,7 +709,7 @@ static int mv(int argc, const char **argv)
709 int flag = 0;
710 unsigned char sha1[20];
711
713 - resolve_ref(item->string, sha1, 1, &flag);
712 + read_ref_full(item->string, sha1, 1, &flag);
713 if (!(flag & REF_ISSYMREF))
714 continue;
715 if (delete_ref(item->string, NULL, REF_NODEREF))
@@ -1220,10 +1219,9 @@ static int set_head(int argc, const char **argv)
1219 usage_with_options(builtin_remote_sethead_usage, options);
1220
1221 if (head_name) {
1223 - unsigned char sha1[20];
1222 strbuf_addf(&buf2, "refs/remotes/%s/%s", argv[0], head_name);
1223 /* make sure it's valid */
1226 - if (!resolve_ref(buf2.buf, sha1, 1, NULL))
1224 + if (!ref_exists(buf2.buf))
1225 result |= error("Not a valid ref: %s", buf2.buf);
1226 else if (create_symref(buf.buf, buf2.buf, "remote set-head"))
1227 result |= error("Could not setup %s", buf.buf);
builtin/replace.c
+2 -2
@@ -58,7 +58,7 @@ static int for_each_replace_name(const char **argv, each_replace_name_fn fn)
58 had_error = 1;
59 continue;
60 }
61 - if (!resolve_ref(ref, sha1, 1, NULL)) {
61 + if (read_ref(ref, sha1)) {
62 error("replace ref '%s' not found.", *p);
63 had_error = 1;
64 continue;
@@ -97,7 +97,7 @@ static int replace_object(const char *object_ref, const char *replace_ref,
97 if (check_refname_format(ref, 0))
98 die("'%s' is not a valid ref name.", ref);
99
100 - if (!resolve_ref(ref, prev, 1, NULL))
100 + if (read_ref(ref, prev))
101 hashclr(prev);
102 else if (!force)
103 die("replace ref '%s' already exists", ref);
builtin/show-ref.c
+1 -1
@@ -225,7 +225,7 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
225 unsigned char sha1[20];
226
227 if (!prefixcmp(*pattern, "refs/") &&
228 - resolve_ref(*pattern, sha1, 1, NULL)) {
228 + !read_ref(*pattern, sha1)) {
229 if (!quiet)
230 show_one(*pattern, sha1);
231 }
builtin/tag.c
+2 -2
@@ -174,7 +174,7 @@ static int for_each_tag_name(const char **argv, each_tag_name_fn fn)
174 had_error = 1;
175 continue;
176 }
177 - if (!resolve_ref(ref, sha1, 1, NULL)) {
177 + if (read_ref(ref, sha1)) {
178 error(_("tag '%s' not found."), *p);
179 had_error = 1;
180 continue;
@@ -518,7 +518,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
518 if (strbuf_check_tag_ref(&ref, tag))
519 die(_("'%s' is not a valid tag name."), tag);
520
521 - if (!resolve_ref(ref.buf, prev, 1, NULL))
521 + if (read_ref(ref.buf, prev))
522 hashclr(prev);
523 else if (!force)
524 die(_("tag '%s' already exists"), tag);
bundle.c
+1 -1
@@ -320,7 +320,7 @@ int create_bundle(struct bundle_header *header, const char *path,
320 continue;
321 if (dwim_ref(e->name, strlen(e->name), sha1, &ref) != 1)
322 continue;
323 - if (!resolve_ref(e->name, sha1, 1, &flag))
323 + if (read_ref_full(e->name, sha1, 1, &flag))
324 flag = 0;
325 display_ref = (flag & REF_ISSYMREF) ? e->name : ref;
326
cache.h
+2
@@ -832,6 +832,8 @@ static inline int get_sha1_with_context(const char *str, unsigned char *sha1, st
832 extern int get_sha1_hex(const char *hex, unsigned char *sha1);
833
834 extern char *sha1_to_hex(const unsigned char *sha1); /* static buffer result! */
835 +extern int read_ref_full(const char *filename, unsigned char *sha1,
836 + int reading, int *flags);
837 extern int read_ref(const char *filename, unsigned char *sha1);
838
839 /*
notes-merge.c
+1 -1
@@ -568,7 +568,7 @@ int notes_merge(struct notes_merge_options *o,
568 o->local_ref, o->remote_ref);
569
570 /* Dereference o->local_ref into local_sha1 */
571 - if (!resolve_ref(o->local_ref, local_sha1, 0, NULL))
571 + if (read_ref_full(o->local_ref, local_sha1, 0, NULL))
572 die("Failed to resolve local notes ref '%s'", o->local_ref);
573 else if (!check_refname_format(o->local_ref, 0) &&
574 is_null_sha1(local_sha1))
refs.c
+16 -11
@@ -334,7 +334,7 @@ static void get_ref_dir(const char *submodule, const char *base,
334 flag |= REF_ISBROKEN;
335 }
336 } else
337 - if (!resolve_ref(ref, sha1, 1, &flag)) {
337 + if (read_ref_full(ref, sha1, 1, &flag)) {
338 hashclr(sha1);
339 flag |= REF_ISBROKEN;
340 }
@@ -612,13 +612,18 @@ struct ref_filter {
612 void *cb_data;
613 };
614
615 -int read_ref(const char *ref, unsigned char *sha1)
615 +int read_ref_full(const char *ref, unsigned char *sha1, int reading, int *flags)
616 {
617 - if (resolve_ref(ref, sha1, 1, NULL))
617 + if (resolve_ref(ref, sha1, reading, flags))
618 return 0;
619 return -1;
620 }
621
622 +int read_ref(const char *ref, unsigned char *sha1)
623 +{
624 + return read_ref_full(ref, sha1, 1, NULL);
625 +}
626 +
627 #define DO_FOR_EACH_INCLUDE_BROKEN 01
628 static int do_one_ref(const char *base, each_ref_fn fn, int trim,
629 int flags, void *cb_data, struct ref_entry *entry)
@@ -663,7 +668,7 @@ int peel_ref(const char *ref, unsigned char *sha1)
668 goto fallback;
669 }
670
666 - if (!resolve_ref(ref, base, 1, &flag))
671 + if (read_ref_full(ref, base, 1, &flag))
672 return -1;
673
674 if ((flag & REF_ISPACKED)) {
@@ -746,7 +751,7 @@ static int do_head_ref(const char *submodule, each_ref_fn fn, void *cb_data)
751 return 0;
752 }
753
749 - if (resolve_ref("HEAD", sha1, 1, &flag))
754 + if (!read_ref_full("HEAD", sha1, 1, &flag))
755 return fn("HEAD", sha1, flag, cb_data);
756
757 return 0;
@@ -826,7 +831,7 @@ int head_ref_namespaced(each_ref_fn fn, void *cb_data)
831 int flag;
832
833 strbuf_addf(&buf, "%sHEAD", get_git_namespace());
829 - if (resolve_ref(buf.buf, sha1, 1, &flag))
834 + if (!read_ref_full(buf.buf, sha1, 1, &flag))
835 ret = fn(buf.buf, sha1, flag, cb_data);
836 strbuf_release(&buf);
837
@@ -1022,7 +1027,7 @@ int refname_match(const char *abbrev_name, const char *full_name, const char **r
1027 static struct ref_lock *verify_lock(struct ref_lock *lock,
1028 const unsigned char *old_sha1, int mustexist)
1029 {
1025 - if (!resolve_ref(lock->ref_name, lock->old_sha1, mustexist, NULL)) {
1030 + if (read_ref_full(lock->ref_name, lock->old_sha1, mustexist, NULL)) {
1031 error("Can't verify ref %s", lock->ref_name);
1032 unlock_ref(lock);
1033 return NULL;
@@ -1377,7 +1382,8 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
1382 goto rollback;
1383 }
1384
1380 - if (resolve_ref(newref, sha1, 1, &flag) && delete_ref(newref, sha1, REF_NODEREF)) {
1385 + if (!read_ref_full(newref, sha1, 1, &flag) &&
1386 + delete_ref(newref, sha1, REF_NODEREF)) {
1387 if (errno==EISDIR) {
1388 if (remove_empty_directories(git_path("%s", newref))) {
1389 error("Directory not empty: %s", newref);
@@ -1929,7 +1935,7 @@ static int do_for_each_reflog(const char *base, each_ref_fn fn, void *cb_data)
1935 retval = do_for_each_reflog(log, fn, cb_data);
1936 } else {
1937 unsigned char sha1[20];
1932 - if (!resolve_ref(log, sha1, 0, NULL))
1938 + if (read_ref_full(log, sha1, 0, NULL))
1939 retval = error("bad ref for %s", log);
1940 else
1941 retval = fn(log, sha1, 0, cb_data);
@@ -2072,7 +2078,6 @@ char *shorten_unambiguous_ref(const char *ref, int strict)
2078 */
2079 for (j = 0; j < rules_to_fail; j++) {
2080 const char *rule = ref_rev_parse_rules[j];
2075 - unsigned char short_objectname[20];
2081 char refname[PATH_MAX];
2082
2083 /* skip matched rule */
@@ -2086,7 +2091,7 @@ char *shorten_unambiguous_ref(const char *ref, int strict)
2091 */
2092 mksnpath(refname, sizeof(refname),
2093 rule, short_name_len, short_name);
2089 - if (!read_ref(refname, short_objectname))
2094 + if (ref_exists(refname))
2095 break;
2096 }
2097
remote.c
+2 -2
@@ -1507,13 +1507,13 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs)
1507 * nothing to report.
1508 */
1509 base = branch->merge[0]->dst;
1510 - if (!resolve_ref(base, sha1, 1, NULL))
1510 + if (read_ref(base, sha1))
1511 return 0;
1512 theirs = lookup_commit_reference(sha1);
1513 if (!theirs)
1514 return 0;
1515
1516 - if (!resolve_ref(branch->refname, sha1, 1, NULL))
1516 + if (read_ref(branch->refname, sha1))
1517 return 0;
1518 ours = lookup_commit_reference(sha1);
1519 if (!ours)