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>

Jeff King committed Mar 28, 2017 at 15:46 UTC 6cd4a8982dbde0b6fee26faba2873c475af50a28
1 file changed +26 -18
refs.c
+26 -18
@@ -429,29 +429,31 @@ int expand_ref(const char *str, int len, unsigned char *sha1, char **ref)
429 {
430 const char **p, *r;
431 int refs_found = 0;
432 + struct strbuf fullref = STRBUF_INIT;
433
434 *ref = NULL;
435 for (p = ref_rev_parse_rules; *p; p++) {
435 - char fullref[PATH_MAX];
436 unsigned char sha1_from_ref[20];
437 unsigned char *this_result;
438 int flag;
439
440 this_result = refs_found ? sha1_from_ref : sha1;
441 - mksnpath(fullref, sizeof(fullref), *p, len, str);
442 - r = resolve_ref_unsafe(fullref, RESOLVE_REF_READING,
441 + strbuf_reset(&fullref);
442 + strbuf_addf(&fullref, *p, len, str);
443 + r = resolve_ref_unsafe(fullref.buf, RESOLVE_REF_READING,
444 this_result, &flag);
445 if (r) {
446 if (!refs_found++)
447 *ref = xstrdup(r);
448 if (!warn_ambiguous_refs)
449 break;
449 - } else if ((flag & REF_ISSYMREF) && strcmp(fullref, "HEAD")) {
450 - warning("ignoring dangling symref %s.", fullref);
451 - } else if ((flag & REF_ISBROKEN) && strchr(fullref, '/')) {
452 - warning("ignoring broken ref %s.", fullref);
450 + } else if ((flag & REF_ISSYMREF) && strcmp(fullref.buf, "HEAD")) {
451 + warning("ignoring dangling symref %s.", fullref.buf);
452 + } else if ((flag & REF_ISBROKEN) && strchr(fullref.buf, '/')) {
453 + warning("ignoring broken ref %s.", fullref.buf);
454 }
455 }
456 + strbuf_release(&fullref);
457 return refs_found;
458 }
459
@@ -460,21 +462,22 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
462 char *last_branch = substitute_branch_name(&str, &len);
463 const char **p;
464 int logs_found = 0;
465 + struct strbuf path = STRBUF_INIT;
466
467 *log = NULL;
468 for (p = ref_rev_parse_rules; *p; p++) {
469 unsigned char hash[20];
467 - char path[PATH_MAX];
470 const char *ref, *it;
471
470 - mksnpath(path, sizeof(path), *p, len, str);
471 - ref = resolve_ref_unsafe(path, RESOLVE_REF_READING,
472 + strbuf_reset(&path);
473 + strbuf_addf(&path, *p, len, str);
474 + ref = resolve_ref_unsafe(path.buf, RESOLVE_REF_READING,
475 hash, NULL);
476 if (!ref)
477 continue;
475 - if (reflog_exists(path))
476 - it = path;
477 - else if (strcmp(ref, path) && reflog_exists(ref))
478 + if (reflog_exists(path.buf))
479 + it = path.buf;
480 + else if (strcmp(ref, path.buf) && reflog_exists(ref))
481 it = ref;
482 else
483 continue;
@@ -485,6 +488,7 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
488 if (!warn_ambiguous_refs)
489 break;
490 }
491 + strbuf_release(&path);
492 free(last_branch);
493 return logs_found;
494 }
@@ -944,6 +948,7 @@ char *shorten_unambiguous_ref(const char *refname, int strict)
948 static char **scanf_fmts;
949 static int nr_rules;
950 char *short_name;
951 + struct strbuf resolved_buf = STRBUF_INIT;
952
953 if (!nr_rules) {
954 /*
@@ -1002,7 +1007,6 @@ char *shorten_unambiguous_ref(const char *refname, int strict)
1007 */
1008 for (j = 0; j < rules_to_fail; j++) {
1009 const char *rule = ref_rev_parse_rules[j];
1005 - char refname[PATH_MAX];
1010
1011 /* skip matched rule */
1012 if (i == j)
@@ -1013,9 +1017,10 @@ char *shorten_unambiguous_ref(const char *refname, int strict)
1017 * (with this previous rule) to a valid ref
1018 * read_ref() returns 0 on success
1019 */
1016 - mksnpath(refname, sizeof(refname),
1017 - rule, short_name_len, short_name);
1018 - if (ref_exists(refname))
1020 + strbuf_reset(&resolved_buf);
1021 + strbuf_addf(&resolved_buf, rule,
1022 + short_name_len, short_name);
1023 + if (ref_exists(resolved_buf.buf))
1024 break;
1025 }
1026
@@ -1023,10 +1028,13 @@ char *shorten_unambiguous_ref(const char *refname, int strict)
1028 * short name is non-ambiguous if all previous rules
1029 * haven't resolved to a valid ref
1030 */
1026 - if (j == rules_to_fail)
1031 + if (j == rules_to_fail) {
1032 + strbuf_release(&resolved_buf);
1033 return short_name;
1034 + }
1035 }
1036
1037 + strbuf_release(&resolved_buf);
1038 free(short_name);
1039 return xstrdup(refname);
1040 }