Skip to content

Commit 6cd4a89

Browse files
peffgitster
authored andcommitted
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 <[email protected]>
1 parent 7f897b6 commit 6cd4a89

File tree

1 file changed

+26
-18
lines changed

1 file changed

+26
-18
lines changed

refs.c

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -429,29 +429,31 @@ int expand_ref(const char *str, int len, unsigned char *sha1, char **ref)
429429
{
430430
const char **p, *r;
431431
int refs_found = 0;
432+
struct strbuf fullref = STRBUF_INIT;
432433

433434
*ref = NULL;
434435
for (p = ref_rev_parse_rules; *p; p++) {
435-
char fullref[PATH_MAX];
436436
unsigned char sha1_from_ref[20];
437437
unsigned char *this_result;
438438
int flag;
439439

440440
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,
443444
this_result, &flag);
444445
if (r) {
445446
if (!refs_found++)
446447
*ref = xstrdup(r);
447448
if (!warn_ambiguous_refs)
448449
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);
453454
}
454455
}
456+
strbuf_release(&fullref);
455457
return refs_found;
456458
}
457459

@@ -460,21 +462,22 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
460462
char *last_branch = substitute_branch_name(&str, &len);
461463
const char **p;
462464
int logs_found = 0;
465+
struct strbuf path = STRBUF_INIT;
463466

464467
*log = NULL;
465468
for (p = ref_rev_parse_rules; *p; p++) {
466469
unsigned char hash[20];
467-
char path[PATH_MAX];
468470
const char *ref, *it;
469471

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,
472475
hash, NULL);
473476
if (!ref)
474477
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))
478481
it = ref;
479482
else
480483
continue;
@@ -485,6 +488,7 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
485488
if (!warn_ambiguous_refs)
486489
break;
487490
}
491+
strbuf_release(&path);
488492
free(last_branch);
489493
return logs_found;
490494
}
@@ -944,6 +948,7 @@ char *shorten_unambiguous_ref(const char *refname, int strict)
944948
static char **scanf_fmts;
945949
static int nr_rules;
946950
char *short_name;
951+
struct strbuf resolved_buf = STRBUF_INIT;
947952

948953
if (!nr_rules) {
949954
/*
@@ -1002,7 +1007,6 @@ char *shorten_unambiguous_ref(const char *refname, int strict)
10021007
*/
10031008
for (j = 0; j < rules_to_fail; j++) {
10041009
const char *rule = ref_rev_parse_rules[j];
1005-
char refname[PATH_MAX];
10061010

10071011
/* skip matched rule */
10081012
if (i == j)
@@ -1013,20 +1017,24 @@ char *shorten_unambiguous_ref(const char *refname, int strict)
10131017
* (with this previous rule) to a valid ref
10141018
* read_ref() returns 0 on success
10151019
*/
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))
10191024
break;
10201025
}
10211026

10221027
/*
10231028
* short name is non-ambiguous if all previous rules
10241029
* haven't resolved to a valid ref
10251030
*/
1026-
if (j == rules_to_fail)
1031+
if (j == rules_to_fail) {
1032+
strbuf_release(&resolved_buf);
10271033
return short_name;
1034+
}
10281035
}
10291036

1037+
strbuf_release(&resolved_buf);
10301038
free(short_name);
10311039
return xstrdup(refname);
10321040
}

0 commit comments

Comments
 (0)