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>

diff --git a/builtin/branch.c b/builtin/branch.c
index 51ca6a02d7433f1394de10a7f6ef1aa565ac03e2..0fe9c4dfe205ee8a41733162f26f0fa96a18e56a 100644 (file)
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -186,7 +186,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)
                free(name);
 
                name = xstrdup(mkpath(fmt, bname.buf));
                free(name);
 
                name = xstrdup(mkpath(fmt, bname.buf));

-               if (!resolve_ref(name, sha1, 1, NULL)) {
+               if (read_ref(name, sha1)) {

                        error(_("%sbranch '%s' not found."),
                                        remote, bname.buf);
                        ret = 1;
                        error(_("%sbranch '%s' not found."),
                                        remote, bname.buf);
                        ret = 1;


@@ -565,7 +565,6 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru
 static void rename_branch(const char *oldname, const char *newname, int force)
 {
        struct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = STRBUF_INIT;
 static void rename_branch(const char *oldname, const char *newname, int force)
 {
        struct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = STRBUF_INIT;

-       unsigned char sha1[20];

        struct strbuf oldsection = STRBUF_INIT, newsection = STRBUF_INIT;
        int recovery = 0;
 
        struct strbuf oldsection = STRBUF_INIT, newsection = STRBUF_INIT;
        int recovery = 0;
 


@@ -577,7 +576,7 @@ static void rename_branch(const char *oldname, const char *newname, int force)
                 * Bad name --- this could be an attempt to rename a
                 * ref that we used to allow to be created by accident.
                 */
                 * Bad name --- this could be an attempt to rename a
                 * ref that we used to allow to be created by accident.
                 */

-               if (resolve_ref(oldref.buf, sha1, 1, NULL))
+               if (ref_exists(oldref.buf))

                        recovery = 1;
                else
                        die(_("Invalid branch name: '%s'"), oldname);
                        recovery = 1;
                else
                        die(_("Invalid branch name: '%s'"), oldname);

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 2a8077242500d54ac50d5829a86b14803fc69126..beeaee41130e21ecbd2622e7ed5e632c94167beb 100644 (file)
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -288,7 +288,7 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,
            commit_locked_index(lock_file))
                die(_("unable to write new index file"));
 
            commit_locked_index(lock_file))
                die(_("unable to write new index file"));
 

-       resolve_ref("HEAD", rev, 0, &flag);
+       read_ref_full("HEAD", rev, 0, &flag);

        head = lookup_commit_reference_gently(rev, 1);
 
        errs |= post_checkout_hook(head, head, 0);
        head = lookup_commit_reference_gently(rev, 1);
 
        errs |= post_checkout_hook(head, head, 0);


@@ -866,7 +866,7 @@ static int parse_branchname_arg(int argc, const char **argv,
        setup_branch_path(new);
 
        if (!check_refname_format(new->path, 0) &&
        setup_branch_path(new);
 
        if (!check_refname_format(new->path, 0) &&

-           resolve_ref(new->path, branch_rev, 1, NULL))
+           !read_ref(new->path, branch_rev))

                hashcpy(rev, branch_rev);
        else
                new->path = NULL; /* not an existing branch */
                hashcpy(rev, branch_rev);
        else
                new->path = NULL; /* not an existing branch */

diff --git a/builtin/merge.c b/builtin/merge.c
index dffd5ec1245865259fd4e72cc304286a1a5b4633..42b4f9e7bcbf542f9726fd518c5ec027f13b6394 100644 (file)
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -420,7 +420,7 @@ static struct object *want_commit(const char *name)
 static void merge_name(const char *remote, struct strbuf *msg)
 {
        struct object *remote_head;
 static void merge_name(const char *remote, struct strbuf *msg)
 {
        struct object *remote_head;

-       unsigned char branch_head[20], buf_sha[20];
+       unsigned char branch_head[20];

        struct strbuf buf = STRBUF_INIT;
        struct strbuf bname = STRBUF_INIT;
        const char *ptr;
        struct strbuf buf = STRBUF_INIT;
        struct strbuf bname = STRBUF_INIT;
        const char *ptr;


@@ -479,7 +479,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
                strbuf_addstr(&truname, "refs/heads/");
                strbuf_addstr(&truname, remote);
                strbuf_setlen(&truname, truname.len - len);
                strbuf_addstr(&truname, "refs/heads/");
                strbuf_addstr(&truname, remote);
                strbuf_setlen(&truname, truname.len - len);

-               if (resolve_ref(truname.buf, buf_sha, 1, NULL)) {
+               if (ref_exists(truname.buf)) {

                        strbuf_addf(msg,
                                    "%s\t\tbranch '%s'%s of .\n",
                                    sha1_to_hex(remote_head->sha1),
                        strbuf_addf(msg,
                                    "%s\t\tbranch '%s'%s of .\n",
                                    sha1_to_hex(remote_head->sha1),

diff --git a/builtin/remote.c b/builtin/remote.c
index c810643815e38f4ada805e2af49973e4e0051801..407abfb0f7545374f1d4c0bcdef22effe5ca1bea 100644 (file)
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -343,8 +343,7 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat
        states->tracked.strdup_strings = 1;
        states->stale.strdup_strings = 1;
        for (ref = fetch_map; ref; ref = ref->next) {
        states->tracked.strdup_strings = 1;
        states->stale.strdup_strings = 1;
        for (ref = fetch_map; ref; ref = ref->next) {

-               unsigned char sha1[20];
-               if (!ref->peer_ref || read_ref(ref->peer_ref->name, sha1))
+               if (!ref->peer_ref || !ref_exists(ref->peer_ref->name))

                        string_list_append(&states->new, abbrev_branch(ref->name));
                else
                        string_list_append(&states->tracked, abbrev_branch(ref->name));
                        string_list_append(&states->new, abbrev_branch(ref->name));
                else
                        string_list_append(&states->tracked, abbrev_branch(ref->name));


@@ -710,7 +709,7 @@ static int mv(int argc, const char **argv)
                int flag = 0;
                unsigned char sha1[20];
 
                int flag = 0;
                unsigned char sha1[20];
 

-               resolve_ref(item->string, sha1, 1, &flag);
+               read_ref_full(item->string, sha1, 1, &flag);

                if (!(flag & REF_ISSYMREF))
                        continue;
                if (delete_ref(item->string, NULL, REF_NODEREF))
                if (!(flag & REF_ISSYMREF))
                        continue;
                if (delete_ref(item->string, NULL, REF_NODEREF))


@@ -1220,10 +1219,9 @@ static int set_head(int argc, const char **argv)
                usage_with_options(builtin_remote_sethead_usage, options);
 
        if (head_name) {
                usage_with_options(builtin_remote_sethead_usage, options);
 
        if (head_name) {

-               unsigned char sha1[20];

                strbuf_addf(&buf2, "refs/remotes/%s/%s", argv[0], head_name);
                /* make sure it's valid */
                strbuf_addf(&buf2, "refs/remotes/%s/%s", argv[0], head_name);
                /* make sure it's valid */

-               if (!resolve_ref(buf2.buf, sha1, 1, NULL))
+               if (!ref_exists(buf2.buf))

                        result |= error("Not a valid ref: %s", buf2.buf);
                else if (create_symref(buf.buf, buf2.buf, "remote set-head"))
                        result |= error("Could not setup %s", buf.buf);
                        result |= error("Not a valid ref: %s", buf2.buf);
                else if (create_symref(buf.buf, buf2.buf, "remote set-head"))
                        result |= error("Could not setup %s", buf.buf);

diff --git a/builtin/replace.c b/builtin/replace.c
index 517fa1031a86f50c0d41ba567237aa701e9c2c05..4a8970e9c95c1e07c1ed919d40f33238dcd05588 100644 (file)
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -58,7 +58,7 @@ static int for_each_replace_name(const char **argv, each_replace_name_fn fn)
                        had_error = 1;
                        continue;
                }
                        had_error = 1;
                        continue;
                }

-               if (!resolve_ref(ref, sha1, 1, NULL)) {
+               if (read_ref(ref, sha1)) {

                        error("replace ref '%s' not found.", *p);
                        had_error = 1;
                        continue;
                        error("replace ref '%s' not found.", *p);
                        had_error = 1;
                        continue;


@@ -97,7 +97,7 @@ static int replace_object(const char *object_ref, const char *replace_ref,
        if (check_refname_format(ref, 0))
                die("'%s' is not a valid ref name.", ref);
 
        if (check_refname_format(ref, 0))
                die("'%s' is not a valid ref name.", ref);
 

-       if (!resolve_ref(ref, prev, 1, NULL))
+       if (read_ref(ref, prev))

                hashclr(prev);
        else if (!force)
                die("replace ref '%s' already exists", ref);
                hashclr(prev);
        else if (!force)
                die("replace ref '%s' already exists", ref);

diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index fafb6dd50081b3af22bdbcde898d170ef47f679d..3911661900f79562133b4ebd6cbb513a06232b5b 100644 (file)
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -225,7 +225,7 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
                        unsigned char sha1[20];
 
                        if (!prefixcmp(*pattern, "refs/") &&
                        unsigned char sha1[20];
 
                        if (!prefixcmp(*pattern, "refs/") &&

-                           resolve_ref(*pattern, sha1, 1, NULL)) {
+                           !read_ref(*pattern, sha1)) {

                                if (!quiet)
                                        show_one(*pattern, sha1);
                        }
                                if (!quiet)
                                        show_one(*pattern, sha1);
                        }

diff --git a/builtin/tag.c b/builtin/tag.c
index 9b6fd95494fd80240f88d49224199b7e621717e4..439249d466ccb9d3e4d78764b8b4b8c44536a7b5 100644 (file)
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -174,7 +174,7 @@ static int for_each_tag_name(const char **argv, each_tag_name_fn fn)
                        had_error = 1;
                        continue;
                }
                        had_error = 1;
                        continue;
                }

-               if (!resolve_ref(ref, sha1, 1, NULL)) {
+               if (read_ref(ref, sha1)) {

                        error(_("tag '%s' not found."), *p);
                        had_error = 1;
                        continue;
                        error(_("tag '%s' not found."), *p);
                        had_error = 1;
                        continue;


@@ -518,7 +518,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
        if (strbuf_check_tag_ref(&ref, tag))
                die(_("'%s' is not a valid tag name."), tag);
 
        if (strbuf_check_tag_ref(&ref, tag))
                die(_("'%s' is not a valid tag name."), tag);
 

-       if (!resolve_ref(ref.buf, prev, 1, NULL))
+       if (read_ref(ref.buf, prev))

                hashclr(prev);
        else if (!force)
                die(_("tag '%s' already exists"), tag);
                hashclr(prev);
        else if (!force)
                die(_("tag '%s' already exists"), tag);

diff --git a/bundle.c b/bundle.c
index 08020bc3a258e055a8f5f6974217cf482edc3ee3..4742f2734eeb3dee117ab02a422056cfee4f71ea 100644 (file)
--- a/bundle.c
+++ b/bundle.c
@@ -320,7 +320,7 @@ int create_bundle(struct bundle_header *header, const char *path,
                        continue;
                if (dwim_ref(e->name, strlen(e->name), sha1, &ref) != 1)
                        continue;
                        continue;
                if (dwim_ref(e->name, strlen(e->name), sha1, &ref) != 1)
                        continue;

-               if (!resolve_ref(e->name, sha1, 1, &flag))
+               if (read_ref_full(e->name, sha1, 1, &flag))

                        flag = 0;
                display_ref = (flag & REF_ISSYMREF) ? e->name : ref;
 
                        flag = 0;
                display_ref = (flag & REF_ISSYMREF) ? e->name : ref;
 

diff --git a/cache.h b/cache.h
index 2e6ad3604e4cc6efd60a69407159967a1940e680..5badece71b1e473b6a2c0f0e446e5ce030b2ecd5 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -832,6 +832,8 @@ static inline int get_sha1_with_context(const char *str, unsigned char *sha1, st
 extern int get_sha1_hex(const char *hex, unsigned char *sha1);
 
 extern char *sha1_to_hex(const unsigned char *sha1);   /* static buffer result! */
 extern int get_sha1_hex(const char *hex, unsigned char *sha1);
 
 extern char *sha1_to_hex(const unsigned char *sha1);   /* static buffer result! */

+extern int read_ref_full(const char *filename, unsigned char *sha1,
+                        int reading, int *flags);

 extern int read_ref(const char *filename, unsigned char *sha1);
 
 /*
 extern int read_ref(const char *filename, unsigned char *sha1);
 
 /*

diff --git a/notes-merge.c b/notes-merge.c
index e9e41993117ae03ed7c9d1f28081a42b2eee97ca..e33c2c9f165ea3ae883860066aca8f91018d2a75 100644 (file)
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -568,7 +568,7 @@ int notes_merge(struct notes_merge_options *o,
               o->local_ref, o->remote_ref);
 
        /* Dereference o->local_ref into local_sha1 */
               o->local_ref, o->remote_ref);
 
        /* Dereference o->local_ref into local_sha1 */

-       if (!resolve_ref(o->local_ref, local_sha1, 0, NULL))
+       if (read_ref_full(o->local_ref, local_sha1, 0, NULL))

                die("Failed to resolve local notes ref '%s'", o->local_ref);
        else if (!check_refname_format(o->local_ref, 0) &&
                is_null_sha1(local_sha1))
                die("Failed to resolve local notes ref '%s'", o->local_ref);
        else if (!check_refname_format(o->local_ref, 0) &&
                is_null_sha1(local_sha1))

diff --git a/refs.c b/refs.c
index e69ba26b74e430388c63af21473438468678bb4b..44c1c86e187568f8cf1224f2f540cb6b73db3469 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -334,7 +334,7 @@ static void get_ref_dir(const char *submodule, const char *base,
                                        flag |= REF_ISBROKEN;
                                }
                        } else
                                        flag |= REF_ISBROKEN;
                                }
                        } else

-                               if (!resolve_ref(ref, sha1, 1, &flag)) {
+                               if (read_ref_full(ref, sha1, 1, &flag)) {

                                        hashclr(sha1);
                                        flag |= REF_ISBROKEN;
                                }
                                        hashclr(sha1);
                                        flag |= REF_ISBROKEN;
                                }


@@ -612,13 +612,18 @@ struct ref_filter {
        void *cb_data;
 };
 
        void *cb_data;
 };
 

-int read_ref(const char *ref, unsigned char *sha1)
+int read_ref_full(const char *ref, unsigned char *sha1, int reading, int *flags)

 {
 {

-       if (resolve_ref(ref, sha1, 1, NULL))
+       if (resolve_ref(ref, sha1, reading, flags))

                return 0;
        return -1;
 }
 
                return 0;
        return -1;
 }
 

+int read_ref(const char *ref, unsigned char *sha1)
+{
+       return read_ref_full(ref, sha1, 1, NULL);
+}
+

 #define DO_FOR_EACH_INCLUDE_BROKEN 01
 static int do_one_ref(const char *base, each_ref_fn fn, int trim,
                      int flags, void *cb_data, struct ref_entry *entry)
 #define DO_FOR_EACH_INCLUDE_BROKEN 01
 static int do_one_ref(const char *base, each_ref_fn fn, int trim,
                      int flags, void *cb_data, struct ref_entry *entry)


@@ -663,7 +668,7 @@ int peel_ref(const char *ref, unsigned char *sha1)
                goto fallback;
        }
 
                goto fallback;
        }
 

-       if (!resolve_ref(ref, base, 1, &flag))
+       if (read_ref_full(ref, base, 1, &flag))

                return -1;
 
        if ((flag & REF_ISPACKED)) {
                return -1;
 
        if ((flag & REF_ISPACKED)) {


@@ -746,7 +751,7 @@ static int do_head_ref(const char *submodule, each_ref_fn fn, void *cb_data)
                return 0;
        }
 
                return 0;
        }
 

-       if (resolve_ref("HEAD", sha1, 1, &flag))
+       if (!read_ref_full("HEAD", sha1, 1, &flag))

                return fn("HEAD", sha1, flag, cb_data);
 
        return 0;
                return fn("HEAD", sha1, flag, cb_data);
 
        return 0;


@@ -826,7 +831,7 @@ int head_ref_namespaced(each_ref_fn fn, void *cb_data)
        int flag;
 
        strbuf_addf(&buf, "%sHEAD", get_git_namespace());
        int flag;
 
        strbuf_addf(&buf, "%sHEAD", get_git_namespace());

-       if (resolve_ref(buf.buf, sha1, 1, &flag))
+       if (!read_ref_full(buf.buf, sha1, 1, &flag))

                ret = fn(buf.buf, sha1, flag, cb_data);
        strbuf_release(&buf);
 
                ret = fn(buf.buf, sha1, flag, cb_data);
        strbuf_release(&buf);
 


@@ -1022,7 +1027,7 @@ int refname_match(const char *abbrev_name, const char *full_name, const char **r
 static struct ref_lock *verify_lock(struct ref_lock *lock,
        const unsigned char *old_sha1, int mustexist)
 {
 static struct ref_lock *verify_lock(struct ref_lock *lock,
        const unsigned char *old_sha1, int mustexist)
 {

-       if (!resolve_ref(lock->ref_name, lock->old_sha1, mustexist, NULL)) {
+       if (read_ref_full(lock->ref_name, lock->old_sha1, mustexist, NULL)) {

                error("Can't verify ref %s", lock->ref_name);
                unlock_ref(lock);
                return NULL;
                error("Can't verify ref %s", lock->ref_name);
                unlock_ref(lock);
                return NULL;


@@ -1377,7 +1382,8 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
                goto rollback;
        }
 
                goto rollback;
        }
 

-       if (resolve_ref(newref, sha1, 1, &flag) && delete_ref(newref, sha1, REF_NODEREF)) {
+       if (!read_ref_full(newref, sha1, 1, &flag) &&
+           delete_ref(newref, sha1, REF_NODEREF)) {

                if (errno==EISDIR) {
                        if (remove_empty_directories(git_path("%s", newref))) {
                                error("Directory not empty: %s", newref);
                if (errno==EISDIR) {
                        if (remove_empty_directories(git_path("%s", newref))) {
                                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)
                                retval = do_for_each_reflog(log, fn, cb_data);
                        } else {
                                unsigned char sha1[20];
                                retval = do_for_each_reflog(log, fn, cb_data);
                        } else {
                                unsigned char sha1[20];

-                               if (!resolve_ref(log, sha1, 0, NULL))
+                               if (read_ref_full(log, sha1, 0, NULL))

                                        retval = error("bad ref for %s", log);
                                else
                                        retval = fn(log, sha1, 0, cb_data);
                                        retval = error("bad ref for %s", log);
                                else
                                        retval = fn(log, sha1, 0, cb_data);


@@ -2072,7 +2078,6 @@ char *shorten_unambiguous_ref(const char *ref, int strict)
                 */
                for (j = 0; j < rules_to_fail; j++) {
                        const char *rule = ref_rev_parse_rules[j];
                 */
                for (j = 0; j < rules_to_fail; j++) {
                        const char *rule = ref_rev_parse_rules[j];

-                       unsigned char short_objectname[20];

                        char refname[PATH_MAX];
 
                        /* skip matched rule */
                        char refname[PATH_MAX];
 
                        /* skip matched rule */


@@ -2086,7 +2091,7 @@ char *shorten_unambiguous_ref(const char *ref, int strict)
                         */
                        mksnpath(refname, sizeof(refname),
                                 rule, short_name_len, short_name);
                         */
                        mksnpath(refname, sizeof(refname),
                                 rule, short_name_len, short_name);

-                       if (!read_ref(refname, short_objectname))
+                       if (ref_exists(refname))

                                break;
                }
 
                                break;
                }
 

diff --git a/remote.c b/remote.c
index e2ef99114478c49863809ea5cef46392a5a8c0e9..6655bb05b2d31bca080cf3cfc431394b82d89160 100644 (file)
--- a/remote.c
+++ b/remote.c
@@ -1507,13 +1507,13 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs)
         * nothing to report.
         */
        base = branch->merge[0]->dst;
         * nothing to report.
         */
        base = branch->merge[0]->dst;

-       if (!resolve_ref(base, sha1, 1, NULL))
+       if (read_ref(base, sha1))

                return 0;
        theirs = lookup_commit_reference(sha1);
        if (!theirs)
                return 0;
 
                return 0;
        theirs = lookup_commit_reference(sha1);
        if (!theirs)
                return 0;
 

-       if (!resolve_ref(branch->refname, sha1, 1, NULL))
+       if (read_ref(branch->refname, sha1))

                return 0;
        ours = lookup_commit_reference(sha1);
        if (!ours)
                return 0;
        ours = lookup_commit_reference(sha1);
        if (!ours)