@matheusbsbs / matheus-bagliano / commits / ce14de03db

refs API: remove "failure_errno" from refs_resolve_ref_unsafe()

Remove the now-unused "failure_errno" parameter from the refs_resolve_ref_unsafe() signature. In my recent 96f6623ada0 (Merge branch 'ab/refs-errno-cleanup', 2021-11-29) series we made all of its callers explicitly request the errno via an output parameter. As that series shows all but one caller ended up passing in a boilerplate "ignore_errno", since they only cared about whether the return value was NULL or not, i.e. if the ref could be resolved. There was one small issue with that series fixed with a follow-up in 31e39123695 (Merge branch 'ab/refs-errno-cleanup', 2022-01-14) a small bug in that series was fixed. After those two there was one caller left in sequencer.c that used the "failure_errno', but as of the preceding commit it uses a boilerplate "ignore_errno" instead. This leaves the public refs API without any use of "failure_errno" at all. We could still do with a bit of cleanup and generalization between refs.c and refs/files-backend.c before the "reftable" integration lands, but that's all internal to the reference code itself. So let's remove this output parameter. Not only isn't it used now, but it's unlikely that we'll want it again in the future. We'd like to slowly move the refs API to a more file-backend independent way of communicating error codes, having it use a "failure_errno" was only the first step in that direction. If this or any other function needs to communicate what specifically is wrong with the requested "refname" it'll be better to have the function set some output enum of well-defined error states than piggy-backend on "errno". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Jan 26, 2022 at 15:37 UTC ce14de03db6e1a29ad92c747542f1eb4357f25d6
7 files changed +33 -77
refs.c
+17 -34
@@ -269,10 +269,9 @@ char *refs_resolve_refdup(struct ref_store *refs,
269 struct object_id *oid, int *flags)
270 {
271 const char *result;
272 - int ignore_errno;
272
273 result = refs_resolve_ref_unsafe(refs, refname, resolve_flags,
275 - oid, flags, &ignore_errno);
274 + oid, flags);
275 return xstrdup_or_null(result);
276 }
277
@@ -294,11 +293,10 @@ struct ref_filter {
293
294 int read_ref_full(const char *refname, int resolve_flags, struct object_id *oid, int *flags)
295 {
297 - int ignore_errno;
296 struct ref_store *refs = get_main_ref_store(the_repository);
297
298 if (refs_resolve_ref_unsafe(refs, refname, resolve_flags,
301 - oid, flags, &ignore_errno))
299 + oid, flags))
300 return 0;
301 return -1;
302 }
@@ -310,9 +308,8 @@ int read_ref(const char *refname, struct object_id *oid)
308
309 int refs_ref_exists(struct ref_store *refs, const char *refname)
310 {
313 - int ignore_errno;
311 return !!refs_resolve_ref_unsafe(refs, refname, RESOLVE_REF_READING,
315 - NULL, NULL, &ignore_errno);
312 + NULL, NULL);
313 }
314
315 int ref_exists(const char *refname)
@@ -656,15 +653,13 @@ int expand_ref(struct repository *repo, const char *str, int len,
653 struct object_id *this_result;
654 int flag;
655 struct ref_store *refs = get_main_ref_store(repo);
659 - int ignore_errno;
656
657 this_result = refs_found ? &oid_from_ref : oid;
658 strbuf_reset(&fullref);
659 strbuf_addf(&fullref, *p, len, str);
660 r = refs_resolve_ref_unsafe(refs, fullref.buf,
661 RESOLVE_REF_READING,
666 - this_result, &flag,
667 - &ignore_errno);
662 + this_result, &flag);
663 if (r) {
664 if (!refs_found++)
665 *ref = xstrdup(r);
@@ -693,14 +688,12 @@ int repo_dwim_log(struct repository *r, const char *str, int len,
688 for (p = ref_rev_parse_rules; *p; p++) {
689 struct object_id hash;
690 const char *ref, *it;
696 - int ignore_errno;
691
692 strbuf_reset(&path);
693 strbuf_addf(&path, *p, len, str);
694 ref = refs_resolve_ref_unsafe(refs, path.buf,
695 RESOLVE_REF_READING,
702 - oid ? &hash : NULL, NULL,
703 - &ignore_errno);
696 + oid ? &hash : NULL, NULL);
697 if (!ref)
698 continue;
699 if (refs_reflog_exists(refs, path.buf))
@@ -1390,10 +1383,9 @@ int refs_head_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
1383 {
1384 struct object_id oid;
1385 int flag;
1393 - int ignore_errno;
1386
1387 if (refs_resolve_ref_unsafe(refs, "HEAD", RESOLVE_REF_READING,
1396 - &oid, &flag, &ignore_errno))
1388 + &oid, &flag))
1389 return fn("HEAD", &oid, flag, cb_data);
1390
1391 return 0;
@@ -1682,15 +1674,13 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
1674 const char *refname,
1675 int resolve_flags,
1676 struct object_id *oid,
1685 - int *flags, int *failure_errno)
1677 + int *flags)
1678 {
1679 static struct strbuf sb_refname = STRBUF_INIT;
1680 struct object_id unused_oid;
1681 int unused_flags;
1682 int symref_count;
1683
1692 - assert(failure_errno);
1693 -
1684 if (!oid)
1685 oid = &unused_oid;
1686 if (!flags)
@@ -1700,10 +1690,8 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
1690
1691 if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
1692 if (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||
1703 - !refname_is_safe(refname)) {
1704 - *failure_errno = EINVAL;
1693 + !refname_is_safe(refname))
1694 return NULL;
1706 - }
1695
1696 /*
1697 * dwim_ref() uses REF_ISBROKEN to distinguish between
@@ -1718,9 +1706,10 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
1706
1707 for (symref_count = 0; symref_count < SYMREF_MAXDEPTH; symref_count++) {
1708 unsigned int read_flags = 0;
1709 + int failure_errno;
1710
1711 if (refs_read_raw_ref(refs, refname, oid, &sb_refname,
1723 - &read_flags, failure_errno)) {
1712 + &read_flags, &failure_errno)) {
1713 *flags |= read_flags;
1714
1715 /* In reading mode, refs must eventually resolve */
@@ -1732,9 +1721,9 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
1721 * may show errors besides ENOENT if there are
1722 * similarly-named refs.
1723 */
1735 - if (*failure_errno != ENOENT &&
1736 - *failure_errno != EISDIR &&
1737 - *failure_errno != ENOTDIR)
1724 + if (failure_errno != ENOENT &&
1725 + failure_errno != EISDIR &&
1726 + failure_errno != ENOTDIR)
1727 return NULL;
1728
1729 oidclr(oid);
@@ -1760,16 +1749,13 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
1749 }
1750 if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
1751 if (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||
1763 - !refname_is_safe(refname)) {
1764 - *failure_errno = EINVAL;
1752 + !refname_is_safe(refname))
1753 return NULL;
1766 - }
1754
1755 *flags |= REF_ISBROKEN | REF_BAD_NAME;
1756 }
1757 }
1758
1772 - *failure_errno = ELOOP;
1759 return NULL;
1760 }
1761
@@ -1784,10 +1770,8 @@ int refs_init_db(struct strbuf *err)
1770 const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
1771 struct object_id *oid, int *flags)
1772 {
1787 - int ignore_errno;
1788 -
1773 return refs_resolve_ref_unsafe(get_main_ref_store(the_repository), refname,
1790 - resolve_flags, oid, flags, &ignore_errno);
1774 + resolve_flags, oid, flags);
1775 }
1776
1777 int resolve_gitlink_ref(const char *submodule, const char *refname,
@@ -1795,15 +1779,14 @@ int resolve_gitlink_ref(const char *submodule, const char *refname,
1779 {
1780 struct ref_store *refs;
1781 int flags;
1798 - int ignore_errno;
1782
1783 refs = get_submodule_ref_store(submodule);
1784
1785 if (!refs)
1786 return -1;
1787
1805 - if (!refs_resolve_ref_unsafe(refs, refname, 0, oid, &flags,
1806 - &ignore_errno) || is_null_oid(oid))
1788 + if (!refs_resolve_ref_unsafe(refs, refname, 0, oid, &flags) ||
1789 + is_null_oid(oid))
1790 return -1;
1791 return 0;
1792 }
refs.h
+1 -6
@@ -58,11 +58,6 @@ struct worktree;
58 * resolved. The function returns NULL for such ref names.
59 * Caps and underscores refers to the special refs, such as HEAD,
60 * FETCH_HEAD and friends, that all live outside of the refs/ directory.
61 - *
62 - * Callers should not inspect "errno" on failure, but rather pass in a
63 - * "failure_errno" parameter, on failure the "errno" will indicate the
64 - * type of failure encountered, but not necessarily one that came from
65 - * a syscall. We might have faked it up.
61 */
62 #define RESOLVE_REF_READING 0x01
63 #define RESOLVE_REF_NO_RECURSE 0x02
@@ -72,7 +67,7 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
67 const char *refname,
68 int resolve_flags,
69 struct object_id *oid,
75 - int *flags, int *failure_errno);
70 + int *flags);
71
72 const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
73 struct object_id *oid, int *flags);
refs/files-backend.c
+9 -22
@@ -277,11 +277,10 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,
277 create_dir_entry(dir->cache, refname.buf,
278 refname.len));
279 } else {
280 - int ignore_errno;
280 if (!refs_resolve_ref_unsafe(&refs->base,
281 refname.buf,
282 RESOLVE_REF_READING,
284 - &oid, &flag, &ignore_errno)) {
283 + &oid, &flag)) {
284 oidclr(&oid);
285 flag |= REF_ISBROKEN;
286 } else if (is_null_oid(&oid)) {
@@ -1006,7 +1005,6 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,
1005 {
1006 struct strbuf ref_file = STRBUF_INIT;
1007 struct ref_lock *lock;
1009 - int ignore_errno;
1008
1009 files_assert_main_repository(refs, "lock_ref_oid_basic");
1010 assert(err);
@@ -1034,7 +1032,7 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,
1032 }
1033
1034 if (!refs_resolve_ref_unsafe(&refs->base, lock->ref_name, 0,
1037 - &lock->old_oid, NULL, &ignore_errno))
1035 + &lock->old_oid, NULL))
1036 oidclr(&lock->old_oid);
1037 goto out;
1038
@@ -1399,7 +1397,6 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,
1397 struct strbuf tmp_renamed_log = STRBUF_INIT;
1398 int log, ret;
1399 struct strbuf err = STRBUF_INIT;
1402 - int ignore_errno;
1400
1401 files_reflog_path(refs, &sb_oldref, oldrefname);
1402 files_reflog_path(refs, &sb_newref, newrefname);
@@ -1413,7 +1410,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,
1410
1411 if (!refs_resolve_ref_unsafe(&refs->base, oldrefname,
1412 RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
1416 - &orig_oid, &flag, &ignore_errno)) {
1413 + &orig_oid, &flag)) {
1414 ret = error("refname %s not found", oldrefname);
1415 goto out;
1416 }
@@ -1459,7 +1456,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,
1456 */
1457 if (!copy && refs_resolve_ref_unsafe(&refs->base, newrefname,
1458 RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
1462 - NULL, NULL, &ignore_errno) &&
1459 + NULL, NULL) &&
1460 refs_delete_ref(&refs->base, NULL, newrefname,
1461 NULL, REF_NO_DEREF)) {
1462 if (errno == EISDIR) {
@@ -1828,12 +1825,10 @@ static int commit_ref_update(struct files_ref_store *refs,
1825 */
1826 int head_flag;
1827 const char *head_ref;
1831 - int ignore_errno;
1828
1829 head_ref = refs_resolve_ref_unsafe(&refs->base, "HEAD",
1830 RESOLVE_REF_READING,
1835 - NULL, &head_flag,
1836 - &ignore_errno);
1831 + NULL, &head_flag);
1832 if (head_ref && (head_flag & REF_ISSYMREF) &&
1833 !strcmp(head_ref, lock->ref_name)) {
1834 struct strbuf log_err = STRBUF_INIT;
@@ -1877,12 +1872,10 @@ static void update_symref_reflog(struct files_ref_store *refs,
1872 {
1873 struct strbuf err = STRBUF_INIT;
1874 struct object_id new_oid;
1880 - int ignore_errno;
1875
1876 if (logmsg &&
1877 refs_resolve_ref_unsafe(&refs->base, target,
1884 - RESOLVE_REF_READING, &new_oid, NULL,
1885 - &ignore_errno) &&
1878 + RESOLVE_REF_READING, &new_oid, NULL) &&
1879 files_log_ref_write(refs, refname, &lock->old_oid,
1880 &new_oid, logmsg, 0, &err)) {
1881 error("%s", err.buf);
@@ -2156,7 +2149,6 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator)
2149 (struct files_reflog_iterator *)ref_iterator;
2150 struct dir_iterator *diter = iter->dir_iterator;
2151 int ok;
2159 - int ignore_errno;
2152
2153 while ((ok = dir_iterator_advance(diter)) == ITER_OK) {
2154 int flags;
@@ -2170,8 +2162,7 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator)
2162
2163 if (!refs_resolve_ref_unsafe(iter->ref_store,
2164 diter->relative_path, 0,
2173 - &iter->oid, &flags,
2174 - &ignore_errno)) {
2165 + &iter->oid, &flags)) {
2166 error("bad ref for %s", diter->path.buf);
2167 continue;
2168 }
@@ -2515,11 +2506,9 @@ static int lock_ref_for_update(struct files_ref_store *refs,
2506 * the transaction, so we have to read it here
2507 * to record and possibly check old_oid:
2508 */
2518 - int ignore_errno;
2509 if (!refs_resolve_ref_unsafe(&refs->base,
2510 referent.buf, 0,
2521 - &lock->old_oid, NULL,
2522 - &ignore_errno)) {
2511 + &lock->old_oid, NULL)) {
2512 if (update->flags & REF_HAVE_OLD) {
2513 strbuf_addf(err, "cannot lock ref '%s': "
2514 "error reading reference",
@@ -3208,14 +3197,12 @@ static int files_reflog_expire(struct ref_store *ref_store,
3197
3198 if ((expire_flags & EXPIRE_REFLOGS_UPDATE_REF) &&
3199 !is_null_oid(&cb.last_kept_oid)) {
3211 - int ignore_errno;
3200 int type;
3201 const char *ref;
3202
3203 ref = refs_resolve_ref_unsafe(&refs->base, refname,
3204 RESOLVE_REF_NO_RECURSE,
3217 - NULL, &type,
3218 - &ignore_errno);
3205 + NULL, &type);
3206 update = !!(ref && !(type & REF_ISSYMREF));
3207 }
3208
remote.c
+1 -2
@@ -508,9 +508,8 @@ static void read_config(struct repository *repo)
508
509 repo->remote_state->current_branch = NULL;
510 if (startup_info->have_repository) {
511 - int ignore_errno;
511 const char *head_ref = refs_resolve_ref_unsafe(
513 - get_main_ref_store(repo), "HEAD", 0, NULL, &flag, &ignore_errno);
512 + get_main_ref_store(repo), "HEAD", 0, NULL, &flag);
513 if (head_ref && (flag & REF_ISSYMREF) &&
514 skip_prefix(head_ref, "refs/heads/", &head_ref)) {
515 repo->remote_state->current_branch = make_branch(
sequencer.c
+1 -3
@@ -1281,7 +1281,6 @@ void print_commit_summary(struct repository *r,
1281 struct strbuf author_ident = STRBUF_INIT;
1282 struct strbuf committer_ident = STRBUF_INIT;
1283 struct ref_store *refs;
1284 - int ignore_errno;
1284
1285 commit = lookup_commit(r, oid);
1286 if (!commit)
@@ -1332,8 +1331,7 @@ void print_commit_summary(struct repository *r,
1331 diff_setup_done(&rev.diffopt);
1332
1333 refs = get_main_ref_store(the_repository);
1335 - head = refs_resolve_ref_unsafe(refs, "HEAD", 0, NULL, NULL,
1336 - &ignore_errno);
1334 + head = refs_resolve_ref_unsafe(refs, "HEAD", 0, NULL, NULL);
1335 if (!head)
1336 die(_("unable to resolve HEAD after creating commit"));
1337 if (!strcmp(head, "HEAD"))
t/helper/test-ref-store.c
+1 -2
@@ -180,10 +180,9 @@ static int cmd_resolve_ref(struct ref_store *refs, const char **argv)
180 int resolve_flags = arg_flags(*argv++, "resolve-flags", empty_flags);
181 int flags;
182 const char *ref;
183 - int ignore_errno;
183
184 ref = refs_resolve_ref_unsafe(refs, refname, resolve_flags,
186 - &oid, &flags, &ignore_errno);
185 + &oid, &flags);
186 printf("%s %s 0x%x\n", oid_to_hex(&oid), ref ? ref : "(null)", flags);
187 return ref ? 0 : 1;
188 }
worktree.c
+3 -8
@@ -28,13 +28,11 @@ static void add_head_info(struct worktree *wt)
28 {
29 int flags;
30 const char *target;
31 - int ignore_errno;
31
32 target = refs_resolve_ref_unsafe(get_worktree_ref_store(wt),
33 "HEAD",
34 0,
36 - &wt->head_oid, &flags,
37 - &ignore_errno);
35 + &wt->head_oid, &flags);
36 if (!target)
37 return;
38
@@ -416,7 +414,6 @@ const struct worktree *find_shared_symref(struct worktree **worktrees,
414 const char *symref_target;
415 struct ref_store *refs;
416 int flags;
419 - int ignore_errno;
417
418 if (wt->is_bare)
419 continue;
@@ -434,8 +431,7 @@ const struct worktree *find_shared_symref(struct worktree **worktrees,
431
432 refs = get_worktree_ref_store(wt);
433 symref_target = refs_resolve_ref_unsafe(refs, symref, 0,
437 - NULL, &flags,
438 - &ignore_errno);
434 + NULL, &flags);
435 if ((flags & REF_ISSYMREF) &&
436 symref_target && !strcmp(symref_target, target)) {
437 existing = wt;
@@ -563,7 +559,6 @@ int other_head_refs(each_ref_fn fn, void *cb_data)
559 struct worktree *wt = *p;
560 struct object_id oid;
561 int flag;
566 - int ignore_errno;
562
563 if (wt->is_current)
564 continue;
@@ -573,7 +568,7 @@ int other_head_refs(each_ref_fn fn, void *cb_data)
568 if (refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
569 refname.buf,
570 RESOLVE_REF_READING,
576 - &oid, &flag, &ignore_errno))
571 + &oid, &flag))
572 ret = fn(refname.buf, &oid, flag, cb_data);
573 if (ret)
574 break;