avoid using mksnpath for refs

Like the previous commit, we'd like to avoid the assumption
that refs fit into PATH_MAX-sized buffers. These callsites
have an extra twist, though: they write the refnames using
mksnpath. This does two things beyond a regular snprintf:

1. It quietly writes "/bad-path/" when truncation occurs.
This saves the caller having to check the error code,
but if you aren't actually feeding the result to a
system call (and we aren't here), it's questionable.

2. It calls cleanup_path(), which removes leading
instances of "./".  That's questionable when dealing
with refnames, as we could silently canonicalize a
syntactically bogus refname into a valid one.

Let's convert each case to use a strbuf. This is preferable
to xstrfmt() because we can reuse the same buffer as we
loop.

Signed-off-by: Jeff King <peff@peff.net>

diff --git a/refs.c b/refs.c
index 0272e332cc9951497dddcd8b5c57a30e80552b4f..c2472184a724197dac236ae7b2a0c06e8168aad5 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -429,29 +429,31 @@ int expand_ref(const char *str, int len, unsigned char *sha1, char **ref)
 {
        const char **p, *r;
        int refs_found = 0;
 {
        const char **p, *r;
        int refs_found = 0;

+       struct strbuf fullref = STRBUF_INIT;

 
        *ref = NULL;
        for (p = ref_rev_parse_rules; *p; p++) {
 
        *ref = NULL;
        for (p = ref_rev_parse_rules; *p; p++) {

-               char fullref[PATH_MAX];

                unsigned char sha1_from_ref[20];
                unsigned char *this_result;
                int flag;
 
                this_result = refs_found ? sha1_from_ref : sha1;
                unsigned char sha1_from_ref[20];
                unsigned char *this_result;
                int flag;
 
                this_result = refs_found ? sha1_from_ref : sha1;

-               mksnpath(fullref, sizeof(fullref), *p, len, str);
-               r = resolve_ref_unsafe(fullref, RESOLVE_REF_READING,
+               strbuf_reset(&fullref);
+               strbuf_addf(&fullref, *p, len, str);
+               r = resolve_ref_unsafe(fullref.buf, RESOLVE_REF_READING,

                                       this_result, &flag);
                if (r) {
                        if (!refs_found++)
                                *ref = xstrdup(r);
                        if (!warn_ambiguous_refs)
                                break;
                                       this_result, &flag);
                if (r) {
                        if (!refs_found++)
                                *ref = xstrdup(r);
                        if (!warn_ambiguous_refs)
                                break;

-               } else if ((flag & REF_ISSYMREF) && strcmp(fullref, "HEAD")) {
-                       warning("ignoring dangling symref %s.", fullref);
-               } else if ((flag & REF_ISBROKEN) && strchr(fullref, '/')) {
-                       warning("ignoring broken ref %s.", fullref);
+               } else if ((flag & REF_ISSYMREF) && strcmp(fullref.buf, "HEAD")) {
+                       warning("ignoring dangling symref %s.", fullref.buf);
+               } else if ((flag & REF_ISBROKEN) && strchr(fullref.buf, '/')) {
+                       warning("ignoring broken ref %s.", fullref.buf);

                }
        }
                }
        }

+       strbuf_release(&fullref);

        return refs_found;
 }
 
        return refs_found;
 }
 


@@ -460,21 +462,22 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
        char *last_branch = substitute_branch_name(&str, &len);
        const char **p;
        int logs_found = 0;
        char *last_branch = substitute_branch_name(&str, &len);
        const char **p;
        int logs_found = 0;

+       struct strbuf path = STRBUF_INIT;

 
        *log = NULL;
        for (p = ref_rev_parse_rules; *p; p++) {
                unsigned char hash[20];
 
        *log = NULL;
        for (p = ref_rev_parse_rules; *p; p++) {
                unsigned char hash[20];

-               char path[PATH_MAX];

                const char *ref, *it;
 
                const char *ref, *it;
 

-               mksnpath(path, sizeof(path), *p, len, str);
-               ref = resolve_ref_unsafe(path, RESOLVE_REF_READING,
+               strbuf_reset(&path);
+               strbuf_addf(&path, *p, len, str);
+               ref = resolve_ref_unsafe(path.buf, RESOLVE_REF_READING,

                                         hash, NULL);
                if (!ref)
                        continue;
                                         hash, NULL);
                if (!ref)
                        continue;

-               if (reflog_exists(path))
-                       it = path;
-               else if (strcmp(ref, path) && reflog_exists(ref))
+               if (reflog_exists(path.buf))
+                       it = path.buf;
+               else if (strcmp(ref, path.buf) && reflog_exists(ref))

                        it = ref;
                else
                        continue;
                        it = ref;
                else
                        continue;


@@ -485,6 +488,7 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
                if (!warn_ambiguous_refs)
                        break;
        }
                if (!warn_ambiguous_refs)
                        break;
        }

+       strbuf_release(&path);

        free(last_branch);
        return logs_found;
 }
        free(last_branch);
        return logs_found;
 }


@@ -944,6 +948,7 @@ char *shorten_unambiguous_ref(const char *refname, int strict)
        static char **scanf_fmts;
        static int nr_rules;
        char *short_name;
        static char **scanf_fmts;
        static int nr_rules;
        char *short_name;

+       struct strbuf resolved_buf = STRBUF_INIT;

 
        if (!nr_rules) {
                /*
 
        if (!nr_rules) {
                /*


@@ -1002,7 +1007,6 @@ char *shorten_unambiguous_ref(const char *refname, 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];

-                       char refname[PATH_MAX];

 
                        /* skip matched rule */
                        if (i == j)
 
                        /* skip matched rule */
                        if (i == j)


@@ -1013,9 +1017,10 @@ char *shorten_unambiguous_ref(const char *refname, int strict)
                         * (with this previous rule) to a valid ref
                         * read_ref() returns 0 on success
                         */
                         * (with this previous rule) to a valid ref
                         * read_ref() returns 0 on success
                         */

-                       mksnpath(refname, sizeof(refname),
-                                rule, short_name_len, short_name);
-                       if (ref_exists(refname))
+                       strbuf_reset(&resolved_buf);
+                       strbuf_addf(&resolved_buf, rule,
+                                   short_name_len, short_name);
+                       if (ref_exists(resolved_buf.buf))

                                break;
                }
 
                                break;
                }
 


@@ -1023,10 +1028,13 @@ char *shorten_unambiguous_ref(const char *refname, int strict)
                 * short name is non-ambiguous if all previous rules
                 * haven't resolved to a valid ref
                 */
                 * short name is non-ambiguous if all previous rules
                 * haven't resolved to a valid ref
                 */

-               if (j == rules_to_fail)
+               if (j == rules_to_fail) {
+                       strbuf_release(&resolved_buf);

                        return short_name;
                        return short_name;

+               }

        }
 
        }
 

+       strbuf_release(&resolved_buf);

        free(short_name);
        return xstrdup(refname);
 }
        free(short_name);
        return xstrdup(refname);
 }