{"thread":{"id":"57052","subject":"errno oversight","startedAt":"2021-12-08T11:40:04Z","lastAt":"2022-01-26T23:57:35Z","messageCount":28,"participants":["Han-Wen Nienhuys","Ævar Arnfjörð Bjarmason","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"443420","messageId":"CAFQ2z_NHXKss4LVBAFVpE7LFXt2OeOz9P9wi-z8riwHXWDb28w@mail.gmail.com","threadId":"57052","inReplyTo":null,"subject":"errno oversight","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@google.com","sentAt":"2021-12-08T11:38:23Z","receivedAt":"2021-12-08T11:40:04Z","isPatch":false,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"In refs.c in origin/next and origin/seen, we have the following fragment:\n\n if (refs_read_raw_ref(refs, refname, oid, &sb_refname,\n &read_flags, failure_errno)) {\n *flags |= read_flags;\n if (errno)\n *failure_errno = errno;\n\noverwriting failure_errno looks like an oversight?\n\n-- \nHan-Wen Nienhuys - Google Munich\nI work 80%. Don't expect answers from me on Fridays.\n--\nGoogle Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich\nRegistergericht und -nummer: Hamburg, HRB 86891\nSitz der Gesellschaft: Hamburg\nGeschäftsführer: Paul Manicle, Halimah DeLaine Prado\n"},{"id":"443437","messageId":"211208.86sfv3l0op.gmgdl@evledraar.gmail.com","threadId":"57052","inReplyTo":"CAFQ2z_NHXKss4LVBAFVpE7LFXt2OeOz9P9wi-z8riwHXWDb28w@mail.gmail.com","subject":"Re: errno oversight","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-08T12:47:51Z","receivedAt":"2021-12-08T12:51:23Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Dec 08 2021, Han-Wen Nienhuys wrote:\n\n> In refs.c in origin/next and origin/seen, we have the following fragment:\n\nIt's in \"master\".\n\n> if (refs_read_raw_ref(refs, refname, oid, &sb_refname,\n> &read_flags, failure_errno)) {\n> *flags |= read_flags;\n> if (errno)\n> *failure_errno = errno;\n>\n> overwriting failure_errno looks like an oversight?\n\nThis is from my ef18119dec8 (refs API: add a version of\nrefs_resolve_ref_unsafe() with \"errno\", 2021-10-16).\n\nI don't see the bug here, overwriting *failure_errno is the point of\nthat variable. We're keeping the right errno value right after a\nfailure, and passing it up to our caller.\n"},{"id":"443439","messageId":"211208.86o85rl07m.gmgdl@evledraar.gmail.com","threadId":"57052","inReplyTo":"211208.86sfv3l0op.gmgdl@evledraar.gmail.com","subject":"Re: errno oversight","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-08T13:00:30Z","receivedAt":"2021-12-08T13:01:40Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Dec 08 2021, Ævar Arnfjörð Bjarmason wrote:\n\n> On Wed, Dec 08 2021, Han-Wen Nienhuys wrote:\n>\n>> In refs.c in origin/next and origin/seen, we have the following fragment:\n>\n> It's in \"master\".\n>\n>> if (refs_read_raw_ref(refs, refname, oid, &sb_refname,\n>> &read_flags, failure_errno)) {\n>> *flags |= read_flags;\n>> if (errno)\n>> *failure_errno = errno;\n>>\n>> overwriting failure_errno looks like an oversight?\n>\n> This is from my ef18119dec8 (refs API: add a version of\n> refs_resolve_ref_unsafe() with \"errno\", 2021-10-16).\n>\n> I don't see the bug here, overwriting *failure_errno is the point of\n> that variable. We're keeping the right errno value right after a\n> failure, and passing it up to our caller.\n\nUrg, sorry. Yes obviously that should use the failure_errno from\nrefs_read_raw_ref().\n\nI'll submit a fix for this soon. There's some good post-cleanup to be\ndone here, it seems only one upstream caller of\nrefs_resolve_ref_unsafe() cares about the failure_errno currently (but I\ndidn't look into your reftable code).\n"},{"id":"443440","messageId":"CAFQ2z_MmWywH2AH3YaB+v67e-0jNsH9sz8dgqYenQNM6Gqz_Hw@mail.gmail.com","threadId":"57052","inReplyTo":"211208.86sfv3l0op.gmgdl@evledraar.gmail.com","subject":"Re: errno oversight","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@google.com","sentAt":"2021-12-08T13:05:10Z","receivedAt":"2021-12-08T13:05:25Z","isPatch":false,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"On Wed, Dec 8, 2021 at 1:51 PM Ævar Arnfjörð Bjarmason wrote:\n> > In refs.c in origin/next and origin/seen, we have the following fragment:\n>\n> It's in \"master\".\n\nCorrect, but it's also in seen.\n\n> > if (refs_read_raw_ref(refs, refname, oid, &sb_refname,\n> > &read_flags, failure_errno)) {\n> > *flags |= read_flags;\n> > if (errno)\n> > *failure_errno = errno;\n> >\n> > overwriting failure_errno looks like an oversight?\n>\n> This is from my ef18119dec8 (refs API: add a version of\n> refs_resolve_ref_unsafe() with \"errno\", 2021-10-16).\n>\n> I don't see the bug here, overwriting *failure_errno is the point of\n> that variable. We're keeping the right errno value right after a\n> failure, and passing it up to our caller.\n\nThe reftable code for refs_read_raw_ref places a sensible value into\nits failure_errno argument. Then the \"if (errno)\" replaces it with\nwhatever the outcome of the last syscall was. I started the errno work\nto get rid of this kind of global state manipulation; am I missing\nsomething?\n\n-- \nHan-Wen Nienhuys - Google Munich\nI work 80%. Don't expect answers from me on Fridays.\n--\n\nGoogle Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich\n\nRegistergericht und -nummer: Hamburg, HRB 86891\n\nSitz der Gesellschaft: Hamburg\n\nGeschäftsführer: Paul Manicle, Halimah DeLaine Prado\n"},{"id":"443496","messageId":"xmqqo85q2a42.fsf@gitster.g","threadId":"57052","inReplyTo":"211208.86o85rl07m.gmgdl@evledraar.gmail.com","subject":"Re: errno oversight","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-12-08T19:02:37Z","receivedAt":"2021-12-08T19:02:44Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n>>> if (refs_read_raw_ref(refs, refname, oid, &sb_refname,\n>>> &read_flags, failure_errno)) {\n>>> *flags |= read_flags;\n>>> if (errno)\n>>> *failure_errno = errno;\n> ...\n> Urg, sorry. Yes obviously that should use the failure_errno from\n> refs_read_raw_ref().\n\nYeah, I was wondering about the same thing, \"refs_read_raw_ref()\ntakes the failure_errno pointer for stuffing errno there, doesn't\nit? does the caller need to do anything?\".\n\n> I'll submit a fix for this soon. There's some good post-cleanup to be\n> done here, it seems only one upstream caller of\n> refs_resolve_ref_unsafe() cares about the failure_errno currently (but I\n> didn't look into your reftable code).\n\nThanks.\n"},{"id":"443565","messageId":"cover-0.3-00000000000-20211209T045735Z-avarab@gmail.com","threadId":"57052","inReplyTo":"xmqqo85q2a42.fsf@gitster.g","subject":"[PATCH 0/3] refs: ab/refs-errno-cleanup fixup + remove \"failure_errno\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-09T05:02:25Z","receivedAt":"2021-12-09T05:02:35Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The 1/3 here fixes the bug Han-Wen pointed out in[1]. As discussed\nthere it's obviously bad in the pre-image, but some oddities in\nrefs_resolve_ref_unsafe() interacts with code it calls ended mostly\nplastering over the differences by accident, so it wasn't caught by\ntesting.\n\nThen 2/3 makes the one external user of that \"failure_errno\" stop\nusing it, and 3/3 removes it entirely from the API interface.\n\n1. https://lore.kernel.org/git/CAFQ2z_NHXKss4LVBAFVpE7LFXt2OeOz9P9wi-z8riwHXWDb28w@mail.gmail.com/\n\nÆvar Arnfjörð Bjarmason (3):\n refs API: use \"failure_errno\", not \"errno\"\n sequencer: don't use die_errno() on refs_resolve_ref_unsafe() failure\n refs API: remove \"failure_errno\" from refs_resolve_ref_unsafe()\n\n refs.c | 53 +++++++++++++--------------------------\n refs.h | 7 +-----\n refs/files-backend.c | 34 ++++++++-----------------\n sequencer.c | 10 +++-----\n t/helper/test-ref-store.c | 3 +--\n worktree.c | 11 +++-----\n 6 files changed, 35 insertions(+), 83 deletions(-)\n\n-- \n2.34.1.930.g218b4aae189\n\n"},{"id":"443566","messageId":"patch-1.3-b983d3b6033-20211209T045735Z-avarab@gmail.com","threadId":"57052","inReplyTo":"cover-0.3-00000000000-20211209T045735Z-avarab@gmail.com","subject":"[PATCH 1/3] refs API: use \"failure_errno\", not \"errno\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-09T05:02:26Z","receivedAt":"2021-12-09T05:02:39Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a logic error in refs_resolve_ref_unsafe() introduced in a recent\nseries of mine to abstract the refs API away from errno. See\n96f6623ada0 (Merge branch 'ab/refs-errno-cleanup', 2021-11-29)for that\nseries.\n\nIn that series introduction of \"failure_errno\" to\nrefs_resolve_ref_unsafe came in ef18119dec8 (refs API: add a version\nof refs_resolve_ref_unsafe() with \"errno\", 2021-10-16). There we'd set\n\"errno = 0\" immediately before refs_read_raw_ref(), and then set\n\"failure_errno\" to \"errno\" if errno was non-zero afterwards.\n\nThen in the next commit 8b72fea7e91 (refs API: make\nrefs_read_raw_ref() not set errno, 2021-10-16) we started expecting\n\"refs_read_raw_ref()\" to set \"failure_errno\". It would do that if\nrefs_read_raw_ref() failed, but it wouldn't be the same errno.\n\nSo we might set the \"errno\" here to any arbitrary bad value, and end\nup e.g. returning NULL when we meant to return the refname from\nrefs_resolve_ref_unsafe(), or the other way around. Instrumenting this\ncode will reveal cases where refs_read_raw_ref() will fail, and\n\"errno\" and \"failure_errno\" will be set to different values.\n\nIn practice I haven't found a case where this scary bug changed\nanything in practice. The reason for that is that we'll not care about\nthe actual value of \"errno\" here per-se, but only whether:\n\n 1. We have an errno\n 2. If it's one of ENOENT, EISDIR or ENOTDIR. See the adjacent code\n added in a1c1d8170db (refs_resolve_ref_unsafe: handle d/f\n conflicts for writes, 2017-10-06)\n\nI.e. if we clobber \"failure_errno\" with \"errno\", but it happened to be\none of those three, and we'll clobber it with another one of the three\nwe were OK.\n\nPerhaps there are cases where the difference ended up mattering, but I\nhaven't found them. Instrumenting the test suite to fail if \"errno\"\nand \"failure_errno\" are different shows a lot of failures, checking if\nthey're different *and* one is but not the other is outside that list\nof three \"errno\" values yields no failures.\n\nBut let's fix the obvious bug. We should just stop paying attention to\n\"errno\" in refs_resolve_ref_unsafe(). In addition let's change the\npartial resetting of \"errno\" in files_read_raw_ref() to happen just\nbefore the \"return\", to ensure that any such bug will be more easily\nspotted in the future.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n refs.c | 2 --\n refs/files-backend.c | 3 +--\n 2 files changed, 1 insertion(+), 4 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 996ac271641..533cf5a2b2e 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1714,8 +1714,6 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \t\tif (refs_read_raw_ref(refs, refname, oid, &sb_refname,\n \t\t\t\t &read_flags, failure_errno)) {\n \t\t\t*flags |= read_flags;\n-\t\t\tif (errno)\n-\t\t\t\t*failure_errno = errno;\n \n \t\t\t/* In reading mode, refs must eventually resolve */\n \t\t\tif (resolve_flags & RESOLVE_REF_READING)\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 4b14f30d48f..85e195a2573 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -387,7 +387,6 @@ static int files_read_raw_ref(struct ref_store *ref_store, const char *refname,\n \tif (lstat(path, &st) < 0) {\n \t\tint ignore_errno;\n \t\tmyerr = errno;\n-\t\terrno = 0;\n \t\tif (myerr != ENOENT)\n \t\t\tgoto out;\n \t\tif (refs_read_raw_ref(refs->packed_ref_store, refname, oid,\n@@ -404,7 +403,6 @@ static int files_read_raw_ref(struct ref_store *ref_store, const char *refname,\n \t\tstrbuf_reset(&sb_contents);\n \t\tif (strbuf_readlink(&sb_contents, path, st.st_size) < 0) {\n \t\t\tmyerr = errno;\n-\t\t\terrno = 0;\n \t\t\tif (myerr == ENOENT || myerr == EINVAL)\n \t\t\t\t/* inconsistent with lstat; retry */\n \t\t\t\tgoto stat_ref;\n@@ -474,6 +472,7 @@ static int files_read_raw_ref(struct ref_store *ref_store, const char *refname,\n \n \tstrbuf_release(&sb_path);\n \tstrbuf_release(&sb_contents);\n+\terrno = 0;\n \treturn ret;\n }\n \n-- \n2.34.1.930.g218b4aae189\n\n"},{"id":"443567","messageId":"patch-2.3-a4d1dadb9c9-20211209T045735Z-avarab@gmail.com","threadId":"57052","inReplyTo":"cover-0.3-00000000000-20211209T045735Z-avarab@gmail.com","subject":"[PATCH 2/3] sequencer: don't use die_errno() on refs_resolve_ref_unsafe() failure","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-09T05:02:27Z","receivedAt":"2021-12-09T05:02:40Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change code that was faithfully migrated to the new \"resolve_errno\"\nAPI in ed90f04155d (refs API: make resolve_ref_unsafe() not set errno,\n2021-10-16) to stop caring about the errno at all.\n\nWhen we fail to resolve \"HEAD\" after the sequencer runs it doesn't\nreally help to say what the \"errno\" value is, since the fake backend\nerrno may or may not reflect anything real about the state of the\n\".git/HEAD\". With the upcoming reftable backend this fakery will\nbecome even more pronounced.\n\nSo let's just die() instead of die_errno() here. This will also help\nsimplify the refs_resolve_ref_unsafe() API. This was the only user of\nit that wasn't ignoring the \"failure_errno\" output parameter.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n sequencer.c | 10 ++++------\n 1 file changed, 4 insertions(+), 6 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex b4135a78c91..a649bd737ba 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -1285,7 +1285,7 @@ void print_commit_summary(struct repository *r,\n \tstruct strbuf author_ident = STRBUF_INIT;\n \tstruct strbuf committer_ident = STRBUF_INIT;\n \tstruct ref_store *refs;\n-\tint resolve_errno;\n+\tint ignore_errno;\n \n \tcommit = lookup_commit(r, oid);\n \tif (!commit)\n@@ -1337,11 +1337,9 @@ void print_commit_summary(struct repository *r,\n \n \trefs = get_main_ref_store(the_repository);\n \thead = refs_resolve_ref_unsafe(refs, \"HEAD\", 0, NULL, NULL,\n-\t\t\t\t &resolve_errno);\n-\tif (!head) {\n-\t\terrno = resolve_errno;\n-\t\tdie_errno(_(\"unable to resolve HEAD after creating commit\"));\n-\t}\n+\t\t\t\t &ignore_errno);\n+\tif (!head)\n+\t\tdie(_(\"unable to resolve HEAD after creating commit\"));\n \tif (!strcmp(head, \"HEAD\"))\n \t\thead = _(\"detached HEAD\");\n \telse\n-- \n2.34.1.930.g218b4aae189\n\n"},{"id":"443568","messageId":"patch-3.3-a42539d103c-20211209T045735Z-avarab@gmail.com","threadId":"57052","inReplyTo":"cover-0.3-00000000000-20211209T045735Z-avarab@gmail.com","subject":"[PATCH 3/3] refs API: remove \"failure_errno\" from refs_resolve_ref_unsafe()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-09T05:02:28Z","receivedAt":"2021-12-09T05:02:40Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Remove the now-unused \"failure_errno\" parameter from the\nrefs_resolve_ref_unsafe() signature. In my recent 96f6623ada0 (Merge\nbranch 'ab/refs-errno-cleanup', 2021-11-29) series we made all of its\ncallers explicitly request the errno via an output parameter.\n\nAs that series shows all but one caller ended up passing in a\nboilerplate \"ignore_errno\", since they only cared about whether the\nreturn value was NULL or not, i.e. if the ref could be resolved.\n\nThere was one caller left in sequencer.c that used the\n\"failure_errnO', but as of the preceding commit it doesn't use that\n\"failure_errno\" either.\n\nSo let's remove this output parameter. Not only isn't it used now, but\nwe'd like to slowly move the refs API to a more file-backend\nindependent way of communicating error codes, having it use a\n\"failure_errno\" was only the first step in that direction. If this or\nany other function needs to communicate what specifically is wrong\nwith the requested \"refname\" it'll be better to have the function set\nsome output enum of well-defined error states than piggy-backend on\n\"errno\".\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n refs.c | 51 +++++++++++++--------------------------\n refs.h | 7 +-----\n refs/files-backend.c | 31 +++++++-----------------\n sequencer.c | 4 +--\n t/helper/test-ref-store.c | 3 +--\n worktree.c | 11 +++------\n 6 files changed, 32 insertions(+), 75 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 533cf5a2b2e..4b1f990213d 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -269,10 +269,9 @@ char *refs_resolve_refdup(struct ref_store *refs,\n \t\t\t struct object_id *oid, int *flags)\n {\n \tconst char *result;\n-\tint ignore_errno;\n \n \tresult = refs_resolve_ref_unsafe(refs, refname, resolve_flags,\n-\t\t\t\t\t oid, flags, &ignore_errno);\n+\t\t\t\t\t oid, flags);\n \treturn xstrdup_or_null(result);\n }\n \n@@ -294,11 +293,10 @@ struct ref_filter {\n \n int read_ref_full(const char *refname, int resolve_flags, struct object_id *oid, int *flags)\n {\n-\tint ignore_errno;\n \tstruct ref_store *refs = get_main_ref_store(the_repository);\n \n \tif (refs_resolve_ref_unsafe(refs, refname, resolve_flags,\n-\t\t\t\t oid, flags, &ignore_errno))\n+\t\t\t\t oid, flags))\n \t\treturn 0;\n \treturn -1;\n }\n@@ -310,9 +308,8 @@ int read_ref(const char *refname, struct object_id *oid)\n \n int refs_ref_exists(struct ref_store *refs, const char *refname)\n {\n-\tint ignore_errno;\n \treturn !!refs_resolve_ref_unsafe(refs, refname, RESOLVE_REF_READING,\n-\t\t\t\t\t NULL, NULL, &ignore_errno);\n+\t\t\t\t\t NULL, NULL);\n }\n \n int ref_exists(const char *refname)\n@@ -656,15 +653,13 @@ int expand_ref(struct repository *repo, const char *str, int len,\n \t\tstruct object_id *this_result;\n \t\tint flag;\n \t\tstruct ref_store *refs = get_main_ref_store(repo);\n-\t\tint ignore_errno;\n \n \t\tthis_result = refs_found ? &oid_from_ref : oid;\n \t\tstrbuf_reset(&fullref);\n \t\tstrbuf_addf(&fullref, *p, len, str);\n \t\tr = refs_resolve_ref_unsafe(refs, fullref.buf,\n \t\t\t\t\t RESOLVE_REF_READING,\n-\t\t\t\t\t this_result, &flag,\n-\t\t\t\t\t &ignore_errno);\n+\t\t\t\t\t this_result, &flag);\n \t\tif (r) {\n \t\t\tif (!refs_found++)\n \t\t\t\t*ref = xstrdup(r);\n@@ -693,14 +688,12 @@ int repo_dwim_log(struct repository *r, const char *str, int len,\n \tfor (p = ref_rev_parse_rules; *p; p++) {\n \t\tstruct object_id hash;\n \t\tconst char *ref, *it;\n-\t\tint ignore_errno;\n \n \t\tstrbuf_reset(&path);\n \t\tstrbuf_addf(&path, *p, len, str);\n \t\tref = refs_resolve_ref_unsafe(refs, path.buf,\n \t\t\t\t\t RESOLVE_REF_READING,\n-\t\t\t\t\t oid ? &hash : NULL, NULL,\n-\t\t\t\t\t &ignore_errno);\n+\t\t\t\t\t oid ? &hash : NULL, NULL);\n \t\tif (!ref)\n \t\t\tcontinue;\n \t\tif (refs_reflog_exists(refs, path.buf))\n@@ -1382,10 +1375,9 @@ int refs_head_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)\n {\n \tstruct object_id oid;\n \tint flag;\n-\tint ignore_errno;\n \n \tif (refs_resolve_ref_unsafe(refs, \"HEAD\", RESOLVE_REF_READING,\n-\t\t\t\t &oid, &flag, &ignore_errno))\n+\t\t\t\t &oid, &flag))\n \t\treturn fn(\"HEAD\", &oid, flag, cb_data);\n \n \treturn 0;\n@@ -1674,15 +1666,13 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \t\t\t\t const char *refname,\n \t\t\t\t int resolve_flags,\n \t\t\t\t struct object_id *oid,\n-\t\t\t\t int *flags, int *failure_errno)\n+\t\t\t\t int *flags)\n {\n \tstatic struct strbuf sb_refname = STRBUF_INIT;\n \tstruct object_id unused_oid;\n \tint unused_flags;\n \tint symref_count;\n \n-\tassert(failure_errno);\n-\n \tif (!oid)\n \t\toid = &unused_oid;\n \tif (!flags)\n@@ -1692,10 +1682,8 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \n \tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {\n \t\tif (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||\n-\t\t !refname_is_safe(refname)) {\n-\t\t\t*failure_errno = EINVAL;\n+\t\t !refname_is_safe(refname))\n \t\t\treturn NULL;\n-\t\t}\n \n \t\t/*\n \t\t * dwim_ref() uses REF_ISBROKEN to distinguish between\n@@ -1710,9 +1698,10 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \n \tfor (symref_count = 0; symref_count < SYMREF_MAXDEPTH; symref_count++) {\n \t\tunsigned int read_flags = 0;\n+\t\tint failure_errno;\n \n \t\tif (refs_read_raw_ref(refs, refname, oid, &sb_refname,\n-\t\t\t\t &read_flags, failure_errno)) {\n+\t\t\t\t &read_flags, &failure_errno)) {\n \t\t\t*flags |= read_flags;\n \n \t\t\t/* In reading mode, refs must eventually resolve */\n@@ -1724,9 +1713,9 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \t\t\t * may show errors besides ENOENT if there are\n \t\t\t * similarly-named refs.\n \t\t\t */\n-\t\t\tif (*failure_errno != ENOENT &&\n-\t\t\t *failure_errno != EISDIR &&\n-\t\t\t *failure_errno != ENOTDIR)\n+\t\t\tif (failure_errno != ENOENT &&\n+\t\t\t failure_errno != EISDIR &&\n+\t\t\t failure_errno != ENOTDIR)\n \t\t\t\treturn NULL;\n \n \t\t\toidclr(oid);\n@@ -1752,16 +1741,13 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \t\t}\n \t\tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {\n \t\t\tif (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||\n-\t\t\t !refname_is_safe(refname)) {\n-\t\t\t\t*failure_errno = EINVAL;\n+\t\t\t !refname_is_safe(refname))\n \t\t\t\treturn NULL;\n-\t\t\t}\n \n \t\t\t*flags |= REF_ISBROKEN | REF_BAD_NAME;\n \t\t}\n \t}\n \n-\t*failure_errno = ELOOP;\n \treturn NULL;\n }\n \n@@ -1776,10 +1762,8 @@ int refs_init_db(struct strbuf *err)\n const char *resolve_ref_unsafe(const char *refname, int resolve_flags,\n \t\t\t struct object_id *oid, int *flags)\n {\n-\tint ignore_errno;\n-\n \treturn refs_resolve_ref_unsafe(get_main_ref_store(the_repository), refname,\n-\t\t\t\t resolve_flags, oid, flags, &ignore_errno);\n+\t\t\t\t resolve_flags, oid, flags);\n }\n \n int resolve_gitlink_ref(const char *submodule, const char *refname,\n@@ -1787,15 +1771,14 @@ int resolve_gitlink_ref(const char *submodule, const char *refname,\n {\n \tstruct ref_store *refs;\n \tint flags;\n-\tint ignore_errno;\n \n \trefs = get_submodule_ref_store(submodule);\n \n \tif (!refs)\n \t\treturn -1;\n \n-\tif (!refs_resolve_ref_unsafe(refs, refname, 0, oid, &flags,\n-\t\t\t\t &ignore_errno) || is_null_oid(oid))\n+\tif (!refs_resolve_ref_unsafe(refs, refname, 0, oid, &flags) ||\n+\t is_null_oid(oid))\n \t\treturn -1;\n \treturn 0;\n }\ndiff --git a/refs.h b/refs.h\nindex 45c34e99e3a..859dfe946af 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -58,11 +58,6 @@ struct worktree;\n * resolved. The function returns NULL for such ref names.\n * Caps and underscores refers to the special refs, such as HEAD,\n * FETCH_HEAD and friends, that all live outside of the refs/ directory.\n- *\n- * Callers should not inspect \"errno\" on failure, but rather pass in a\n- * \"failure_errno\" parameter, on failure the \"errno\" will indicate the\n- * type of failure encountered, but not necessarily one that came from\n- * a syscall. We might have faked it up.\n */\n #define RESOLVE_REF_READING 0x01\n #define RESOLVE_REF_NO_RECURSE 0x02\n@@ -72,7 +67,7 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \t\t\t\t const char *refname,\n \t\t\t\t int resolve_flags,\n \t\t\t\t struct object_id *oid,\n-\t\t\t\t int *flags, int *failure_errno);\n+\t\t\t\t int *flags);\n \n const char *resolve_ref_unsafe(const char *refname, int resolve_flags,\n \t\t\t struct object_id *oid, int *flags);\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 85e195a2573..0356debdaff 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -282,11 +282,10 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,\n \t\t\t\t\t create_dir_entry(dir->cache, refname.buf,\n \t\t\t\t\t\t\t refname.len));\n \t\t} else {\n-\t\t\tint ignore_errno;\n \t\t\tif (!refs_resolve_ref_unsafe(&refs->base,\n \t\t\t\t\t\t refname.buf,\n \t\t\t\t\t\t RESOLVE_REF_READING,\n-\t\t\t\t\t\t &oid, &flag, &ignore_errno)) {\n+\t\t\t\t\t\t &oid, &flag)) {\n \t\t\t\toidclr(&oid);\n \t\t\t\tflag |= REF_ISBROKEN;\n \t\t\t} else if (is_null_oid(&oid)) {\n@@ -1011,7 +1010,6 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,\n {\n \tstruct strbuf ref_file = STRBUF_INIT;\n \tstruct ref_lock *lock;\n-\tint ignore_errno;\n \n \tfiles_assert_main_repository(refs, \"lock_ref_oid_basic\");\n \tassert(err);\n@@ -1039,7 +1037,7 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,\n \t}\n \n \tif (!refs_resolve_ref_unsafe(&refs->base, lock->ref_name, 0,\n-\t\t\t\t &lock->old_oid, NULL, &ignore_errno))\n+\t\t\t\t &lock->old_oid, NULL))\n \t\toidclr(&lock->old_oid);\n \tgoto out;\n \n@@ -1403,7 +1401,6 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,\n \tstruct strbuf tmp_renamed_log = STRBUF_INIT;\n \tint log, ret;\n \tstruct strbuf err = STRBUF_INIT;\n-\tint ignore_errno;\n \n \tfiles_reflog_path(refs, &sb_oldref, oldrefname);\n \tfiles_reflog_path(refs, &sb_newref, newrefname);\n@@ -1417,7 +1414,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,\n \n \tif (!refs_resolve_ref_unsafe(&refs->base, oldrefname,\n \t\t\t\t RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n-\t\t\t\t &orig_oid, &flag, &ignore_errno)) {\n+\t\t\t\t &orig_oid, &flag)) {\n \t\tret = error(\"refname %s not found\", oldrefname);\n \t\tgoto out;\n \t}\n@@ -1463,7 +1460,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,\n \t */\n \tif (!copy && refs_resolve_ref_unsafe(&refs->base, newrefname,\n \t\t\t\t\t RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n-\t\t\t\t\t NULL, NULL, &ignore_errno) &&\n+\t\t\t\t\t NULL, NULL) &&\n \t refs_delete_ref(&refs->base, NULL, newrefname,\n \t\t\t NULL, REF_NO_DEREF)) {\n \t\tif (errno == EISDIR) {\n@@ -1828,12 +1825,10 @@ static int commit_ref_update(struct files_ref_store *refs,\n \t\t */\n \t\tint head_flag;\n \t\tconst char *head_ref;\n-\t\tint ignore_errno;\n \n \t\thead_ref = refs_resolve_ref_unsafe(&refs->base, \"HEAD\",\n \t\t\t\t\t\t RESOLVE_REF_READING,\n-\t\t\t\t\t\t NULL, &head_flag,\n-\t\t\t\t\t\t &ignore_errno);\n+\t\t\t\t\t\t NULL, &head_flag);\n \t\tif (head_ref && (head_flag & REF_ISSYMREF) &&\n \t\t !strcmp(head_ref, lock->ref_name)) {\n \t\t\tstruct strbuf log_err = STRBUF_INIT;\n@@ -1877,12 +1872,10 @@ static void update_symref_reflog(struct files_ref_store *refs,\n {\n \tstruct strbuf err = STRBUF_INIT;\n \tstruct object_id new_oid;\n-\tint ignore_errno;\n \n \tif (logmsg &&\n \t refs_resolve_ref_unsafe(&refs->base, target,\n-\t\t\t\t RESOLVE_REF_READING, &new_oid, NULL,\n-\t\t\t\t &ignore_errno) &&\n+\t\t\t\t RESOLVE_REF_READING, &new_oid, NULL) &&\n \t files_log_ref_write(refs, refname, &lock->old_oid,\n \t\t\t\t&new_oid, logmsg, 0, &err)) {\n \t\terror(\"%s\", err.buf);\n@@ -2156,7 +2149,6 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator)\n \t\t(struct files_reflog_iterator *)ref_iterator;\n \tstruct dir_iterator *diter = iter->dir_iterator;\n \tint ok;\n-\tint ignore_errno;\n \n \twhile ((ok = dir_iterator_advance(diter)) == ITER_OK) {\n \t\tint flags;\n@@ -2170,8 +2162,7 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator)\n \n \t\tif (!refs_resolve_ref_unsafe(iter->ref_store,\n \t\t\t\t\t diter->relative_path, 0,\n-\t\t\t\t\t &iter->oid, &flags,\n-\t\t\t\t\t &ignore_errno)) {\n+\t\t\t\t\t &iter->oid, &flags)) {\n \t\t\terror(\"bad ref for %s\", diter->path.buf);\n \t\t\tcontinue;\n \t\t}\n@@ -2515,11 +2506,9 @@ static int lock_ref_for_update(struct files_ref_store *refs,\n \t\t\t * the transaction, so we have to read it here\n \t\t\t * to record and possibly check old_oid:\n \t\t\t */\n-\t\t\tint ignore_errno;\n \t\t\tif (!refs_resolve_ref_unsafe(&refs->base,\n \t\t\t\t\t\t referent.buf, 0,\n-\t\t\t\t\t\t &lock->old_oid, NULL,\n-\t\t\t\t\t\t &ignore_errno)) {\n+\t\t\t\t\t\t &lock->old_oid, NULL)) {\n \t\t\t\tif (update->flags & REF_HAVE_OLD) {\n \t\t\t\t\tstrbuf_addf(err, \"cannot lock ref '%s': \"\n \t\t\t\t\t\t \"error reading reference\",\n@@ -3210,14 +3199,12 @@ static int files_reflog_expire(struct ref_store *ref_store,\n \n \t\tif ((flags & EXPIRE_REFLOGS_UPDATE_REF) &&\n \t\t !is_null_oid(&cb.last_kept_oid)) {\n-\t\t\tint ignore_errno;\n \t\t\tint type;\n \t\t\tconst char *ref;\n \n \t\t\tref = refs_resolve_ref_unsafe(&refs->base, refname,\n \t\t\t\t\t\t RESOLVE_REF_NO_RECURSE,\n-\t\t\t\t\t\t NULL, &type,\n-\t\t\t\t\t\t &ignore_errno);\n+\t\t\t\t\t\t NULL, &type);\n \t\t\tupdate = !!(ref && !(type & REF_ISSYMREF));\n \t\t}\n \ndiff --git a/sequencer.c b/sequencer.c\nindex a649bd737ba..d2002513cbe 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -1285,7 +1285,6 @@ void print_commit_summary(struct repository *r,\n \tstruct strbuf author_ident = STRBUF_INIT;\n \tstruct strbuf committer_ident = STRBUF_INIT;\n \tstruct ref_store *refs;\n-\tint ignore_errno;\n \n \tcommit = lookup_commit(r, oid);\n \tif (!commit)\n@@ -1336,8 +1335,7 @@ void print_commit_summary(struct repository *r,\n \tdiff_setup_done(&rev.diffopt);\n \n \trefs = get_main_ref_store(the_repository);\n-\thead = refs_resolve_ref_unsafe(refs, \"HEAD\", 0, NULL, NULL,\n-\t\t\t\t &ignore_errno);\n+\thead = refs_resolve_ref_unsafe(refs, \"HEAD\", 0, NULL, NULL);\n \tif (!head)\n \t\tdie(_(\"unable to resolve HEAD after creating commit\"));\n \tif (!strcmp(head, \"HEAD\"))\ndiff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c\nindex 3986665037a..b314b81a45b 100644\n--- a/t/helper/test-ref-store.c\n+++ b/t/helper/test-ref-store.c\n@@ -123,10 +123,9 @@ static int cmd_resolve_ref(struct ref_store *refs, const char **argv)\n \tint resolve_flags = arg_flags(*argv++, \"resolve-flags\");\n \tint flags;\n \tconst char *ref;\n-\tint ignore_errno;\n \n \tref = refs_resolve_ref_unsafe(refs, refname, resolve_flags,\n-\t\t\t\t &oid, &flags, &ignore_errno);\n+\t\t\t\t &oid, &flags);\n \tprintf(\"%s %s 0x%x\\n\", oid_to_hex(&oid), ref ? ref : \"(null)\", flags);\n \treturn ref ? 0 : 1;\n }\ndiff --git a/worktree.c b/worktree.c\nindex 2c155b10150..b127eda266b 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -28,13 +28,11 @@ static void add_head_info(struct worktree *wt)\n {\n \tint flags;\n \tconst char *target;\n-\tint ignore_errno;\n \n \ttarget = refs_resolve_ref_unsafe(get_worktree_ref_store(wt),\n \t\t\t\t\t \"HEAD\",\n \t\t\t\t\t 0,\n-\t\t\t\t\t &wt->head_oid, &flags,\n-\t\t\t\t\t &ignore_errno);\n+\t\t\t\t\t &wt->head_oid, &flags);\n \tif (!target)\n \t\treturn;\n \n@@ -420,7 +418,6 @@ const struct worktree *find_shared_symref(const char *symref,\n \t\tconst char *symref_target;\n \t\tstruct ref_store *refs;\n \t\tint flags;\n-\t\tint ignore_errno;\n \n \t\tif (wt->is_bare)\n \t\t\tcontinue;\n@@ -438,8 +435,7 @@ const struct worktree *find_shared_symref(const char *symref,\n \n \t\trefs = get_worktree_ref_store(wt);\n \t\tsymref_target = refs_resolve_ref_unsafe(refs, symref, 0,\n-\t\t\t\t\t\t\tNULL, &flags,\n-\t\t\t\t\t\t\t&ignore_errno);\n+\t\t\t\t\t\t\tNULL, &flags);\n \t\tif ((flags & REF_ISSYMREF) &&\n \t\t symref_target && !strcmp(symref_target, target)) {\n \t\t\texisting = wt;\n@@ -567,7 +563,6 @@ int other_head_refs(each_ref_fn fn, void *cb_data)\n \t\tstruct worktree *wt = *p;\n \t\tstruct object_id oid;\n \t\tint flag;\n-\t\tint ignore_errno;\n \n \t\tif (wt->is_current)\n \t\t\tcontinue;\n@@ -577,7 +572,7 @@ int other_head_refs(each_ref_fn fn, void *cb_data)\n \t\tif (refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n \t\t\t\t\t refname.buf,\n \t\t\t\t\t RESOLVE_REF_READING,\n-\t\t\t\t\t &oid, &flag, &ignore_errno))\n+\t\t\t\t\t &oid, &flag))\n \t\t\tret = fn(refname.buf, &oid, flag, cb_data);\n \t\tif (ret)\n \t\t\tbreak;\n-- \n2.34.1.930.g218b4aae189\n\n"},{"id":"443914","messageId":"cover-v2-0.3-00000000000-20211212T195108Z-avarab@gmail.com","threadId":"57052","inReplyTo":"cover-0.3-00000000000-20211209T045735Z-avarab@gmail.com","subject":"[PATCH v2 0/3] refs: ab/refs-errno-cleanup fixup + remove \"failure_errno\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-12T19:53:10Z","receivedAt":"2021-12-12T19:54:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"A re-roll of a fix to my recently landed topic to remove \"errno\" use\nfrom the refs API.\n\nThe only update is a rebasing on \"master\", this previouly semantically\nconflicted with an in-flight topic that's been merged to \"master\".\n\n1. https://lore.kernel.org/git/cover-0.3-00000000000-20211209T045735Z-avarab@gmail.com/\n\nÆvar Arnfjörð Bjarmason (3):\n refs API: use \"failure_errno\", not \"errno\"\n sequencer: don't use die_errno() on refs_resolve_ref_unsafe() failure\n refs API: remove \"failure_errno\" from refs_resolve_ref_unsafe()\n\n refs.c | 53 +++++++++++++--------------------------\n refs.h | 7 +-----\n refs/files-backend.c | 34 ++++++++-----------------\n remote.c | 3 +--\n sequencer.c | 10 +++-----\n t/helper/test-ref-store.c | 3 +--\n worktree.c | 11 +++-----\n 7 files changed, 36 insertions(+), 85 deletions(-)\n\nRange-diff against v1:\n1: b983d3b6033 = 1: 161fcad1578 refs API: use \"failure_errno\", not \"errno\"\n2: a4d1dadb9c9 = 2: 12d453d3884 sequencer: don't use die_errno() on refs_resolve_ref_unsafe() failure\n3: a42539d103c ! 3: 614590b2d10 refs API: remove \"failure_errno\" from refs_resolve_ref_unsafe()\n @@ refs/files-backend.c: static int files_reflog_expire(struct ref_store *ref_store\n \t\t}\n \n \n + ## remote.c ##\n +@@ remote.c: static void read_config(struct repository *repo)\n + \n + \trepo->remote_state->current_branch = NULL;\n + \tif (startup_info->have_repository) {\n +-\t\tint ignore_errno;\n + \t\tconst char *head_ref = refs_resolve_ref_unsafe(\n +-\t\t\tget_main_ref_store(repo), \"HEAD\", 0, NULL, &flag, &ignore_errno);\n ++\t\t\tget_main_ref_store(repo), \"HEAD\", 0, NULL, &flag);\n + \t\tif (head_ref && (flag & REF_ISSYMREF) &&\n + \t\t skip_prefix(head_ref, \"refs/heads/\", &head_ref)) {\n + \t\t\trepo->remote_state->current_branch = make_branch(\n +\n ## sequencer.c ##\n @@ sequencer.c: void print_commit_summary(struct repository *r,\n \tstruct strbuf author_ident = STRBUF_INIT;\n-- \n2.34.1.929.ge922d848c7a\n\n"},{"id":"443915","messageId":"patch-v2-1.3-161fcad1578-20211212T195108Z-avarab@gmail.com","threadId":"57052","inReplyTo":"cover-v2-0.3-00000000000-20211212T195108Z-avarab@gmail.com","subject":"[PATCH v2 1/3] refs API: use \"failure_errno\", not \"errno\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-12T19:53:11Z","receivedAt":"2021-12-12T19:54:26Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a logic error in refs_resolve_ref_unsafe() introduced in a recent\nseries of mine to abstract the refs API away from errno. See\n96f6623ada0 (Merge branch 'ab/refs-errno-cleanup', 2021-11-29)for that\nseries.\n\nIn that series introduction of \"failure_errno\" to\nrefs_resolve_ref_unsafe came in ef18119dec8 (refs API: add a version\nof refs_resolve_ref_unsafe() with \"errno\", 2021-10-16). There we'd set\n\"errno = 0\" immediately before refs_read_raw_ref(), and then set\n\"failure_errno\" to \"errno\" if errno was non-zero afterwards.\n\nThen in the next commit 8b72fea7e91 (refs API: make\nrefs_read_raw_ref() not set errno, 2021-10-16) we started expecting\n\"refs_read_raw_ref()\" to set \"failure_errno\". It would do that if\nrefs_read_raw_ref() failed, but it wouldn't be the same errno.\n\nSo we might set the \"errno\" here to any arbitrary bad value, and end\nup e.g. returning NULL when we meant to return the refname from\nrefs_resolve_ref_unsafe(), or the other way around. Instrumenting this\ncode will reveal cases where refs_read_raw_ref() will fail, and\n\"errno\" and \"failure_errno\" will be set to different values.\n\nIn practice I haven't found a case where this scary bug changed\nanything in practice. The reason for that is that we'll not care about\nthe actual value of \"errno\" here per-se, but only whether:\n\n 1. We have an errno\n 2. If it's one of ENOENT, EISDIR or ENOTDIR. See the adjacent code\n added in a1c1d8170db (refs_resolve_ref_unsafe: handle d/f\n conflicts for writes, 2017-10-06)\n\nI.e. if we clobber \"failure_errno\" with \"errno\", but it happened to be\none of those three, and we'll clobber it with another one of the three\nwe were OK.\n\nPerhaps there are cases where the difference ended up mattering, but I\nhaven't found them. Instrumenting the test suite to fail if \"errno\"\nand \"failure_errno\" are different shows a lot of failures, checking if\nthey're different *and* one is but not the other is outside that list\nof three \"errno\" values yields no failures.\n\nBut let's fix the obvious bug. We should just stop paying attention to\n\"errno\" in refs_resolve_ref_unsafe(). In addition let's change the\npartial resetting of \"errno\" in files_read_raw_ref() to happen just\nbefore the \"return\", to ensure that any such bug will be more easily\nspotted in the future.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n refs.c | 2 --\n refs/files-backend.c | 3 +--\n 2 files changed, 1 insertion(+), 4 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 4338875d86b..6cc845c7ac2 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1721,8 +1721,6 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \t\tif (refs_read_raw_ref(refs, refname, oid, &sb_refname,\n \t\t\t\t &read_flags, failure_errno)) {\n \t\t\t*flags |= read_flags;\n-\t\t\tif (errno)\n-\t\t\t\t*failure_errno = errno;\n \n \t\t\t/* In reading mode, refs must eventually resolve */\n \t\t\tif (resolve_flags & RESOLVE_REF_READING)\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 237a2afb5d7..fd34d344c30 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -387,7 +387,6 @@ static int files_read_raw_ref(struct ref_store *ref_store, const char *refname,\n \tif (lstat(path, &st) < 0) {\n \t\tint ignore_errno;\n \t\tmyerr = errno;\n-\t\terrno = 0;\n \t\tif (myerr != ENOENT)\n \t\t\tgoto out;\n \t\tif (refs_read_raw_ref(refs->packed_ref_store, refname, oid,\n@@ -404,7 +403,6 @@ static int files_read_raw_ref(struct ref_store *ref_store, const char *refname,\n \t\tstrbuf_reset(&sb_contents);\n \t\tif (strbuf_readlink(&sb_contents, path, st.st_size) < 0) {\n \t\t\tmyerr = errno;\n-\t\t\terrno = 0;\n \t\t\tif (myerr == ENOENT || myerr == EINVAL)\n \t\t\t\t/* inconsistent with lstat; retry */\n \t\t\t\tgoto stat_ref;\n@@ -474,6 +472,7 @@ static int files_read_raw_ref(struct ref_store *ref_store, const char *refname,\n \n \tstrbuf_release(&sb_path);\n \tstrbuf_release(&sb_contents);\n+\terrno = 0;\n \treturn ret;\n }\n \n-- \n2.34.1.929.ge922d848c7a\n\n"},{"id":"443916","messageId":"patch-v2-2.3-12d453d3884-20211212T195108Z-avarab@gmail.com","threadId":"57052","inReplyTo":"cover-v2-0.3-00000000000-20211212T195108Z-avarab@gmail.com","subject":"[PATCH v2 2/3] sequencer: don't use die_errno() on refs_resolve_ref_unsafe() failure","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-12T19:53:12Z","receivedAt":"2021-12-12T19:54:27Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change code that was faithfully migrated to the new \"resolve_errno\"\nAPI in ed90f04155d (refs API: make resolve_ref_unsafe() not set errno,\n2021-10-16) to stop caring about the errno at all.\n\nWhen we fail to resolve \"HEAD\" after the sequencer runs it doesn't\nreally help to say what the \"errno\" value is, since the fake backend\nerrno may or may not reflect anything real about the state of the\n\".git/HEAD\". With the upcoming reftable backend this fakery will\nbecome even more pronounced.\n\nSo let's just die() instead of die_errno() here. This will also help\nsimplify the refs_resolve_ref_unsafe() API. This was the only user of\nit that wasn't ignoring the \"failure_errno\" output parameter.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n sequencer.c | 10 ++++------\n 1 file changed, 4 insertions(+), 6 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 795b370dd34..62bf6cac66b 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -1285,7 +1285,7 @@ void print_commit_summary(struct repository *r,\n \tstruct strbuf author_ident = STRBUF_INIT;\n \tstruct strbuf committer_ident = STRBUF_INIT;\n \tstruct ref_store *refs;\n-\tint resolve_errno;\n+\tint ignore_errno;\n \n \tcommit = lookup_commit(r, oid);\n \tif (!commit)\n@@ -1337,11 +1337,9 @@ void print_commit_summary(struct repository *r,\n \n \trefs = get_main_ref_store(the_repository);\n \thead = refs_resolve_ref_unsafe(refs, \"HEAD\", 0, NULL, NULL,\n-\t\t\t\t &resolve_errno);\n-\tif (!head) {\n-\t\terrno = resolve_errno;\n-\t\tdie_errno(_(\"unable to resolve HEAD after creating commit\"));\n-\t}\n+\t\t\t\t &ignore_errno);\n+\tif (!head)\n+\t\tdie(_(\"unable to resolve HEAD after creating commit\"));\n \tif (!strcmp(head, \"HEAD\"))\n \t\thead = _(\"detached HEAD\");\n \telse\n-- \n2.34.1.929.ge922d848c7a\n\n"},{"id":"443917","messageId":"patch-v2-3.3-614590b2d10-20211212T195108Z-avarab@gmail.com","threadId":"57052","inReplyTo":"cover-v2-0.3-00000000000-20211212T195108Z-avarab@gmail.com","subject":"[PATCH v2 3/3] refs API: remove \"failure_errno\" from refs_resolve_ref_unsafe()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-12T19:53:13Z","receivedAt":"2021-12-12T19:54:29Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Remove the now-unused \"failure_errno\" parameter from the\nrefs_resolve_ref_unsafe() signature. In my recent 96f6623ada0 (Merge\nbranch 'ab/refs-errno-cleanup', 2021-11-29) series we made all of its\ncallers explicitly request the errno via an output parameter.\n\nAs that series shows all but one caller ended up passing in a\nboilerplate \"ignore_errno\", since they only cared about whether the\nreturn value was NULL or not, i.e. if the ref could be resolved.\n\nThere was one caller left in sequencer.c that used the\n\"failure_errnO', but as of the preceding commit it doesn't use that\n\"failure_errno\" either.\n\nSo let's remove this output parameter. Not only isn't it used now, but\nwe'd like to slowly move the refs API to a more file-backend\nindependent way of communicating error codes, having it use a\n\"failure_errno\" was only the first step in that direction. If this or\nany other function needs to communicate what specifically is wrong\nwith the requested \"refname\" it'll be better to have the function set\nsome output enum of well-defined error states than piggy-backend on\n\"errno\".\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n refs.c | 51 +++++++++++++--------------------------\n refs.h | 7 +-----\n refs/files-backend.c | 31 +++++++-----------------\n remote.c | 3 +--\n sequencer.c | 4 +--\n t/helper/test-ref-store.c | 3 +--\n worktree.c | 11 +++------\n 7 files changed, 33 insertions(+), 77 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 6cc845c7ac2..1ab97f99e23 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -269,10 +269,9 @@ char *refs_resolve_refdup(struct ref_store *refs,\n \t\t\t struct object_id *oid, int *flags)\n {\n \tconst char *result;\n-\tint ignore_errno;\n \n \tresult = refs_resolve_ref_unsafe(refs, refname, resolve_flags,\n-\t\t\t\t\t oid, flags, &ignore_errno);\n+\t\t\t\t\t oid, flags);\n \treturn xstrdup_or_null(result);\n }\n \n@@ -294,11 +293,10 @@ struct ref_filter {\n \n int read_ref_full(const char *refname, int resolve_flags, struct object_id *oid, int *flags)\n {\n-\tint ignore_errno;\n \tstruct ref_store *refs = get_main_ref_store(the_repository);\n \n \tif (refs_resolve_ref_unsafe(refs, refname, resolve_flags,\n-\t\t\t\t oid, flags, &ignore_errno))\n+\t\t\t\t oid, flags))\n \t\treturn 0;\n \treturn -1;\n }\n@@ -310,9 +308,8 @@ int read_ref(const char *refname, struct object_id *oid)\n \n int refs_ref_exists(struct ref_store *refs, const char *refname)\n {\n-\tint ignore_errno;\n \treturn !!refs_resolve_ref_unsafe(refs, refname, RESOLVE_REF_READING,\n-\t\t\t\t\t NULL, NULL, &ignore_errno);\n+\t\t\t\t\t NULL, NULL);\n }\n \n int ref_exists(const char *refname)\n@@ -656,15 +653,13 @@ int expand_ref(struct repository *repo, const char *str, int len,\n \t\tstruct object_id *this_result;\n \t\tint flag;\n \t\tstruct ref_store *refs = get_main_ref_store(repo);\n-\t\tint ignore_errno;\n \n \t\tthis_result = refs_found ? &oid_from_ref : oid;\n \t\tstrbuf_reset(&fullref);\n \t\tstrbuf_addf(&fullref, *p, len, str);\n \t\tr = refs_resolve_ref_unsafe(refs, fullref.buf,\n \t\t\t\t\t RESOLVE_REF_READING,\n-\t\t\t\t\t this_result, &flag,\n-\t\t\t\t\t &ignore_errno);\n+\t\t\t\t\t this_result, &flag);\n \t\tif (r) {\n \t\t\tif (!refs_found++)\n \t\t\t\t*ref = xstrdup(r);\n@@ -693,14 +688,12 @@ int repo_dwim_log(struct repository *r, const char *str, int len,\n \tfor (p = ref_rev_parse_rules; *p; p++) {\n \t\tstruct object_id hash;\n \t\tconst char *ref, *it;\n-\t\tint ignore_errno;\n \n \t\tstrbuf_reset(&path);\n \t\tstrbuf_addf(&path, *p, len, str);\n \t\tref = refs_resolve_ref_unsafe(refs, path.buf,\n \t\t\t\t\t RESOLVE_REF_READING,\n-\t\t\t\t\t oid ? &hash : NULL, NULL,\n-\t\t\t\t\t &ignore_errno);\n+\t\t\t\t\t oid ? &hash : NULL, NULL);\n \t\tif (!ref)\n \t\t\tcontinue;\n \t\tif (refs_reflog_exists(refs, path.buf))\n@@ -1389,10 +1382,9 @@ int refs_head_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)\n {\n \tstruct object_id oid;\n \tint flag;\n-\tint ignore_errno;\n \n \tif (refs_resolve_ref_unsafe(refs, \"HEAD\", RESOLVE_REF_READING,\n-\t\t\t\t &oid, &flag, &ignore_errno))\n+\t\t\t\t &oid, &flag))\n \t\treturn fn(\"HEAD\", &oid, flag, cb_data);\n \n \treturn 0;\n@@ -1681,15 +1673,13 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \t\t\t\t const char *refname,\n \t\t\t\t int resolve_flags,\n \t\t\t\t struct object_id *oid,\n-\t\t\t\t int *flags, int *failure_errno)\n+\t\t\t\t int *flags)\n {\n \tstatic struct strbuf sb_refname = STRBUF_INIT;\n \tstruct object_id unused_oid;\n \tint unused_flags;\n \tint symref_count;\n \n-\tassert(failure_errno);\n-\n \tif (!oid)\n \t\toid = &unused_oid;\n \tif (!flags)\n@@ -1699,10 +1689,8 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \n \tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {\n \t\tif (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||\n-\t\t !refname_is_safe(refname)) {\n-\t\t\t*failure_errno = EINVAL;\n+\t\t !refname_is_safe(refname))\n \t\t\treturn NULL;\n-\t\t}\n \n \t\t/*\n \t\t * dwim_ref() uses REF_ISBROKEN to distinguish between\n@@ -1717,9 +1705,10 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \n \tfor (symref_count = 0; symref_count < SYMREF_MAXDEPTH; symref_count++) {\n \t\tunsigned int read_flags = 0;\n+\t\tint failure_errno;\n \n \t\tif (refs_read_raw_ref(refs, refname, oid, &sb_refname,\n-\t\t\t\t &read_flags, failure_errno)) {\n+\t\t\t\t &read_flags, &failure_errno)) {\n \t\t\t*flags |= read_flags;\n \n \t\t\t/* In reading mode, refs must eventually resolve */\n@@ -1731,9 +1720,9 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \t\t\t * may show errors besides ENOENT if there are\n \t\t\t * similarly-named refs.\n \t\t\t */\n-\t\t\tif (*failure_errno != ENOENT &&\n-\t\t\t *failure_errno != EISDIR &&\n-\t\t\t *failure_errno != ENOTDIR)\n+\t\t\tif (failure_errno != ENOENT &&\n+\t\t\t failure_errno != EISDIR &&\n+\t\t\t failure_errno != ENOTDIR)\n \t\t\t\treturn NULL;\n \n \t\t\toidclr(oid);\n@@ -1759,16 +1748,13 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \t\t}\n \t\tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {\n \t\t\tif (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||\n-\t\t\t !refname_is_safe(refname)) {\n-\t\t\t\t*failure_errno = EINVAL;\n+\t\t\t !refname_is_safe(refname))\n \t\t\t\treturn NULL;\n-\t\t\t}\n \n \t\t\t*flags |= REF_ISBROKEN | REF_BAD_NAME;\n \t\t}\n \t}\n \n-\t*failure_errno = ELOOP;\n \treturn NULL;\n }\n \n@@ -1783,10 +1769,8 @@ int refs_init_db(struct strbuf *err)\n const char *resolve_ref_unsafe(const char *refname, int resolve_flags,\n \t\t\t struct object_id *oid, int *flags)\n {\n-\tint ignore_errno;\n-\n \treturn refs_resolve_ref_unsafe(get_main_ref_store(the_repository), refname,\n-\t\t\t\t resolve_flags, oid, flags, &ignore_errno);\n+\t\t\t\t resolve_flags, oid, flags);\n }\n \n int resolve_gitlink_ref(const char *submodule, const char *refname,\n@@ -1794,15 +1778,14 @@ int resolve_gitlink_ref(const char *submodule, const char *refname,\n {\n \tstruct ref_store *refs;\n \tint flags;\n-\tint ignore_errno;\n \n \trefs = get_submodule_ref_store(submodule);\n \n \tif (!refs)\n \t\treturn -1;\n \n-\tif (!refs_resolve_ref_unsafe(refs, refname, 0, oid, &flags,\n-\t\t\t\t &ignore_errno) || is_null_oid(oid))\n+\tif (!refs_resolve_ref_unsafe(refs, refname, 0, oid, &flags) ||\n+\t is_null_oid(oid))\n \t\treturn -1;\n \treturn 0;\n }\ndiff --git a/refs.h b/refs.h\nindex bb50d1eb195..6f27d4abfc1 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -58,11 +58,6 @@ struct worktree;\n * resolved. The function returns NULL for such ref names.\n * Caps and underscores refers to the special refs, such as HEAD,\n * FETCH_HEAD and friends, that all live outside of the refs/ directory.\n- *\n- * Callers should not inspect \"errno\" on failure, but rather pass in a\n- * \"failure_errno\" parameter, on failure the \"errno\" will indicate the\n- * type of failure encountered, but not necessarily one that came from\n- * a syscall. We might have faked it up.\n */\n #define RESOLVE_REF_READING 0x01\n #define RESOLVE_REF_NO_RECURSE 0x02\n@@ -72,7 +67,7 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \t\t\t\t const char *refname,\n \t\t\t\t int resolve_flags,\n \t\t\t\t struct object_id *oid,\n-\t\t\t\t int *flags, int *failure_errno);\n+\t\t\t\t int *flags);\n \n const char *resolve_ref_unsafe(const char *refname, int resolve_flags,\n \t\t\t struct object_id *oid, int *flags);\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex fd34d344c30..4d5f14dc594 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -282,11 +282,10 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,\n \t\t\t\t\t create_dir_entry(dir->cache, refname.buf,\n \t\t\t\t\t\t\t refname.len));\n \t\t} else {\n-\t\t\tint ignore_errno;\n \t\t\tif (!refs_resolve_ref_unsafe(&refs->base,\n \t\t\t\t\t\t refname.buf,\n \t\t\t\t\t\t RESOLVE_REF_READING,\n-\t\t\t\t\t\t &oid, &flag, &ignore_errno)) {\n+\t\t\t\t\t\t &oid, &flag)) {\n \t\t\t\toidclr(&oid);\n \t\t\t\tflag |= REF_ISBROKEN;\n \t\t\t} else if (is_null_oid(&oid)) {\n@@ -1011,7 +1010,6 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,\n {\n \tstruct strbuf ref_file = STRBUF_INIT;\n \tstruct ref_lock *lock;\n-\tint ignore_errno;\n \n \tfiles_assert_main_repository(refs, \"lock_ref_oid_basic\");\n \tassert(err);\n@@ -1039,7 +1037,7 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,\n \t}\n \n \tif (!refs_resolve_ref_unsafe(&refs->base, lock->ref_name, 0,\n-\t\t\t\t &lock->old_oid, NULL, &ignore_errno))\n+\t\t\t\t &lock->old_oid, NULL))\n \t\toidclr(&lock->old_oid);\n \tgoto out;\n \n@@ -1403,7 +1401,6 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,\n \tstruct strbuf tmp_renamed_log = STRBUF_INIT;\n \tint log, ret;\n \tstruct strbuf err = STRBUF_INIT;\n-\tint ignore_errno;\n \n \tfiles_reflog_path(refs, &sb_oldref, oldrefname);\n \tfiles_reflog_path(refs, &sb_newref, newrefname);\n@@ -1417,7 +1414,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,\n \n \tif (!refs_resolve_ref_unsafe(&refs->base, oldrefname,\n \t\t\t\t RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n-\t\t\t\t &orig_oid, &flag, &ignore_errno)) {\n+\t\t\t\t &orig_oid, &flag)) {\n \t\tret = error(\"refname %s not found\", oldrefname);\n \t\tgoto out;\n \t}\n@@ -1463,7 +1460,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,\n \t */\n \tif (!copy && refs_resolve_ref_unsafe(&refs->base, newrefname,\n \t\t\t\t\t RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n-\t\t\t\t\t NULL, NULL, &ignore_errno) &&\n+\t\t\t\t\t NULL, NULL) &&\n \t refs_delete_ref(&refs->base, NULL, newrefname,\n \t\t\t NULL, REF_NO_DEREF)) {\n \t\tif (errno == EISDIR) {\n@@ -1827,12 +1824,10 @@ static int commit_ref_update(struct files_ref_store *refs,\n \t\t */\n \t\tint head_flag;\n \t\tconst char *head_ref;\n-\t\tint ignore_errno;\n \n \t\thead_ref = refs_resolve_ref_unsafe(&refs->base, \"HEAD\",\n \t\t\t\t\t\t RESOLVE_REF_READING,\n-\t\t\t\t\t\t NULL, &head_flag,\n-\t\t\t\t\t\t &ignore_errno);\n+\t\t\t\t\t\t NULL, &head_flag);\n \t\tif (head_ref && (head_flag & REF_ISSYMREF) &&\n \t\t !strcmp(head_ref, lock->ref_name)) {\n \t\t\tstruct strbuf log_err = STRBUF_INIT;\n@@ -1876,12 +1871,10 @@ static void update_symref_reflog(struct files_ref_store *refs,\n {\n \tstruct strbuf err = STRBUF_INIT;\n \tstruct object_id new_oid;\n-\tint ignore_errno;\n \n \tif (logmsg &&\n \t refs_resolve_ref_unsafe(&refs->base, target,\n-\t\t\t\t RESOLVE_REF_READING, &new_oid, NULL,\n-\t\t\t\t &ignore_errno) &&\n+\t\t\t\t RESOLVE_REF_READING, &new_oid, NULL) &&\n \t files_log_ref_write(refs, refname, &lock->old_oid,\n \t\t\t\t&new_oid, logmsg, 0, &err)) {\n \t\terror(\"%s\", err.buf);\n@@ -2155,7 +2148,6 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator)\n \t\t(struct files_reflog_iterator *)ref_iterator;\n \tstruct dir_iterator *diter = iter->dir_iterator;\n \tint ok;\n-\tint ignore_errno;\n \n \twhile ((ok = dir_iterator_advance(diter)) == ITER_OK) {\n \t\tint flags;\n@@ -2169,8 +2161,7 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator)\n \n \t\tif (!refs_resolve_ref_unsafe(iter->ref_store,\n \t\t\t\t\t diter->relative_path, 0,\n-\t\t\t\t\t &iter->oid, &flags,\n-\t\t\t\t\t &ignore_errno)) {\n+\t\t\t\t\t &iter->oid, &flags)) {\n \t\t\terror(\"bad ref for %s\", diter->path.buf);\n \t\t\tcontinue;\n \t\t}\n@@ -2514,11 +2505,9 @@ static int lock_ref_for_update(struct files_ref_store *refs,\n \t\t\t * the transaction, so we have to read it here\n \t\t\t * to record and possibly check old_oid:\n \t\t\t */\n-\t\t\tint ignore_errno;\n \t\t\tif (!refs_resolve_ref_unsafe(&refs->base,\n \t\t\t\t\t\t referent.buf, 0,\n-\t\t\t\t\t\t &lock->old_oid, NULL,\n-\t\t\t\t\t\t &ignore_errno)) {\n+\t\t\t\t\t\t &lock->old_oid, NULL)) {\n \t\t\t\tif (update->flags & REF_HAVE_OLD) {\n \t\t\t\t\tstrbuf_addf(err, \"cannot lock ref '%s': \"\n \t\t\t\t\t\t \"error reading reference\",\n@@ -3209,14 +3198,12 @@ static int files_reflog_expire(struct ref_store *ref_store,\n \n \t\tif ((flags & EXPIRE_REFLOGS_UPDATE_REF) &&\n \t\t !is_null_oid(&cb.last_kept_oid)) {\n-\t\t\tint ignore_errno;\n \t\t\tint type;\n \t\t\tconst char *ref;\n \n \t\t\tref = refs_resolve_ref_unsafe(&refs->base, refname,\n \t\t\t\t\t\t RESOLVE_REF_NO_RECURSE,\n-\t\t\t\t\t\t NULL, &type,\n-\t\t\t\t\t\t &ignore_errno);\n+\t\t\t\t\t\t NULL, &type);\n \t\t\tupdate = !!(ref && !(type & REF_ISSYMREF));\n \t\t}\n \ndiff --git a/remote.c b/remote.c\nindex a6d8ec6c1ac..c97c626eaa8 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -508,9 +508,8 @@ static void read_config(struct repository *repo)\n \n \trepo->remote_state->current_branch = NULL;\n \tif (startup_info->have_repository) {\n-\t\tint ignore_errno;\n \t\tconst char *head_ref = refs_resolve_ref_unsafe(\n-\t\t\tget_main_ref_store(repo), \"HEAD\", 0, NULL, &flag, &ignore_errno);\n+\t\t\tget_main_ref_store(repo), \"HEAD\", 0, NULL, &flag);\n \t\tif (head_ref && (flag & REF_ISSYMREF) &&\n \t\t skip_prefix(head_ref, \"refs/heads/\", &head_ref)) {\n \t\t\trepo->remote_state->current_branch = make_branch(\ndiff --git a/sequencer.c b/sequencer.c\nindex 62bf6cac66b..015e0bdd832 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -1285,7 +1285,6 @@ void print_commit_summary(struct repository *r,\n \tstruct strbuf author_ident = STRBUF_INIT;\n \tstruct strbuf committer_ident = STRBUF_INIT;\n \tstruct ref_store *refs;\n-\tint ignore_errno;\n \n \tcommit = lookup_commit(r, oid);\n \tif (!commit)\n@@ -1336,8 +1335,7 @@ void print_commit_summary(struct repository *r,\n \tdiff_setup_done(&rev.diffopt);\n \n \trefs = get_main_ref_store(the_repository);\n-\thead = refs_resolve_ref_unsafe(refs, \"HEAD\", 0, NULL, NULL,\n-\t\t\t\t &ignore_errno);\n+\thead = refs_resolve_ref_unsafe(refs, \"HEAD\", 0, NULL, NULL);\n \tif (!head)\n \t\tdie(_(\"unable to resolve HEAD after creating commit\"));\n \tif (!strcmp(head, \"HEAD\"))\ndiff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c\nindex 73461c29d3c..98f827edfb7 100644\n--- a/t/helper/test-ref-store.c\n+++ b/t/helper/test-ref-store.c\n@@ -123,10 +123,9 @@ static int cmd_resolve_ref(struct ref_store *refs, const char **argv)\n \tint resolve_flags = arg_flags(*argv++, \"resolve-flags\");\n \tint flags;\n \tconst char *ref;\n-\tint ignore_errno;\n \n \tref = refs_resolve_ref_unsafe(refs, refname, resolve_flags,\n-\t\t\t\t &oid, &flags, &ignore_errno);\n+\t\t\t\t &oid, &flags);\n \tprintf(\"%s %s 0x%x\\n\", oid_to_hex(&oid), ref ? ref : \"(null)\", flags);\n \treturn ref ? 0 : 1;\n }\ndiff --git a/worktree.c b/worktree.c\nindex 2c155b10150..b127eda266b 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -28,13 +28,11 @@ static void add_head_info(struct worktree *wt)\n {\n \tint flags;\n \tconst char *target;\n-\tint ignore_errno;\n \n \ttarget = refs_resolve_ref_unsafe(get_worktree_ref_store(wt),\n \t\t\t\t\t \"HEAD\",\n \t\t\t\t\t 0,\n-\t\t\t\t\t &wt->head_oid, &flags,\n-\t\t\t\t\t &ignore_errno);\n+\t\t\t\t\t &wt->head_oid, &flags);\n \tif (!target)\n \t\treturn;\n \n@@ -420,7 +418,6 @@ const struct worktree *find_shared_symref(const char *symref,\n \t\tconst char *symref_target;\n \t\tstruct ref_store *refs;\n \t\tint flags;\n-\t\tint ignore_errno;\n \n \t\tif (wt->is_bare)\n \t\t\tcontinue;\n@@ -438,8 +435,7 @@ const struct worktree *find_shared_symref(const char *symref,\n \n \t\trefs = get_worktree_ref_store(wt);\n \t\tsymref_target = refs_resolve_ref_unsafe(refs, symref, 0,\n-\t\t\t\t\t\t\tNULL, &flags,\n-\t\t\t\t\t\t\t&ignore_errno);\n+\t\t\t\t\t\t\tNULL, &flags);\n \t\tif ((flags & REF_ISSYMREF) &&\n \t\t symref_target && !strcmp(symref_target, target)) {\n \t\t\texisting = wt;\n@@ -567,7 +563,6 @@ int other_head_refs(each_ref_fn fn, void *cb_data)\n \t\tstruct worktree *wt = *p;\n \t\tstruct object_id oid;\n \t\tint flag;\n-\t\tint ignore_errno;\n \n \t\tif (wt->is_current)\n \t\t\tcontinue;\n@@ -577,7 +572,7 @@ int other_head_refs(each_ref_fn fn, void *cb_data)\n \t\tif (refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n \t\t\t\t\t refname.buf,\n \t\t\t\t\t RESOLVE_REF_READING,\n-\t\t\t\t\t &oid, &flag, &ignore_errno))\n+\t\t\t\t\t &oid, &flag))\n \t\t\tret = fn(refname.buf, &oid, flag, cb_data);\n \t\tif (ret)\n \t\t\tbreak;\n-- \n2.34.1.929.ge922d848c7a\n\n"},{"id":"445989","messageId":"cover-v3-0.3-00000000000-20220112T123117Z-avarab@gmail.com","threadId":"57052","inReplyTo":"cover-v2-0.3-00000000000-20211212T195108Z-avarab@gmail.com","subject":"[PATCH v3 0/3] For v2.35.0: refs: ab/refs-errno-cleanup fixup + remove \"failure_errno\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-01-12T12:36:45Z","receivedAt":"2022-01-12T12:37:20Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This is a follow-up to the recently landed ab/refs-errno-cleanup\ntopic, I missed a spot and left some meaningless use of \"errno\" in the\nrefs (file) backend.\n\nPer the $subject I hope we can get this into v2.35.0. I submitted a\nv1[1] and v2[2] of this around the holidays so I think it may have\nfallen through the cracks.\n\n1. https://lore.kernel.org/git/cover-0.3-00000000000-20211209T045735Z-avarab@gmail.com/\n2. https://lore.kernel.org/git/cover-v2-0.3-00000000000-20211212T195108Z-avarab@gmail.com/\n\nÆvar Arnfjörð Bjarmason (3):\n refs API: use \"failure_errno\", not \"errno\"\n sequencer: don't use die_errno() on refs_resolve_ref_unsafe() failure\n refs API: remove \"failure_errno\" from refs_resolve_ref_unsafe()\n\n refs.c | 53 +++++++++++++--------------------------\n refs.h | 7 +-----\n refs/files-backend.c | 34 ++++++++-----------------\n remote.c | 3 +--\n sequencer.c | 10 +++-----\n t/helper/test-ref-store.c | 3 +--\n worktree.c | 11 +++-----\n 7 files changed, 36 insertions(+), 85 deletions(-)\n\nRange-diff against v2:\n1: 161fcad1578 = 1: a45268ac24b refs API: use \"failure_errno\", not \"errno\"\n2: 12d453d3884 = 2: 8d8691a5e93 sequencer: don't use die_errno() on refs_resolve_ref_unsafe() failure\n3: 614590b2d10 ! 3: 8f937d8f64a refs API: remove \"failure_errno\" from refs_resolve_ref_unsafe()\n @@ Commit message\n return value was NULL or not, i.e. if the ref could be resolved.\n \n There was one caller left in sequencer.c that used the\n - \"failure_errnO', but as of the preceding commit it doesn't use that\n - \"failure_errno\" either.\n + \"failure_errno', but as of the preceding commit it uses a boilerplate\n + \"ignore_errno\" instead.\n \n So let's remove this output parameter. Not only isn't it used now, but\n - we'd like to slowly move the refs API to a more file-backend\n - independent way of communicating error codes, having it use a\n - \"failure_errno\" was only the first step in that direction. If this or\n - any other function needs to communicate what specifically is wrong\n - with the requested \"refname\" it'll be better to have the function set\n - some output enum of well-defined error states than piggy-backend on\n - \"errno\".\n + it's unlikely that we'll want it again in the future. We'd like to\n + slowly move the refs API to a more file-backend independent way of\n + communicating error codes, having it use a \"failure_errno\" was only\n + the first step in that direction. If this or any other function needs\n + to communicate what specifically is wrong with the requested \"refname\"\n + it'll be better to have the function set some output enum of\n + well-defined error states than piggy-backend on \"errno\".\n \n Signed-off-by: Ævar Arnfjörð Bjarmason \n \n @@ refs/files-backend.c: static int lock_ref_for_update(struct files_ref_store *ref\n \t\t\t\t\t\t \"error reading reference\",\n @@ refs/files-backend.c: static int files_reflog_expire(struct ref_store *ref_store,\n \n - \t\tif ((flags & EXPIRE_REFLOGS_UPDATE_REF) &&\n + \t\tif ((expire_flags & EXPIRE_REFLOGS_UPDATE_REF) &&\n \t\t !is_null_oid(&cb.last_kept_oid)) {\n -\t\t\tint ignore_errno;\n \t\t\tint type;\n @@ sequencer.c: void print_commit_summary(struct repository *r,\n \n ## t/helper/test-ref-store.c ##\n @@ t/helper/test-ref-store.c: static int cmd_resolve_ref(struct ref_store *refs, const char **argv)\n - \tint resolve_flags = arg_flags(*argv++, \"resolve-flags\");\n + \tint resolve_flags = arg_flags(*argv++, \"resolve-flags\", empty_flags);\n \tint flags;\n \tconst char *ref;\n -\tint ignore_errno;\n @@ worktree.c: static void add_head_info(struct worktree *wt)\n \tif (!target)\n \t\treturn;\n \n -@@ worktree.c: const struct worktree *find_shared_symref(const char *symref,\n +@@ worktree.c: const struct worktree *find_shared_symref(struct worktree **worktrees,\n \t\tconst char *symref_target;\n \t\tstruct ref_store *refs;\n \t\tint flags;\n @@ worktree.c: const struct worktree *find_shared_symref(const char *symref,\n \n \t\tif (wt->is_bare)\n \t\t\tcontinue;\n -@@ worktree.c: const struct worktree *find_shared_symref(const char *symref,\n +@@ worktree.c: const struct worktree *find_shared_symref(struct worktree **worktrees,\n \n \t\trefs = get_worktree_ref_store(wt);\n \t\tsymref_target = refs_resolve_ref_unsafe(refs, symref, 0,\n-- \n2.35.0.rc0.848.gb9d3879eb1d\n\n"},{"id":"445990","messageId":"patch-v3-1.3-a45268ac24b-20220112T123117Z-avarab@gmail.com","threadId":"57052","inReplyTo":"cover-v3-0.3-00000000000-20220112T123117Z-avarab@gmail.com","subject":"[PATCH v3 1/3] refs API: use \"failure_errno\", not \"errno\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-01-12T12:36:46Z","receivedAt":"2022-01-12T12:37:22Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a logic error in refs_resolve_ref_unsafe() introduced in a recent\nseries of mine to abstract the refs API away from errno. See\n96f6623ada0 (Merge branch 'ab/refs-errno-cleanup', 2021-11-29)for that\nseries.\n\nIn that series introduction of \"failure_errno\" to\nrefs_resolve_ref_unsafe came in ef18119dec8 (refs API: add a version\nof refs_resolve_ref_unsafe() with \"errno\", 2021-10-16). There we'd set\n\"errno = 0\" immediately before refs_read_raw_ref(), and then set\n\"failure_errno\" to \"errno\" if errno was non-zero afterwards.\n\nThen in the next commit 8b72fea7e91 (refs API: make\nrefs_read_raw_ref() not set errno, 2021-10-16) we started expecting\n\"refs_read_raw_ref()\" to set \"failure_errno\". It would do that if\nrefs_read_raw_ref() failed, but it wouldn't be the same errno.\n\nSo we might set the \"errno\" here to any arbitrary bad value, and end\nup e.g. returning NULL when we meant to return the refname from\nrefs_resolve_ref_unsafe(), or the other way around. Instrumenting this\ncode will reveal cases where refs_read_raw_ref() will fail, and\n\"errno\" and \"failure_errno\" will be set to different values.\n\nIn practice I haven't found a case where this scary bug changed\nanything in practice. The reason for that is that we'll not care about\nthe actual value of \"errno\" here per-se, but only whether:\n\n 1. We have an errno\n 2. If it's one of ENOENT, EISDIR or ENOTDIR. See the adjacent code\n added in a1c1d8170db (refs_resolve_ref_unsafe: handle d/f\n conflicts for writes, 2017-10-06)\n\nI.e. if we clobber \"failure_errno\" with \"errno\", but it happened to be\none of those three, and we'll clobber it with another one of the three\nwe were OK.\n\nPerhaps there are cases where the difference ended up mattering, but I\nhaven't found them. Instrumenting the test suite to fail if \"errno\"\nand \"failure_errno\" are different shows a lot of failures, checking if\nthey're different *and* one is but not the other is outside that list\nof three \"errno\" values yields no failures.\n\nBut let's fix the obvious bug. We should just stop paying attention to\n\"errno\" in refs_resolve_ref_unsafe(). In addition let's change the\npartial resetting of \"errno\" in files_read_raw_ref() to happen just\nbefore the \"return\", to ensure that any such bug will be more easily\nspotted in the future.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n refs.c | 2 --\n refs/files-backend.c | 3 +--\n 2 files changed, 1 insertion(+), 4 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex bd2546ae230..addb26293b4 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1722,8 +1722,6 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \t\tif (refs_read_raw_ref(refs, refname, oid, &sb_refname,\n \t\t\t\t &read_flags, failure_errno)) {\n \t\t\t*flags |= read_flags;\n-\t\t\tif (errno)\n-\t\t\t\t*failure_errno = errno;\n \n \t\t\t/* In reading mode, refs must eventually resolve */\n \t\t\tif (resolve_flags & RESOLVE_REF_READING)\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex b529fdf237e..43a3b882d7c 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -382,7 +382,6 @@ static int files_read_raw_ref(struct ref_store *ref_store, const char *refname,\n \tif (lstat(path, &st) < 0) {\n \t\tint ignore_errno;\n \t\tmyerr = errno;\n-\t\terrno = 0;\n \t\tif (myerr != ENOENT)\n \t\t\tgoto out;\n \t\tif (refs_read_raw_ref(refs->packed_ref_store, refname, oid,\n@@ -399,7 +398,6 @@ static int files_read_raw_ref(struct ref_store *ref_store, const char *refname,\n \t\tstrbuf_reset(&sb_contents);\n \t\tif (strbuf_readlink(&sb_contents, path, st.st_size) < 0) {\n \t\t\tmyerr = errno;\n-\t\t\terrno = 0;\n \t\t\tif (myerr == ENOENT || myerr == EINVAL)\n \t\t\t\t/* inconsistent with lstat; retry */\n \t\t\t\tgoto stat_ref;\n@@ -469,6 +467,7 @@ static int files_read_raw_ref(struct ref_store *ref_store, const char *refname,\n \n \tstrbuf_release(&sb_path);\n \tstrbuf_release(&sb_contents);\n+\terrno = 0;\n \treturn ret;\n }\n \n-- \n2.35.0.rc0.848.gb9d3879eb1d\n\n"},{"id":"445991","messageId":"patch-v3-2.3-8d8691a5e93-20220112T123117Z-avarab@gmail.com","threadId":"57052","inReplyTo":"cover-v3-0.3-00000000000-20220112T123117Z-avarab@gmail.com","subject":"[PATCH v3 2/3] sequencer: don't use die_errno() on refs_resolve_ref_unsafe() failure","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-01-12T12:36:47Z","receivedAt":"2022-01-12T12:37:25Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change code that was faithfully migrated to the new \"resolve_errno\"\nAPI in ed90f04155d (refs API: make resolve_ref_unsafe() not set errno,\n2021-10-16) to stop caring about the errno at all.\n\nWhen we fail to resolve \"HEAD\" after the sequencer runs it doesn't\nreally help to say what the \"errno\" value is, since the fake backend\nerrno may or may not reflect anything real about the state of the\n\".git/HEAD\". With the upcoming reftable backend this fakery will\nbecome even more pronounced.\n\nSo let's just die() instead of die_errno() here. This will also help\nsimplify the refs_resolve_ref_unsafe() API. This was the only user of\nit that wasn't ignoring the \"failure_errno\" output parameter.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n sequencer.c | 10 ++++------\n 1 file changed, 4 insertions(+), 6 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 6abd72160cc..03cdf548d72 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -1281,7 +1281,7 @@ void print_commit_summary(struct repository *r,\n \tstruct strbuf author_ident = STRBUF_INIT;\n \tstruct strbuf committer_ident = STRBUF_INIT;\n \tstruct ref_store *refs;\n-\tint resolve_errno;\n+\tint ignore_errno;\n \n \tcommit = lookup_commit(r, oid);\n \tif (!commit)\n@@ -1333,11 +1333,9 @@ void print_commit_summary(struct repository *r,\n \n \trefs = get_main_ref_store(the_repository);\n \thead = refs_resolve_ref_unsafe(refs, \"HEAD\", 0, NULL, NULL,\n-\t\t\t\t &resolve_errno);\n-\tif (!head) {\n-\t\terrno = resolve_errno;\n-\t\tdie_errno(_(\"unable to resolve HEAD after creating commit\"));\n-\t}\n+\t\t\t\t &ignore_errno);\n+\tif (!head)\n+\t\tdie(_(\"unable to resolve HEAD after creating commit\"));\n \tif (!strcmp(head, \"HEAD\"))\n \t\thead = _(\"detached HEAD\");\n \telse\n-- \n2.35.0.rc0.848.gb9d3879eb1d\n\n"},{"id":"445992","messageId":"patch-v3-3.3-8f937d8f64a-20220112T123117Z-avarab@gmail.com","threadId":"57052","inReplyTo":"cover-v3-0.3-00000000000-20220112T123117Z-avarab@gmail.com","subject":"[PATCH v3 3/3] refs API: remove \"failure_errno\" from refs_resolve_ref_unsafe()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-01-12T12:36:48Z","receivedAt":"2022-01-12T12:37:28Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Remove the now-unused \"failure_errno\" parameter from the\nrefs_resolve_ref_unsafe() signature. In my recent 96f6623ada0 (Merge\nbranch 'ab/refs-errno-cleanup', 2021-11-29) series we made all of its\ncallers explicitly request the errno via an output parameter.\n\nAs that series shows all but one caller ended up passing in a\nboilerplate \"ignore_errno\", since they only cared about whether the\nreturn value was NULL or not, i.e. if the ref could be resolved.\n\nThere was one caller left in sequencer.c that used the\n\"failure_errno', but as of the preceding commit it uses a boilerplate\n\"ignore_errno\" instead.\n\nSo let's remove this output parameter. Not only isn't it used now, but\nit's unlikely that we'll want it again in the future. We'd like to\nslowly move the refs API to a more file-backend independent way of\ncommunicating error codes, having it use a \"failure_errno\" was only\nthe first step in that direction. If this or any other function needs\nto communicate what specifically is wrong with the requested \"refname\"\nit'll be better to have the function set some output enum of\nwell-defined error states than piggy-backend on \"errno\".\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n refs.c | 51 +++++++++++++--------------------------\n refs.h | 7 +-----\n refs/files-backend.c | 31 +++++++-----------------\n remote.c | 3 +--\n sequencer.c | 4 +--\n t/helper/test-ref-store.c | 3 +--\n worktree.c | 11 +++------\n 7 files changed, 33 insertions(+), 77 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex addb26293b4..7017ae59804 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -269,10 +269,9 @@ char *refs_resolve_refdup(struct ref_store *refs,\n \t\t\t struct object_id *oid, int *flags)\n {\n \tconst char *result;\n-\tint ignore_errno;\n \n \tresult = refs_resolve_ref_unsafe(refs, refname, resolve_flags,\n-\t\t\t\t\t oid, flags, &ignore_errno);\n+\t\t\t\t\t oid, flags);\n \treturn xstrdup_or_null(result);\n }\n \n@@ -294,11 +293,10 @@ struct ref_filter {\n \n int read_ref_full(const char *refname, int resolve_flags, struct object_id *oid, int *flags)\n {\n-\tint ignore_errno;\n \tstruct ref_store *refs = get_main_ref_store(the_repository);\n \n \tif (refs_resolve_ref_unsafe(refs, refname, resolve_flags,\n-\t\t\t\t oid, flags, &ignore_errno))\n+\t\t\t\t oid, flags))\n \t\treturn 0;\n \treturn -1;\n }\n@@ -310,9 +308,8 @@ int read_ref(const char *refname, struct object_id *oid)\n \n int refs_ref_exists(struct ref_store *refs, const char *refname)\n {\n-\tint ignore_errno;\n \treturn !!refs_resolve_ref_unsafe(refs, refname, RESOLVE_REF_READING,\n-\t\t\t\t\t NULL, NULL, &ignore_errno);\n+\t\t\t\t\t NULL, NULL);\n }\n \n int ref_exists(const char *refname)\n@@ -656,15 +653,13 @@ int expand_ref(struct repository *repo, const char *str, int len,\n \t\tstruct object_id *this_result;\n \t\tint flag;\n \t\tstruct ref_store *refs = get_main_ref_store(repo);\n-\t\tint ignore_errno;\n \n \t\tthis_result = refs_found ? &oid_from_ref : oid;\n \t\tstrbuf_reset(&fullref);\n \t\tstrbuf_addf(&fullref, *p, len, str);\n \t\tr = refs_resolve_ref_unsafe(refs, fullref.buf,\n \t\t\t\t\t RESOLVE_REF_READING,\n-\t\t\t\t\t this_result, &flag,\n-\t\t\t\t\t &ignore_errno);\n+\t\t\t\t\t this_result, &flag);\n \t\tif (r) {\n \t\t\tif (!refs_found++)\n \t\t\t\t*ref = xstrdup(r);\n@@ -693,14 +688,12 @@ int repo_dwim_log(struct repository *r, const char *str, int len,\n \tfor (p = ref_rev_parse_rules; *p; p++) {\n \t\tstruct object_id hash;\n \t\tconst char *ref, *it;\n-\t\tint ignore_errno;\n \n \t\tstrbuf_reset(&path);\n \t\tstrbuf_addf(&path, *p, len, str);\n \t\tref = refs_resolve_ref_unsafe(refs, path.buf,\n \t\t\t\t\t RESOLVE_REF_READING,\n-\t\t\t\t\t oid ? &hash : NULL, NULL,\n-\t\t\t\t\t &ignore_errno);\n+\t\t\t\t\t oid ? &hash : NULL, NULL);\n \t\tif (!ref)\n \t\t\tcontinue;\n \t\tif (refs_reflog_exists(refs, path.buf))\n@@ -1390,10 +1383,9 @@ int refs_head_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)\n {\n \tstruct object_id oid;\n \tint flag;\n-\tint ignore_errno;\n \n \tif (refs_resolve_ref_unsafe(refs, \"HEAD\", RESOLVE_REF_READING,\n-\t\t\t\t &oid, &flag, &ignore_errno))\n+\t\t\t\t &oid, &flag))\n \t\treturn fn(\"HEAD\", &oid, flag, cb_data);\n \n \treturn 0;\n@@ -1682,15 +1674,13 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \t\t\t\t const char *refname,\n \t\t\t\t int resolve_flags,\n \t\t\t\t struct object_id *oid,\n-\t\t\t\t int *flags, int *failure_errno)\n+\t\t\t\t int *flags)\n {\n \tstatic struct strbuf sb_refname = STRBUF_INIT;\n \tstruct object_id unused_oid;\n \tint unused_flags;\n \tint symref_count;\n \n-\tassert(failure_errno);\n-\n \tif (!oid)\n \t\toid = &unused_oid;\n \tif (!flags)\n@@ -1700,10 +1690,8 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \n \tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {\n \t\tif (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||\n-\t\t !refname_is_safe(refname)) {\n-\t\t\t*failure_errno = EINVAL;\n+\t\t !refname_is_safe(refname))\n \t\t\treturn NULL;\n-\t\t}\n \n \t\t/*\n \t\t * dwim_ref() uses REF_ISBROKEN to distinguish between\n@@ -1718,9 +1706,10 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \n \tfor (symref_count = 0; symref_count < SYMREF_MAXDEPTH; symref_count++) {\n \t\tunsigned int read_flags = 0;\n+\t\tint failure_errno;\n \n \t\tif (refs_read_raw_ref(refs, refname, oid, &sb_refname,\n-\t\t\t\t &read_flags, failure_errno)) {\n+\t\t\t\t &read_flags, &failure_errno)) {\n \t\t\t*flags |= read_flags;\n \n \t\t\t/* In reading mode, refs must eventually resolve */\n@@ -1732,9 +1721,9 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \t\t\t * may show errors besides ENOENT if there are\n \t\t\t * similarly-named refs.\n \t\t\t */\n-\t\t\tif (*failure_errno != ENOENT &&\n-\t\t\t *failure_errno != EISDIR &&\n-\t\t\t *failure_errno != ENOTDIR)\n+\t\t\tif (failure_errno != ENOENT &&\n+\t\t\t failure_errno != EISDIR &&\n+\t\t\t failure_errno != ENOTDIR)\n \t\t\t\treturn NULL;\n \n \t\t\toidclr(oid);\n@@ -1760,16 +1749,13 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \t\t}\n \t\tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {\n \t\t\tif (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||\n-\t\t\t !refname_is_safe(refname)) {\n-\t\t\t\t*failure_errno = EINVAL;\n+\t\t\t !refname_is_safe(refname))\n \t\t\t\treturn NULL;\n-\t\t\t}\n \n \t\t\t*flags |= REF_ISBROKEN | REF_BAD_NAME;\n \t\t}\n \t}\n \n-\t*failure_errno = ELOOP;\n \treturn NULL;\n }\n \n@@ -1784,10 +1770,8 @@ int refs_init_db(struct strbuf *err)\n const char *resolve_ref_unsafe(const char *refname, int resolve_flags,\n \t\t\t struct object_id *oid, int *flags)\n {\n-\tint ignore_errno;\n-\n \treturn refs_resolve_ref_unsafe(get_main_ref_store(the_repository), refname,\n-\t\t\t\t resolve_flags, oid, flags, &ignore_errno);\n+\t\t\t\t resolve_flags, oid, flags);\n }\n \n int resolve_gitlink_ref(const char *submodule, const char *refname,\n@@ -1795,15 +1779,14 @@ int resolve_gitlink_ref(const char *submodule, const char *refname,\n {\n \tstruct ref_store *refs;\n \tint flags;\n-\tint ignore_errno;\n \n \trefs = get_submodule_ref_store(submodule);\n \n \tif (!refs)\n \t\treturn -1;\n \n-\tif (!refs_resolve_ref_unsafe(refs, refname, 0, oid, &flags,\n-\t\t\t\t &ignore_errno) || is_null_oid(oid))\n+\tif (!refs_resolve_ref_unsafe(refs, refname, 0, oid, &flags) ||\n+\t is_null_oid(oid))\n \t\treturn -1;\n \treturn 0;\n }\ndiff --git a/refs.h b/refs.h\nindex 8f91a7f9ff2..cd2d0c1ac09 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -58,11 +58,6 @@ struct worktree;\n * resolved. The function returns NULL for such ref names.\n * Caps and underscores refers to the special refs, such as HEAD,\n * FETCH_HEAD and friends, that all live outside of the refs/ directory.\n- *\n- * Callers should not inspect \"errno\" on failure, but rather pass in a\n- * \"failure_errno\" parameter, on failure the \"errno\" will indicate the\n- * type of failure encountered, but not necessarily one that came from\n- * a syscall. We might have faked it up.\n */\n #define RESOLVE_REF_READING 0x01\n #define RESOLVE_REF_NO_RECURSE 0x02\n@@ -72,7 +67,7 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \t\t\t\t const char *refname,\n \t\t\t\t int resolve_flags,\n \t\t\t\t struct object_id *oid,\n-\t\t\t\t int *flags, int *failure_errno);\n+\t\t\t\t int *flags);\n \n const char *resolve_ref_unsafe(const char *refname, int resolve_flags,\n \t\t\t struct object_id *oid, int *flags);\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 43a3b882d7c..a40267b3ae9 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -277,11 +277,10 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,\n \t\t\t\t\t create_dir_entry(dir->cache, refname.buf,\n \t\t\t\t\t\t\t refname.len));\n \t\t} else {\n-\t\t\tint ignore_errno;\n \t\t\tif (!refs_resolve_ref_unsafe(&refs->base,\n \t\t\t\t\t\t refname.buf,\n \t\t\t\t\t\t RESOLVE_REF_READING,\n-\t\t\t\t\t\t &oid, &flag, &ignore_errno)) {\n+\t\t\t\t\t\t &oid, &flag)) {\n \t\t\t\toidclr(&oid);\n \t\t\t\tflag |= REF_ISBROKEN;\n \t\t\t} else if (is_null_oid(&oid)) {\n@@ -1006,7 +1005,6 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,\n {\n \tstruct strbuf ref_file = STRBUF_INIT;\n \tstruct ref_lock *lock;\n-\tint ignore_errno;\n \n \tfiles_assert_main_repository(refs, \"lock_ref_oid_basic\");\n \tassert(err);\n@@ -1034,7 +1032,7 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,\n \t}\n \n \tif (!refs_resolve_ref_unsafe(&refs->base, lock->ref_name, 0,\n-\t\t\t\t &lock->old_oid, NULL, &ignore_errno))\n+\t\t\t\t &lock->old_oid, NULL))\n \t\toidclr(&lock->old_oid);\n \tgoto out;\n \n@@ -1399,7 +1397,6 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,\n \tstruct strbuf tmp_renamed_log = STRBUF_INIT;\n \tint log, ret;\n \tstruct strbuf err = STRBUF_INIT;\n-\tint ignore_errno;\n \n \tfiles_reflog_path(refs, &sb_oldref, oldrefname);\n \tfiles_reflog_path(refs, &sb_newref, newrefname);\n@@ -1413,7 +1410,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,\n \n \tif (!refs_resolve_ref_unsafe(&refs->base, oldrefname,\n \t\t\t\t RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n-\t\t\t\t &orig_oid, &flag, &ignore_errno)) {\n+\t\t\t\t &orig_oid, &flag)) {\n \t\tret = error(\"refname %s not found\", oldrefname);\n \t\tgoto out;\n \t}\n@@ -1459,7 +1456,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,\n \t */\n \tif (!copy && refs_resolve_ref_unsafe(&refs->base, newrefname,\n \t\t\t\t\t RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n-\t\t\t\t\t NULL, NULL, &ignore_errno) &&\n+\t\t\t\t\t NULL, NULL) &&\n \t refs_delete_ref(&refs->base, NULL, newrefname,\n \t\t\t NULL, REF_NO_DEREF)) {\n \t\tif (errno == EISDIR) {\n@@ -1828,12 +1825,10 @@ static int commit_ref_update(struct files_ref_store *refs,\n \t\t */\n \t\tint head_flag;\n \t\tconst char *head_ref;\n-\t\tint ignore_errno;\n \n \t\thead_ref = refs_resolve_ref_unsafe(&refs->base, \"HEAD\",\n \t\t\t\t\t\t RESOLVE_REF_READING,\n-\t\t\t\t\t\t NULL, &head_flag,\n-\t\t\t\t\t\t &ignore_errno);\n+\t\t\t\t\t\t NULL, &head_flag);\n \t\tif (head_ref && (head_flag & REF_ISSYMREF) &&\n \t\t !strcmp(head_ref, lock->ref_name)) {\n \t\t\tstruct strbuf log_err = STRBUF_INIT;\n@@ -1877,12 +1872,10 @@ static void update_symref_reflog(struct files_ref_store *refs,\n {\n \tstruct strbuf err = STRBUF_INIT;\n \tstruct object_id new_oid;\n-\tint ignore_errno;\n \n \tif (logmsg &&\n \t refs_resolve_ref_unsafe(&refs->base, target,\n-\t\t\t\t RESOLVE_REF_READING, &new_oid, NULL,\n-\t\t\t\t &ignore_errno) &&\n+\t\t\t\t RESOLVE_REF_READING, &new_oid, NULL) &&\n \t files_log_ref_write(refs, refname, &lock->old_oid,\n \t\t\t\t&new_oid, logmsg, 0, &err)) {\n \t\terror(\"%s\", err.buf);\n@@ -2156,7 +2149,6 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator)\n \t\t(struct files_reflog_iterator *)ref_iterator;\n \tstruct dir_iterator *diter = iter->dir_iterator;\n \tint ok;\n-\tint ignore_errno;\n \n \twhile ((ok = dir_iterator_advance(diter)) == ITER_OK) {\n \t\tint flags;\n@@ -2170,8 +2162,7 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator)\n \n \t\tif (!refs_resolve_ref_unsafe(iter->ref_store,\n \t\t\t\t\t diter->relative_path, 0,\n-\t\t\t\t\t &iter->oid, &flags,\n-\t\t\t\t\t &ignore_errno)) {\n+\t\t\t\t\t &iter->oid, &flags)) {\n \t\t\terror(\"bad ref for %s\", diter->path.buf);\n \t\t\tcontinue;\n \t\t}\n@@ -2515,11 +2506,9 @@ static int lock_ref_for_update(struct files_ref_store *refs,\n \t\t\t * the transaction, so we have to read it here\n \t\t\t * to record and possibly check old_oid:\n \t\t\t */\n-\t\t\tint ignore_errno;\n \t\t\tif (!refs_resolve_ref_unsafe(&refs->base,\n \t\t\t\t\t\t referent.buf, 0,\n-\t\t\t\t\t\t &lock->old_oid, NULL,\n-\t\t\t\t\t\t &ignore_errno)) {\n+\t\t\t\t\t\t &lock->old_oid, NULL)) {\n \t\t\t\tif (update->flags & REF_HAVE_OLD) {\n \t\t\t\t\tstrbuf_addf(err, \"cannot lock ref '%s': \"\n \t\t\t\t\t\t \"error reading reference\",\n@@ -3208,14 +3197,12 @@ static int files_reflog_expire(struct ref_store *ref_store,\n \n \t\tif ((expire_flags & EXPIRE_REFLOGS_UPDATE_REF) &&\n \t\t !is_null_oid(&cb.last_kept_oid)) {\n-\t\t\tint ignore_errno;\n \t\t\tint type;\n \t\t\tconst char *ref;\n \n \t\t\tref = refs_resolve_ref_unsafe(&refs->base, refname,\n \t\t\t\t\t\t RESOLVE_REF_NO_RECURSE,\n-\t\t\t\t\t\t NULL, &type,\n-\t\t\t\t\t\t &ignore_errno);\n+\t\t\t\t\t\t NULL, &type);\n \t\t\tupdate = !!(ref && !(type & REF_ISSYMREF));\n \t\t}\n \ndiff --git a/remote.c b/remote.c\nindex a6d8ec6c1ac..c97c626eaa8 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -508,9 +508,8 @@ static void read_config(struct repository *repo)\n \n \trepo->remote_state->current_branch = NULL;\n \tif (startup_info->have_repository) {\n-\t\tint ignore_errno;\n \t\tconst char *head_ref = refs_resolve_ref_unsafe(\n-\t\t\tget_main_ref_store(repo), \"HEAD\", 0, NULL, &flag, &ignore_errno);\n+\t\t\tget_main_ref_store(repo), \"HEAD\", 0, NULL, &flag);\n \t\tif (head_ref && (flag & REF_ISSYMREF) &&\n \t\t skip_prefix(head_ref, \"refs/heads/\", &head_ref)) {\n \t\t\trepo->remote_state->current_branch = make_branch(\ndiff --git a/sequencer.c b/sequencer.c\nindex 03cdf548d72..d57b51ed555 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -1281,7 +1281,6 @@ void print_commit_summary(struct repository *r,\n \tstruct strbuf author_ident = STRBUF_INIT;\n \tstruct strbuf committer_ident = STRBUF_INIT;\n \tstruct ref_store *refs;\n-\tint ignore_errno;\n \n \tcommit = lookup_commit(r, oid);\n \tif (!commit)\n@@ -1332,8 +1331,7 @@ void print_commit_summary(struct repository *r,\n \tdiff_setup_done(&rev.diffopt);\n \n \trefs = get_main_ref_store(the_repository);\n-\thead = refs_resolve_ref_unsafe(refs, \"HEAD\", 0, NULL, NULL,\n-\t\t\t\t &ignore_errno);\n+\thead = refs_resolve_ref_unsafe(refs, \"HEAD\", 0, NULL, NULL);\n \tif (!head)\n \t\tdie(_(\"unable to resolve HEAD after creating commit\"));\n \tif (!strcmp(head, \"HEAD\"))\ndiff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c\nindex 3e4ddaee705..9646d85fc84 100644\n--- a/t/helper/test-ref-store.c\n+++ b/t/helper/test-ref-store.c\n@@ -180,10 +180,9 @@ static int cmd_resolve_ref(struct ref_store *refs, const char **argv)\n \tint resolve_flags = arg_flags(*argv++, \"resolve-flags\", empty_flags);\n \tint flags;\n \tconst char *ref;\n-\tint ignore_errno;\n \n \tref = refs_resolve_ref_unsafe(refs, refname, resolve_flags,\n-\t\t\t\t &oid, &flags, &ignore_errno);\n+\t\t\t\t &oid, &flags);\n \tprintf(\"%s %s 0x%x\\n\", oid_to_hex(&oid), ref ? ref : \"(null)\", flags);\n \treturn ref ? 0 : 1;\n }\ndiff --git a/worktree.c b/worktree.c\nindex 6f598dcfcdf..e8f6f6ae6f4 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -28,13 +28,11 @@ static void add_head_info(struct worktree *wt)\n {\n \tint flags;\n \tconst char *target;\n-\tint ignore_errno;\n \n \ttarget = refs_resolve_ref_unsafe(get_worktree_ref_store(wt),\n \t\t\t\t\t \"HEAD\",\n \t\t\t\t\t 0,\n-\t\t\t\t\t &wt->head_oid, &flags,\n-\t\t\t\t\t &ignore_errno);\n+\t\t\t\t\t &wt->head_oid, &flags);\n \tif (!target)\n \t\treturn;\n \n@@ -416,7 +414,6 @@ const struct worktree *find_shared_symref(struct worktree **worktrees,\n \t\tconst char *symref_target;\n \t\tstruct ref_store *refs;\n \t\tint flags;\n-\t\tint ignore_errno;\n \n \t\tif (wt->is_bare)\n \t\t\tcontinue;\n@@ -434,8 +431,7 @@ const struct worktree *find_shared_symref(struct worktree **worktrees,\n \n \t\trefs = get_worktree_ref_store(wt);\n \t\tsymref_target = refs_resolve_ref_unsafe(refs, symref, 0,\n-\t\t\t\t\t\t\tNULL, &flags,\n-\t\t\t\t\t\t\t&ignore_errno);\n+\t\t\t\t\t\t\tNULL, &flags);\n \t\tif ((flags & REF_ISSYMREF) &&\n \t\t symref_target && !strcmp(symref_target, target)) {\n \t\t\texisting = wt;\n@@ -563,7 +559,6 @@ int other_head_refs(each_ref_fn fn, void *cb_data)\n \t\tstruct worktree *wt = *p;\n \t\tstruct object_id oid;\n \t\tint flag;\n-\t\tint ignore_errno;\n \n \t\tif (wt->is_current)\n \t\t\tcontinue;\n@@ -573,7 +568,7 @@ int other_head_refs(each_ref_fn fn, void *cb_data)\n \t\tif (refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n \t\t\t\t\t refname.buf,\n \t\t\t\t\t RESOLVE_REF_READING,\n-\t\t\t\t\t &oid, &flag, &ignore_errno))\n+\t\t\t\t\t &oid, &flag))\n \t\t\tret = fn(refname.buf, &oid, flag, cb_data);\n \t\tif (ret)\n \t\t\tbreak;\n-- \n2.35.0.rc0.848.gb9d3879eb1d\n\n"},{"id":"446060","messageId":"xmqqo84gu4sb.fsf@gitster.g","threadId":"57052","inReplyTo":"cover-v3-0.3-00000000000-20220112T123117Z-avarab@gmail.com","subject":"Re: [PATCH v3 0/3] For v2.35.0: refs: ab/refs-errno-cleanup fixup + remove \"failure_errno\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-01-12T19:34:28Z","receivedAt":"2022-01-12T19:35:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> This is a follow-up to the recently landed ab/refs-errno-cleanup\n> topic, I missed a spot and left some meaningless use of \"errno\" in the\n> refs (file) backend.\n\nIs it a fix \"oops the ones we merged to 'master' were buggy and\nneeds these on top to be correct\"?\n\nIf it is merely a follow-up \"I am doing more of the same thing as we\naimed to do in that topic\", I'd rather leave it to the next cycle.\n\nI'll come back to the topic to find it out from the patches\nthemselves, but after I dealt with other patches that are more\nclearly fix-ups first.\n\nThanks.\n"},{"id":"446065","messageId":"xmqqh7a8u3my.fsf@gitster.g","threadId":"57052","inReplyTo":"patch-v3-1.3-a45268ac24b-20220112T123117Z-avarab@gmail.com","subject":"Re: [PATCH v3 1/3] refs API: use \"failure_errno\", not \"errno\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-01-12T19:59:17Z","receivedAt":"2022-01-12T20:00:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> @@ -1722,8 +1722,6 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n> \t\tif (refs_read_raw_ref(refs, refname, oid, &sb_refname,\n> \t\t\t\t &read_flags, failure_errno)) {\n> \t\t\t*flags |= read_flags;\n> -\t\t\tif (errno)\n> -\t\t\t\t*failure_errno = errno;\n\nLooks good. \n\nThe whole point of passing failure_errno down to refs_read_raw_ref()\nis that we capture the reason of the failure there without having to\nrely on errno at this point in the flow. Removal of this assignment\nmakes perfect sense.\n\nBut ...\n\n> diff --git a/refs/files-backend.c b/refs/files-backend.c\n> index b529fdf237e..43a3b882d7c 100644\n> --- a/refs/files-backend.c\n> +++ b/refs/files-backend.c\n> @@ -382,7 +382,6 @@ static int files_read_raw_ref(struct ref_store *ref_store, const char *refname,\n> \tif (lstat(path, &st) < 0) {\n> \t\tint ignore_errno;\n> \t\tmyerr = errno;\n> -\t\terrno = 0;\n> \t\tif (myerr != ENOENT)\n> \t\t\tgoto out;\n> \t\tif (refs_read_raw_ref(refs->packed_ref_store, refname, oid,\n> @@ -399,7 +398,6 @@ static int files_read_raw_ref(struct ref_store *ref_store, const char *refname,\n> \t\tstrbuf_reset(&sb_contents);\n> \t\tif (strbuf_readlink(&sb_contents, path, st.st_size) < 0) {\n> \t\t\tmyerr = errno;\n> -\t\t\terrno = 0;\n> \t\t\tif (myerr == ENOENT || myerr == EINVAL)\n> \t\t\t\t/* inconsistent with lstat; retry */\n> \t\t\t\tgoto stat_ref;\n> @@ -469,6 +467,7 @@ static int files_read_raw_ref(struct ref_store *ref_store, const char *refname,\n> \n> \tstrbuf_release(&sb_path);\n> \tstrbuf_release(&sb_contents);\n> +\terrno = 0;\n> \treturn ret;\n> }\n\n... it is not clear to me if this part makes sense. If everybody\nabove the callstack uses failure_errno as the source of truth,\nclearing errno here in this function should not be necessary and is\nmisleading to readers of the code, no?\n"},{"id":"446066","messageId":"xmqqczkwu3h1.fsf@gitster.g","threadId":"57052","inReplyTo":"patch-v3-2.3-8d8691a5e93-20220112T123117Z-avarab@gmail.com","subject":"Re: [PATCH v3 2/3] sequencer: don't use die_errno() on refs_resolve_ref_unsafe() failure","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-01-12T20:02:50Z","receivedAt":"2022-01-12T20:05:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Change code that was faithfully migrated to the new \"resolve_errno\"\n> API in ed90f04155d (refs API: make resolve_ref_unsafe() not set errno,\n> 2021-10-16) to stop caring about the errno at all.\n>\n> When we fail to resolve \"HEAD\" after the sequencer runs it doesn't\n> really help to say what the \"errno\" value is, since the fake backend\n> errno may or may not reflect anything real about the state of the\n> \".git/HEAD\". With the upcoming reftable backend this fakery will\n> become even more pronounced.\n\nOK. In principle I agree with the reasoning. Perhaps it can be\nbetter done as a preparation for the reftable backend in future\ncycles, not now.\n\n> So let's just die() instead of die_errno() here. This will also help\n> simplify the refs_resolve_ref_unsafe() API. This was the only user of\n> it that wasn't ignoring the \"failure_errno\" output parameter.\n\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> sequencer.c | 10 ++++------\n> 1 file changed, 4 insertions(+), 6 deletions(-)\n>\n> diff --git a/sequencer.c b/sequencer.c\n> index 6abd72160cc..03cdf548d72 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -1281,7 +1281,7 @@ void print_commit_summary(struct repository *r,\n> \tstruct strbuf author_ident = STRBUF_INIT;\n> \tstruct strbuf committer_ident = STRBUF_INIT;\n> \tstruct ref_store *refs;\n> -\tint resolve_errno;\n> +\tint ignore_errno;\n> \n> \tcommit = lookup_commit(r, oid);\n> \tif (!commit)\n> @@ -1333,11 +1333,9 @@ void print_commit_summary(struct repository *r,\n> \n> \trefs = get_main_ref_store(the_repository);\n> \thead = refs_resolve_ref_unsafe(refs, \"HEAD\", 0, NULL, NULL,\n> -\t\t\t\t &resolve_errno);\n> -\tif (!head) {\n> -\t\terrno = resolve_errno;\n> -\t\tdie_errno(_(\"unable to resolve HEAD after creating commit\"));\n> -\t}\n> +\t\t\t\t &ignore_errno);\n> +\tif (!head)\n> +\t\tdie(_(\"unable to resolve HEAD after creating commit\"));\n> \tif (!strcmp(head, \"HEAD\"))\n> \t\thead = _(\"detached HEAD\");\n> \telse\n"},{"id":"446120","messageId":"220113.86pmovvnis.gmgdl@evledraar.gmail.com","threadId":"57052","inReplyTo":"xmqqh7a8u3my.fsf@gitster.g","subject":"Re: [PATCH v3 1/3] refs API: use \"failure_errno\", not \"errno\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-01-13T12:14:59Z","receivedAt":"2022-01-13T12:16:47Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Jan 12 2022, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> @@ -1722,8 +1722,6 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n>> \t\tif (refs_read_raw_ref(refs, refname, oid, &sb_refname,\n>> \t\t\t\t &read_flags, failure_errno)) {\n>> \t\t\t*flags |= read_flags;\n>> -\t\t\tif (errno)\n>> -\t\t\t\t*failure_errno = errno;\n>\n> Looks good. \n>\n> The whole point of passing failure_errno down to refs_read_raw_ref()\n> is that we capture the reason of the failure there without having to\n> rely on errno at this point in the flow. Removal of this assignment\n> makes perfect sense.\n>\n> But ...\n>\n>> diff --git a/refs/files-backend.c b/refs/files-backend.c\n>> index b529fdf237e..43a3b882d7c 100644\n>> --- a/refs/files-backend.c\n>> +++ b/refs/files-backend.c\n>> @@ -382,7 +382,6 @@ static int files_read_raw_ref(struct ref_store *ref_store, const char *refname,\n>> \tif (lstat(path, &st) < 0) {\n>> \t\tint ignore_errno;\n>> \t\tmyerr = errno;\n>> -\t\terrno = 0;\n>> \t\tif (myerr != ENOENT)\n>> \t\t\tgoto out;\n>> \t\tif (refs_read_raw_ref(refs->packed_ref_store, refname, oid,\n>> @@ -399,7 +398,6 @@ static int files_read_raw_ref(struct ref_store *ref_store, const char *refname,\n>> \t\tstrbuf_reset(&sb_contents);\n>> \t\tif (strbuf_readlink(&sb_contents, path, st.st_size) < 0) {\n>> \t\t\tmyerr = errno;\n>> -\t\t\terrno = 0;\n>> \t\t\tif (myerr == ENOENT || myerr == EINVAL)\n>> \t\t\t\t/* inconsistent with lstat; retry */\n>> \t\t\t\tgoto stat_ref;\n>> @@ -469,6 +467,7 @@ static int files_read_raw_ref(struct ref_store *ref_store, const char *refname,\n>> \n>> \tstrbuf_release(&sb_path);\n>> \tstrbuf_release(&sb_contents);\n>> +\terrno = 0;\n>> \treturn ret;\n>> }\n>\n> ... it is not clear to me if this part makes sense. If everybody\n> above the callstack uses failure_errno as the source of truth,\n> clearing errno here in this function should not be necessary and is\n> misleading to readers of the code, no?\n\nIt's consistent with various other existing refs* code as we made this\ntransition, see:\n\n git grep -W 'errno = 0' -- 'refs*'\n\nI.e. we'd like to not only transition existing users away from errno,\nbut to ensure that the file backend errno semantics don't inadvertently\nleak outside the API.\n\nDoing so is a bit pedantic for sure, but ensures that we won't need to\ndeal with any subtle bugs in that are with the reftable backend.\n"},{"id":"446121","messageId":"220113.86lezjvn2d.gmgdl@evledraar.gmail.com","threadId":"57052","inReplyTo":"xmqqo84gu4sb.fsf@gitster.g","subject":"Re: [PATCH v3 0/3] For v2.35.0: refs: ab/refs-errno-cleanup fixup + remove \"failure_errno\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-01-13T12:22:55Z","receivedAt":"2022-01-13T12:26:55Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Jan 12 2022, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> This is a follow-up to the recently landed ab/refs-errno-cleanup\n>> topic, I missed a spot and left some meaningless use of \"errno\" in the\n>> refs (file) backend.\n>\n> Is it a fix \"oops the ones we merged to 'master' were buggy and\n> needs these on top to be correct\"?\n>\n> If it is merely a follow-up \"I am doing more of the same thing as we\n> aimed to do in that topic\", I'd rather leave it to the next cycle.\n>\n> I'll come back to the topic to find it out from the patches\n> themselves, but after I dealt with other patches that are more\n> clearly fix-ups first.\n\n3/3 isn't needed for v2.35.0, but I figured if you were queuing this\nthat obvious cleanup after the also-not-strictly-needed 2/3 made sense.\n\nBut I can leave both of those until after the release if you'd like.\n\nI think we should have 1/3 for 2.35.0 as rationalized in its commit\nmessage. I.e. I haven't found a way for that omission in my ef18119dec8\nto break things in practice, but I'm also not confident that I thought\nthrough all the potential cases. Changing that code to be obviously\ncorrect in terms of the current API is an easy enough fix.\n\nSo just let me know how you'd like to proceed. I.e. I can re-roll a v2\nwith just 1/3, but I think this v1 is also good-to-go as-is. Thanks!\n"},{"id":"446175","messageId":"xmqq8rvjihzq.fsf@gitster.g","threadId":"57052","inReplyTo":"220113.86lezjvn2d.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v3 0/3] For v2.35.0: refs: ab/refs-errno-cleanup fixup + remove \"failure_errno\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-01-13T18:54:33Z","receivedAt":"2022-01-13T18:54:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> So just let me know how you'd like to proceed. I.e. I can re-roll a v2\n> with just 1/3, but I think this v1 is also good-to-go as-is. Thanks!\n\nI can just pick up [v1 1/3] and ignore the rest, which would mean\nyou do not have to send v2, right?\n"},{"id":"446217","messageId":"220114.86sftqse1q.gmgdl@evledraar.gmail.com","threadId":"57052","inReplyTo":"xmqq8rvjihzq.fsf@gitster.g","subject":"Re: [PATCH v3 0/3] For v2.35.0: refs: ab/refs-errno-cleanup fixup + remove \"failure_errno\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-01-14T12:21:44Z","receivedAt":"2022-01-14T12:21:57Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Jan 13 2022, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> So just let me know how you'd like to proceed. I.e. I can re-roll a v2\n>> with just 1/3, but I think this v1 is also good-to-go as-is. Thanks!\n>\n> I can just pick up [v1 1/3] and ignore the rest, which would mean\n> you do not have to send v2, right?\n\nLate reply, but yes (and I see you did that already). Thanks!\n"},{"id":"446962","messageId":"cover-v4-0.2-00000000000-20220126T143427Z-avarab@gmail.com","threadId":"57052","inReplyTo":"xmqqo84gu4sb.fsf@gitster.g","subject":"[PATCH v4 0/2] refs: remove the last use of \"errno\" from the public API","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-01-26T14:36:59Z","receivedAt":"2022-01-26T14:37:11Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Wed, Jan 12 2022, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> This is a follow-up to the recently landed ab/refs-errno-cleanup\n>> topic, I missed a spot and left some meaningless use of \"errno\" in the\n>> refs (file) backend.\n>\n> Is it a fix \"oops the ones we merged to 'master' were buggy and\n> needs these on top to be correct\"?\n>\n> If it is merely a follow-up \"I am doing more of the same thing as we\n> aimed to do in that topic\", I'd rather leave it to the next cycle.\n\nAs discussed before the last release you merged 1/3 of v3 of this down\nas 31e39123695 (Merge branch 'ab/refs-errno-cleanup', 2022-01-14).\n\nNow that we're post-release here's the remaining 2 patches, which as\nthe range-diff shows have no changes since v3, except an updated\ncommit message for 2/2 mentioning that previously merged 31e39123695.\n\nThis has no conflicts currently with \"seen\", but with the ongoing\nreftable integration that probably won't be true for long, so I think\nit would be good to queue this up sooner than later. I'm hoping that\nbetween myself & Han-Wen's main push that we can get \"real\" reftable\nintegration started during this cycle.\n\nÆvar Arnfjörð Bjarmason (2):\n sequencer: don't use die_errno() on refs_resolve_ref_unsafe() failure\n refs API: remove \"failure_errno\" from refs_resolve_ref_unsafe()\n\n refs.c | 51 +++++++++++++--------------------------\n refs.h | 7 +-----\n refs/files-backend.c | 31 +++++++-----------------\n remote.c | 3 +--\n sequencer.c | 10 +++-----\n t/helper/test-ref-store.c | 3 +--\n worktree.c | 11 +++------\n 7 files changed, 35 insertions(+), 81 deletions(-)\n\nRange-diff against v3:\n1: a45268ac24b < -: ----------- refs API: use \"failure_errno\", not \"errno\"\n2: 8d8691a5e93 = 1: 7f31277fd57 sequencer: don't use die_errno() on refs_resolve_ref_unsafe() failure\n3: 8f937d8f64a ! 2: 5e6f63afb40 refs API: remove \"failure_errno\" from refs_resolve_ref_unsafe()\n @@ Commit message\n boilerplate \"ignore_errno\", since they only cared about whether the\n return value was NULL or not, i.e. if the ref could be resolved.\n \n - There was one caller left in sequencer.c that used the\n + There was one small issue with that series fixed with a follow-up in\n + 31e39123695 (Merge branch 'ab/refs-errno-cleanup', 2022-01-14) a small\n + bug in that series was fixed.\n +\n + After those two there was one caller left in sequencer.c that used the\n \"failure_errno', but as of the preceding commit it uses a boilerplate\n \"ignore_errno\" instead.\n \n + This leaves the public refs API without any use of \"failure_errno\" at\n + all. We could still do with a bit of cleanup and generalization\n + between refs.c and refs/files-backend.c before the \"reftable\"\n + integration lands, but that's all internal to the reference code\n + itself.\n +\n So let's remove this output parameter. Not only isn't it used now, but\n it's unlikely that we'll want it again in the future. We'd like to\n slowly move the refs API to a more file-backend independent way of\n-- \n2.35.0.890.g96f29f9df61\n\n"},{"id":"446961","messageId":"patch-v4-1.2-7f31277fd57-20220126T143427Z-avarab@gmail.com","threadId":"57052","inReplyTo":"cover-v4-0.2-00000000000-20220126T143427Z-avarab@gmail.com","subject":"[PATCH v4 1/2] sequencer: don't use die_errno() on refs_resolve_ref_unsafe() failure","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-01-26T14:37:00Z","receivedAt":"2022-01-26T14:37:12Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change code that was faithfully migrated to the new \"resolve_errno\"\nAPI in ed90f04155d (refs API: make resolve_ref_unsafe() not set errno,\n2021-10-16) to stop caring about the errno at all.\n\nWhen we fail to resolve \"HEAD\" after the sequencer runs it doesn't\nreally help to say what the \"errno\" value is, since the fake backend\nerrno may or may not reflect anything real about the state of the\n\".git/HEAD\". With the upcoming reftable backend this fakery will\nbecome even more pronounced.\n\nSo let's just die() instead of die_errno() here. This will also help\nsimplify the refs_resolve_ref_unsafe() API. This was the only user of\nit that wasn't ignoring the \"failure_errno\" output parameter.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n sequencer.c | 10 ++++------\n 1 file changed, 4 insertions(+), 6 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 6abd72160cc..03cdf548d72 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -1281,7 +1281,7 @@ void print_commit_summary(struct repository *r,\n \tstruct strbuf author_ident = STRBUF_INIT;\n \tstruct strbuf committer_ident = STRBUF_INIT;\n \tstruct ref_store *refs;\n-\tint resolve_errno;\n+\tint ignore_errno;\n \n \tcommit = lookup_commit(r, oid);\n \tif (!commit)\n@@ -1333,11 +1333,9 @@ void print_commit_summary(struct repository *r,\n \n \trefs = get_main_ref_store(the_repository);\n \thead = refs_resolve_ref_unsafe(refs, \"HEAD\", 0, NULL, NULL,\n-\t\t\t\t &resolve_errno);\n-\tif (!head) {\n-\t\terrno = resolve_errno;\n-\t\tdie_errno(_(\"unable to resolve HEAD after creating commit\"));\n-\t}\n+\t\t\t\t &ignore_errno);\n+\tif (!head)\n+\t\tdie(_(\"unable to resolve HEAD after creating commit\"));\n \tif (!strcmp(head, \"HEAD\"))\n \t\thead = _(\"detached HEAD\");\n \telse\n-- \n2.35.0.890.g96f29f9df61\n\n"},{"id":"446963","messageId":"patch-v4-2.2-5e6f63afb40-20220126T143427Z-avarab@gmail.com","threadId":"57052","inReplyTo":"cover-v4-0.2-00000000000-20220126T143427Z-avarab@gmail.com","subject":"[PATCH v4 2/2] refs API: remove \"failure_errno\" from refs_resolve_ref_unsafe()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-01-26T14:37:01Z","receivedAt":"2022-01-26T14:37:14Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Remove the now-unused \"failure_errno\" parameter from the\nrefs_resolve_ref_unsafe() signature. In my recent 96f6623ada0 (Merge\nbranch 'ab/refs-errno-cleanup', 2021-11-29) series we made all of its\ncallers explicitly request the errno via an output parameter.\n\nAs that series shows all but one caller ended up passing in a\nboilerplate \"ignore_errno\", since they only cared about whether the\nreturn value was NULL or not, i.e. if the ref could be resolved.\n\nThere was one small issue with that series fixed with a follow-up in\n31e39123695 (Merge branch 'ab/refs-errno-cleanup', 2022-01-14) a small\nbug in that series was fixed.\n\nAfter those two there was one caller left in sequencer.c that used the\n\"failure_errno', but as of the preceding commit it uses a boilerplate\n\"ignore_errno\" instead.\n\nThis leaves the public refs API without any use of \"failure_errno\" at\nall. We could still do with a bit of cleanup and generalization\nbetween refs.c and refs/files-backend.c before the \"reftable\"\nintegration lands, but that's all internal to the reference code\nitself.\n\nSo let's remove this output parameter. Not only isn't it used now, but\nit's unlikely that we'll want it again in the future. We'd like to\nslowly move the refs API to a more file-backend independent way of\ncommunicating error codes, having it use a \"failure_errno\" was only\nthe first step in that direction. If this or any other function needs\nto communicate what specifically is wrong with the requested \"refname\"\nit'll be better to have the function set some output enum of\nwell-defined error states than piggy-backend on \"errno\".\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n refs.c | 51 +++++++++++++--------------------------\n refs.h | 7 +-----\n refs/files-backend.c | 31 +++++++-----------------\n remote.c | 3 +--\n sequencer.c | 4 +--\n t/helper/test-ref-store.c | 3 +--\n worktree.c | 11 +++------\n 7 files changed, 33 insertions(+), 77 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex addb26293b4..7017ae59804 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -269,10 +269,9 @@ char *refs_resolve_refdup(struct ref_store *refs,\n \t\t\t struct object_id *oid, int *flags)\n {\n \tconst char *result;\n-\tint ignore_errno;\n \n \tresult = refs_resolve_ref_unsafe(refs, refname, resolve_flags,\n-\t\t\t\t\t oid, flags, &ignore_errno);\n+\t\t\t\t\t oid, flags);\n \treturn xstrdup_or_null(result);\n }\n \n@@ -294,11 +293,10 @@ struct ref_filter {\n \n int read_ref_full(const char *refname, int resolve_flags, struct object_id *oid, int *flags)\n {\n-\tint ignore_errno;\n \tstruct ref_store *refs = get_main_ref_store(the_repository);\n \n \tif (refs_resolve_ref_unsafe(refs, refname, resolve_flags,\n-\t\t\t\t oid, flags, &ignore_errno))\n+\t\t\t\t oid, flags))\n \t\treturn 0;\n \treturn -1;\n }\n@@ -310,9 +308,8 @@ int read_ref(const char *refname, struct object_id *oid)\n \n int refs_ref_exists(struct ref_store *refs, const char *refname)\n {\n-\tint ignore_errno;\n \treturn !!refs_resolve_ref_unsafe(refs, refname, RESOLVE_REF_READING,\n-\t\t\t\t\t NULL, NULL, &ignore_errno);\n+\t\t\t\t\t NULL, NULL);\n }\n \n int ref_exists(const char *refname)\n@@ -656,15 +653,13 @@ int expand_ref(struct repository *repo, const char *str, int len,\n \t\tstruct object_id *this_result;\n \t\tint flag;\n \t\tstruct ref_store *refs = get_main_ref_store(repo);\n-\t\tint ignore_errno;\n \n \t\tthis_result = refs_found ? &oid_from_ref : oid;\n \t\tstrbuf_reset(&fullref);\n \t\tstrbuf_addf(&fullref, *p, len, str);\n \t\tr = refs_resolve_ref_unsafe(refs, fullref.buf,\n \t\t\t\t\t RESOLVE_REF_READING,\n-\t\t\t\t\t this_result, &flag,\n-\t\t\t\t\t &ignore_errno);\n+\t\t\t\t\t this_result, &flag);\n \t\tif (r) {\n \t\t\tif (!refs_found++)\n \t\t\t\t*ref = xstrdup(r);\n@@ -693,14 +688,12 @@ int repo_dwim_log(struct repository *r, const char *str, int len,\n \tfor (p = ref_rev_parse_rules; *p; p++) {\n \t\tstruct object_id hash;\n \t\tconst char *ref, *it;\n-\t\tint ignore_errno;\n \n \t\tstrbuf_reset(&path);\n \t\tstrbuf_addf(&path, *p, len, str);\n \t\tref = refs_resolve_ref_unsafe(refs, path.buf,\n \t\t\t\t\t RESOLVE_REF_READING,\n-\t\t\t\t\t oid ? &hash : NULL, NULL,\n-\t\t\t\t\t &ignore_errno);\n+\t\t\t\t\t oid ? &hash : NULL, NULL);\n \t\tif (!ref)\n \t\t\tcontinue;\n \t\tif (refs_reflog_exists(refs, path.buf))\n@@ -1390,10 +1383,9 @@ int refs_head_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)\n {\n \tstruct object_id oid;\n \tint flag;\n-\tint ignore_errno;\n \n \tif (refs_resolve_ref_unsafe(refs, \"HEAD\", RESOLVE_REF_READING,\n-\t\t\t\t &oid, &flag, &ignore_errno))\n+\t\t\t\t &oid, &flag))\n \t\treturn fn(\"HEAD\", &oid, flag, cb_data);\n \n \treturn 0;\n@@ -1682,15 +1674,13 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \t\t\t\t const char *refname,\n \t\t\t\t int resolve_flags,\n \t\t\t\t struct object_id *oid,\n-\t\t\t\t int *flags, int *failure_errno)\n+\t\t\t\t int *flags)\n {\n \tstatic struct strbuf sb_refname = STRBUF_INIT;\n \tstruct object_id unused_oid;\n \tint unused_flags;\n \tint symref_count;\n \n-\tassert(failure_errno);\n-\n \tif (!oid)\n \t\toid = &unused_oid;\n \tif (!flags)\n@@ -1700,10 +1690,8 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \n \tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {\n \t\tif (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||\n-\t\t !refname_is_safe(refname)) {\n-\t\t\t*failure_errno = EINVAL;\n+\t\t !refname_is_safe(refname))\n \t\t\treturn NULL;\n-\t\t}\n \n \t\t/*\n \t\t * dwim_ref() uses REF_ISBROKEN to distinguish between\n@@ -1718,9 +1706,10 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \n \tfor (symref_count = 0; symref_count < SYMREF_MAXDEPTH; symref_count++) {\n \t\tunsigned int read_flags = 0;\n+\t\tint failure_errno;\n \n \t\tif (refs_read_raw_ref(refs, refname, oid, &sb_refname,\n-\t\t\t\t &read_flags, failure_errno)) {\n+\t\t\t\t &read_flags, &failure_errno)) {\n \t\t\t*flags |= read_flags;\n \n \t\t\t/* In reading mode, refs must eventually resolve */\n@@ -1732,9 +1721,9 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \t\t\t * may show errors besides ENOENT if there are\n \t\t\t * similarly-named refs.\n \t\t\t */\n-\t\t\tif (*failure_errno != ENOENT &&\n-\t\t\t *failure_errno != EISDIR &&\n-\t\t\t *failure_errno != ENOTDIR)\n+\t\t\tif (failure_errno != ENOENT &&\n+\t\t\t failure_errno != EISDIR &&\n+\t\t\t failure_errno != ENOTDIR)\n \t\t\t\treturn NULL;\n \n \t\t\toidclr(oid);\n@@ -1760,16 +1749,13 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \t\t}\n \t\tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {\n \t\t\tif (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||\n-\t\t\t !refname_is_safe(refname)) {\n-\t\t\t\t*failure_errno = EINVAL;\n+\t\t\t !refname_is_safe(refname))\n \t\t\t\treturn NULL;\n-\t\t\t}\n \n \t\t\t*flags |= REF_ISBROKEN | REF_BAD_NAME;\n \t\t}\n \t}\n \n-\t*failure_errno = ELOOP;\n \treturn NULL;\n }\n \n@@ -1784,10 +1770,8 @@ int refs_init_db(struct strbuf *err)\n const char *resolve_ref_unsafe(const char *refname, int resolve_flags,\n \t\t\t struct object_id *oid, int *flags)\n {\n-\tint ignore_errno;\n-\n \treturn refs_resolve_ref_unsafe(get_main_ref_store(the_repository), refname,\n-\t\t\t\t resolve_flags, oid, flags, &ignore_errno);\n+\t\t\t\t resolve_flags, oid, flags);\n }\n \n int resolve_gitlink_ref(const char *submodule, const char *refname,\n@@ -1795,15 +1779,14 @@ int resolve_gitlink_ref(const char *submodule, const char *refname,\n {\n \tstruct ref_store *refs;\n \tint flags;\n-\tint ignore_errno;\n \n \trefs = get_submodule_ref_store(submodule);\n \n \tif (!refs)\n \t\treturn -1;\n \n-\tif (!refs_resolve_ref_unsafe(refs, refname, 0, oid, &flags,\n-\t\t\t\t &ignore_errno) || is_null_oid(oid))\n+\tif (!refs_resolve_ref_unsafe(refs, refname, 0, oid, &flags) ||\n+\t is_null_oid(oid))\n \t\treturn -1;\n \treturn 0;\n }\ndiff --git a/refs.h b/refs.h\nindex 8f91a7f9ff2..cd2d0c1ac09 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -58,11 +58,6 @@ struct worktree;\n * resolved. The function returns NULL for such ref names.\n * Caps and underscores refers to the special refs, such as HEAD,\n * FETCH_HEAD and friends, that all live outside of the refs/ directory.\n- *\n- * Callers should not inspect \"errno\" on failure, but rather pass in a\n- * \"failure_errno\" parameter, on failure the \"errno\" will indicate the\n- * type of failure encountered, but not necessarily one that came from\n- * a syscall. We might have faked it up.\n */\n #define RESOLVE_REF_READING 0x01\n #define RESOLVE_REF_NO_RECURSE 0x02\n@@ -72,7 +67,7 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \t\t\t\t const char *refname,\n \t\t\t\t int resolve_flags,\n \t\t\t\t struct object_id *oid,\n-\t\t\t\t int *flags, int *failure_errno);\n+\t\t\t\t int *flags);\n \n const char *resolve_ref_unsafe(const char *refname, int resolve_flags,\n \t\t\t struct object_id *oid, int *flags);\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 43a3b882d7c..a40267b3ae9 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -277,11 +277,10 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,\n \t\t\t\t\t create_dir_entry(dir->cache, refname.buf,\n \t\t\t\t\t\t\t refname.len));\n \t\t} else {\n-\t\t\tint ignore_errno;\n \t\t\tif (!refs_resolve_ref_unsafe(&refs->base,\n \t\t\t\t\t\t refname.buf,\n \t\t\t\t\t\t RESOLVE_REF_READING,\n-\t\t\t\t\t\t &oid, &flag, &ignore_errno)) {\n+\t\t\t\t\t\t &oid, &flag)) {\n \t\t\t\toidclr(&oid);\n \t\t\t\tflag |= REF_ISBROKEN;\n \t\t\t} else if (is_null_oid(&oid)) {\n@@ -1006,7 +1005,6 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,\n {\n \tstruct strbuf ref_file = STRBUF_INIT;\n \tstruct ref_lock *lock;\n-\tint ignore_errno;\n \n \tfiles_assert_main_repository(refs, \"lock_ref_oid_basic\");\n \tassert(err);\n@@ -1034,7 +1032,7 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,\n \t}\n \n \tif (!refs_resolve_ref_unsafe(&refs->base, lock->ref_name, 0,\n-\t\t\t\t &lock->old_oid, NULL, &ignore_errno))\n+\t\t\t\t &lock->old_oid, NULL))\n \t\toidclr(&lock->old_oid);\n \tgoto out;\n \n@@ -1399,7 +1397,6 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,\n \tstruct strbuf tmp_renamed_log = STRBUF_INIT;\n \tint log, ret;\n \tstruct strbuf err = STRBUF_INIT;\n-\tint ignore_errno;\n \n \tfiles_reflog_path(refs, &sb_oldref, oldrefname);\n \tfiles_reflog_path(refs, &sb_newref, newrefname);\n@@ -1413,7 +1410,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,\n \n \tif (!refs_resolve_ref_unsafe(&refs->base, oldrefname,\n \t\t\t\t RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n-\t\t\t\t &orig_oid, &flag, &ignore_errno)) {\n+\t\t\t\t &orig_oid, &flag)) {\n \t\tret = error(\"refname %s not found\", oldrefname);\n \t\tgoto out;\n \t}\n@@ -1459,7 +1456,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,\n \t */\n \tif (!copy && refs_resolve_ref_unsafe(&refs->base, newrefname,\n \t\t\t\t\t RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n-\t\t\t\t\t NULL, NULL, &ignore_errno) &&\n+\t\t\t\t\t NULL, NULL) &&\n \t refs_delete_ref(&refs->base, NULL, newrefname,\n \t\t\t NULL, REF_NO_DEREF)) {\n \t\tif (errno == EISDIR) {\n@@ -1828,12 +1825,10 @@ static int commit_ref_update(struct files_ref_store *refs,\n \t\t */\n \t\tint head_flag;\n \t\tconst char *head_ref;\n-\t\tint ignore_errno;\n \n \t\thead_ref = refs_resolve_ref_unsafe(&refs->base, \"HEAD\",\n \t\t\t\t\t\t RESOLVE_REF_READING,\n-\t\t\t\t\t\t NULL, &head_flag,\n-\t\t\t\t\t\t &ignore_errno);\n+\t\t\t\t\t\t NULL, &head_flag);\n \t\tif (head_ref && (head_flag & REF_ISSYMREF) &&\n \t\t !strcmp(head_ref, lock->ref_name)) {\n \t\t\tstruct strbuf log_err = STRBUF_INIT;\n@@ -1877,12 +1872,10 @@ static void update_symref_reflog(struct files_ref_store *refs,\n {\n \tstruct strbuf err = STRBUF_INIT;\n \tstruct object_id new_oid;\n-\tint ignore_errno;\n \n \tif (logmsg &&\n \t refs_resolve_ref_unsafe(&refs->base, target,\n-\t\t\t\t RESOLVE_REF_READING, &new_oid, NULL,\n-\t\t\t\t &ignore_errno) &&\n+\t\t\t\t RESOLVE_REF_READING, &new_oid, NULL) &&\n \t files_log_ref_write(refs, refname, &lock->old_oid,\n \t\t\t\t&new_oid, logmsg, 0, &err)) {\n \t\terror(\"%s\", err.buf);\n@@ -2156,7 +2149,6 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator)\n \t\t(struct files_reflog_iterator *)ref_iterator;\n \tstruct dir_iterator *diter = iter->dir_iterator;\n \tint ok;\n-\tint ignore_errno;\n \n \twhile ((ok = dir_iterator_advance(diter)) == ITER_OK) {\n \t\tint flags;\n@@ -2170,8 +2162,7 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator)\n \n \t\tif (!refs_resolve_ref_unsafe(iter->ref_store,\n \t\t\t\t\t diter->relative_path, 0,\n-\t\t\t\t\t &iter->oid, &flags,\n-\t\t\t\t\t &ignore_errno)) {\n+\t\t\t\t\t &iter->oid, &flags)) {\n \t\t\terror(\"bad ref for %s\", diter->path.buf);\n \t\t\tcontinue;\n \t\t}\n@@ -2515,11 +2506,9 @@ static int lock_ref_for_update(struct files_ref_store *refs,\n \t\t\t * the transaction, so we have to read it here\n \t\t\t * to record and possibly check old_oid:\n \t\t\t */\n-\t\t\tint ignore_errno;\n \t\t\tif (!refs_resolve_ref_unsafe(&refs->base,\n \t\t\t\t\t\t referent.buf, 0,\n-\t\t\t\t\t\t &lock->old_oid, NULL,\n-\t\t\t\t\t\t &ignore_errno)) {\n+\t\t\t\t\t\t &lock->old_oid, NULL)) {\n \t\t\t\tif (update->flags & REF_HAVE_OLD) {\n \t\t\t\t\tstrbuf_addf(err, \"cannot lock ref '%s': \"\n \t\t\t\t\t\t \"error reading reference\",\n@@ -3208,14 +3197,12 @@ static int files_reflog_expire(struct ref_store *ref_store,\n \n \t\tif ((expire_flags & EXPIRE_REFLOGS_UPDATE_REF) &&\n \t\t !is_null_oid(&cb.last_kept_oid)) {\n-\t\t\tint ignore_errno;\n \t\t\tint type;\n \t\t\tconst char *ref;\n \n \t\t\tref = refs_resolve_ref_unsafe(&refs->base, refname,\n \t\t\t\t\t\t RESOLVE_REF_NO_RECURSE,\n-\t\t\t\t\t\t NULL, &type,\n-\t\t\t\t\t\t &ignore_errno);\n+\t\t\t\t\t\t NULL, &type);\n \t\t\tupdate = !!(ref && !(type & REF_ISSYMREF));\n \t\t}\n \ndiff --git a/remote.c b/remote.c\nindex a6d8ec6c1ac..c97c626eaa8 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -508,9 +508,8 @@ static void read_config(struct repository *repo)\n \n \trepo->remote_state->current_branch = NULL;\n \tif (startup_info->have_repository) {\n-\t\tint ignore_errno;\n \t\tconst char *head_ref = refs_resolve_ref_unsafe(\n-\t\t\tget_main_ref_store(repo), \"HEAD\", 0, NULL, &flag, &ignore_errno);\n+\t\t\tget_main_ref_store(repo), \"HEAD\", 0, NULL, &flag);\n \t\tif (head_ref && (flag & REF_ISSYMREF) &&\n \t\t skip_prefix(head_ref, \"refs/heads/\", &head_ref)) {\n \t\t\trepo->remote_state->current_branch = make_branch(\ndiff --git a/sequencer.c b/sequencer.c\nindex 03cdf548d72..d57b51ed555 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -1281,7 +1281,6 @@ void print_commit_summary(struct repository *r,\n \tstruct strbuf author_ident = STRBUF_INIT;\n \tstruct strbuf committer_ident = STRBUF_INIT;\n \tstruct ref_store *refs;\n-\tint ignore_errno;\n \n \tcommit = lookup_commit(r, oid);\n \tif (!commit)\n@@ -1332,8 +1331,7 @@ void print_commit_summary(struct repository *r,\n \tdiff_setup_done(&rev.diffopt);\n \n \trefs = get_main_ref_store(the_repository);\n-\thead = refs_resolve_ref_unsafe(refs, \"HEAD\", 0, NULL, NULL,\n-\t\t\t\t &ignore_errno);\n+\thead = refs_resolve_ref_unsafe(refs, \"HEAD\", 0, NULL, NULL);\n \tif (!head)\n \t\tdie(_(\"unable to resolve HEAD after creating commit\"));\n \tif (!strcmp(head, \"HEAD\"))\ndiff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c\nindex 3e4ddaee705..9646d85fc84 100644\n--- a/t/helper/test-ref-store.c\n+++ b/t/helper/test-ref-store.c\n@@ -180,10 +180,9 @@ static int cmd_resolve_ref(struct ref_store *refs, const char **argv)\n \tint resolve_flags = arg_flags(*argv++, \"resolve-flags\", empty_flags);\n \tint flags;\n \tconst char *ref;\n-\tint ignore_errno;\n \n \tref = refs_resolve_ref_unsafe(refs, refname, resolve_flags,\n-\t\t\t\t &oid, &flags, &ignore_errno);\n+\t\t\t\t &oid, &flags);\n \tprintf(\"%s %s 0x%x\\n\", oid_to_hex(&oid), ref ? ref : \"(null)\", flags);\n \treturn ref ? 0 : 1;\n }\ndiff --git a/worktree.c b/worktree.c\nindex 6f598dcfcdf..e8f6f6ae6f4 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -28,13 +28,11 @@ static void add_head_info(struct worktree *wt)\n {\n \tint flags;\n \tconst char *target;\n-\tint ignore_errno;\n \n \ttarget = refs_resolve_ref_unsafe(get_worktree_ref_store(wt),\n \t\t\t\t\t \"HEAD\",\n \t\t\t\t\t 0,\n-\t\t\t\t\t &wt->head_oid, &flags,\n-\t\t\t\t\t &ignore_errno);\n+\t\t\t\t\t &wt->head_oid, &flags);\n \tif (!target)\n \t\treturn;\n \n@@ -416,7 +414,6 @@ const struct worktree *find_shared_symref(struct worktree **worktrees,\n \t\tconst char *symref_target;\n \t\tstruct ref_store *refs;\n \t\tint flags;\n-\t\tint ignore_errno;\n \n \t\tif (wt->is_bare)\n \t\t\tcontinue;\n@@ -434,8 +431,7 @@ const struct worktree *find_shared_symref(struct worktree **worktrees,\n \n \t\trefs = get_worktree_ref_store(wt);\n \t\tsymref_target = refs_resolve_ref_unsafe(refs, symref, 0,\n-\t\t\t\t\t\t\tNULL, &flags,\n-\t\t\t\t\t\t\t&ignore_errno);\n+\t\t\t\t\t\t\tNULL, &flags);\n \t\tif ((flags & REF_ISSYMREF) &&\n \t\t symref_target && !strcmp(symref_target, target)) {\n \t\t\texisting = wt;\n@@ -563,7 +559,6 @@ int other_head_refs(each_ref_fn fn, void *cb_data)\n \t\tstruct worktree *wt = *p;\n \t\tstruct object_id oid;\n \t\tint flag;\n-\t\tint ignore_errno;\n \n \t\tif (wt->is_current)\n \t\t\tcontinue;\n@@ -573,7 +568,7 @@ int other_head_refs(each_ref_fn fn, void *cb_data)\n \t\tif (refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n \t\t\t\t\t refname.buf,\n \t\t\t\t\t RESOLVE_REF_READING,\n-\t\t\t\t\t &oid, &flag, &ignore_errno))\n+\t\t\t\t\t &oid, &flag))\n \t\t\tret = fn(refname.buf, &oid, flag, cb_data);\n \t\tif (ret)\n \t\t\tbreak;\n-- \n2.35.0.890.g96f29f9df61\n\n"},{"id":"447020","messageId":"xmqqpmoenj85.fsf@gitster.g","threadId":"57052","inReplyTo":"patch-v4-1.2-7f31277fd57-20220126T143427Z-avarab@gmail.com","subject":"Re: [PATCH v4 1/2] sequencer: don't use die_errno() on refs_resolve_ref_unsafe() failure","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-01-26T23:57:30Z","receivedAt":"2022-01-26T23:57:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Change code that was faithfully migrated to the new \"resolve_errno\"\n> API in ed90f04155d (refs API: make resolve_ref_unsafe() not set errno,\n> 2021-10-16) to stop caring about the errno at all.\n>\n> When we fail to resolve \"HEAD\" after the sequencer runs it doesn't\n> really help to say what the \"errno\" value is, since the fake backend\n> errno may or may not reflect anything real about the state of the\n> \".git/HEAD\".\n\nOK. Assuming that the eventual goal is to drop the return value\nparameter from the resolve_ref_unsafe() function (because according\nto this caller, it is not yielding any meaningful value), this\nchange makes perfect sense.\n\n> With the upcoming reftable backend this fakery will\n> become even more pronounced.\n\nThis too.\n\nNicely explained and executed.\n\nThanks.\n\n\n"}]}