From: "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Han-Wen Nienhuys <hanwen@google.com>,
Han-Wen Nienhuys <hanwenn@gmail.com>,
Han-Wen Nienhuys <hanwen@google.com>
Subject: [PATCH v2 3/3] refs: make errno output explicit for read_raw_ref_fn
Date: Fri, 23 Apr 2021 15:31:47 +0000 [thread overview]
Message-ID: <7fbc1c754f435c6d621734685ebee1bdc539c000.1619191907.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1011.v2.git.git.1619191907.gitgitgadget@gmail.com>
From: Han-Wen Nienhuys <hanwen@google.com>
read_raw_ref_fn needs to supply a credible errno for a number of cases. These
are primarily:
1) The files backend calls read_raw_ref from lock_raw_ref, and uses the
resulting error codes to create/remove directories as needed.
2) ENOENT should be translated in a zero OID, optionally with REF_ISBROKEN set,
returning the last successfully resolved symref. This is necessary so
read_raw_ref("HEAD") on an empty repo returns refs/heads/main (or the default branch
du-jour), and we know on which branch to create the first commit.
Make this information flow explicit by adding a failure_errno to the signature
of read_raw_ref. All errnos from the files backend are still propagated
unchanged, even though inspection suggests only ENOTDIR, EISDIR and ENOENT are
relevant.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
refs.c | 7 +++++--
refs/debug.c | 4 ++--
refs/files-backend.c | 17 ++++++++---------
refs/packed-backend.c | 5 +++--
refs/refs-internal.h | 9 ++++++---
5 files changed, 24 insertions(+), 18 deletions(-)
diff --git a/refs.c b/refs.c
index 261fd82beb98..43e2ad6b612a 100644
--- a/refs.c
+++ b/refs.c
@@ -1675,13 +1675,16 @@ int refs_read_raw_ref(struct ref_store *ref_store,
const char *refname, struct object_id *oid,
struct strbuf *referent, unsigned int *type)
{
+ int result, failure;
if (!strcmp(refname, "FETCH_HEAD") || !strcmp(refname, "MERGE_HEAD")) {
return refs_read_special_head(ref_store, refname, oid, referent,
type);
}
- return ref_store->be->read_raw_ref(ref_store, refname, oid, referent,
- type);
+ result = ref_store->be->read_raw_ref(ref_store, refname, oid, referent,
+ type, &failure);
+ errno = failure;
+ return result;
}
/* This function needs to return a meaningful errno on failure */
diff --git a/refs/debug.c b/refs/debug.c
index 922e64fa6ad9..887dbb14be6e 100644
--- a/refs/debug.c
+++ b/refs/debug.c
@@ -238,14 +238,14 @@ debug_ref_iterator_begin(struct ref_store *ref_store, const char *prefix,
static int debug_read_raw_ref(struct ref_store *ref_store, const char *refname,
struct object_id *oid, struct strbuf *referent,
- unsigned int *type)
+ unsigned int *type, int *failure_errno)
{
struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
int res = 0;
oidcpy(oid, &null_oid);
res = drefs->refs->be->read_raw_ref(drefs->refs, refname, oid, referent,
- type);
+ type, failure_errno);
if (res == 0) {
trace_printf_key(&trace_refs, "read_raw_ref: %s: %s (=> %s) type %x: %d\n",
diff --git a/refs/files-backend.c b/refs/files-backend.c
index c9511da1d387..3ba3a96e1c6b 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -343,7 +343,7 @@ static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs)
static int files_read_raw_ref(struct ref_store *ref_store,
const char *refname, struct object_id *oid,
- struct strbuf *referent, unsigned int *type)
+ struct strbuf *referent, unsigned int *type, int *failure_errno)
{
struct files_ref_store *refs =
files_downcast(ref_store, REF_STORE_READ, "read_raw_ref");
@@ -354,7 +354,6 @@ static int files_read_raw_ref(struct ref_store *ref_store,
struct stat st;
int fd;
int ret = -1;
- int save_errno;
int remaining_retries = 3;
*type = 0;
@@ -459,10 +458,9 @@ static int files_read_raw_ref(struct ref_store *ref_store,
ret = parse_loose_ref_contents(buf, oid, referent, type);
out:
- save_errno = errno;
+ *failure_errno = errno;
strbuf_release(&sb_path);
strbuf_release(&sb_contents);
- errno = save_errno;
return ret;
}
@@ -541,6 +539,7 @@ static int lock_raw_ref(struct files_ref_store *refs,
struct strbuf ref_file = STRBUF_INIT;
int attempts_remaining = 3;
int ret = TRANSACTION_GENERIC_ERROR;
+ int failure_errno = 0;
assert(err);
files_assert_main_repository(refs, "lock_raw_ref");
@@ -630,8 +629,8 @@ static int lock_raw_ref(struct files_ref_store *refs,
*/
if (files_read_raw_ref(&refs->base, refname,
- &lock->old_oid, referent, type)) {
- if (errno == ENOENT) {
+ &lock->old_oid, referent, type, &failure_errno)) {
+ if (failure_errno == ENOENT) {
if (mustexist) {
/* Garden variety missing reference. */
strbuf_addf(err, "unable to resolve reference '%s'",
@@ -655,7 +654,7 @@ static int lock_raw_ref(struct files_ref_store *refs,
* reference named "refs/foo/bar/baz".
*/
}
- } else if (errno == EISDIR) {
+ } else if (failure_errno == EISDIR) {
/*
* There is a directory in the way. It might have
* contained references that have been deleted. If
@@ -693,13 +692,13 @@ static int lock_raw_ref(struct files_ref_store *refs,
goto error_return;
}
}
- } else if (errno == EINVAL && (*type & REF_ISBROKEN)) {
+ } else if (failure_errno == EINVAL && (*type & REF_ISBROKEN)) {
strbuf_addf(err, "unable to resolve reference '%s': "
"reference broken", refname);
goto error_return;
} else {
strbuf_addf(err, "unable to resolve reference '%s': %s",
- refname, strerror(errno));
+ refname, strerror(failure_errno));
goto error_return;
}
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index dfecdbc1db60..9a09ad7f5f29 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -726,7 +726,8 @@ static struct snapshot *get_snapshot(struct packed_ref_store *refs)
static int packed_read_raw_ref(struct ref_store *ref_store,
const char *refname, struct object_id *oid,
- struct strbuf *referent, unsigned int *type)
+ struct strbuf *referent, unsigned int *type,
+ int *failure_errno)
{
struct packed_ref_store *refs =
packed_downcast(ref_store, REF_STORE_READ, "read_raw_ref");
@@ -739,7 +740,7 @@ static int packed_read_raw_ref(struct ref_store *ref_store,
if (!rec) {
/* refname is not a packed reference. */
- errno = ENOENT;
+ *failure_errno = ENOENT;
return -1;
}
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 29728a339fed..15cc0ddd68ab 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -617,9 +617,11 @@ typedef int reflog_expire_fn(struct ref_store *ref_store,
* properly-formatted or even safe reference name. NEITHER INPUT NOR
* OUTPUT REFERENCE NAMES ARE VALIDATED WITHIN THIS FUNCTION.
*
- * Return 0 on success. If the ref doesn't exist, set errno to ENOENT and return
+ * Return 0 on success. If the ref doesn't exist, set failure_errno to ENOENT and return
* -1. If the ref exists but is neither a symbolic ref nor an object ID, it is
- * broken; set REF_ISBROKEN in type, and return -1. If there is another error
+ * broken; set REF_ISBROKEN in type, and return -1. For the files backend, EISDIR and ENOTDIR
+ * may be set if the ref name is a directory
+ * If there is another error
* reading the ref, set errno appropriately and return -1.
*
* Backend-specific flags might be set in type as well, regardless of
@@ -636,7 +638,8 @@ typedef int reflog_expire_fn(struct ref_store *ref_store,
*/
typedef int read_raw_ref_fn(struct ref_store *ref_store,
const char *refname, struct object_id *oid,
- struct strbuf *referent, unsigned int *type);
+ struct strbuf *referent, unsigned int *type,
+ int *failure_errno);
struct ref_storage_be {
struct ref_storage_be *next;
--
gitgitgadget
prev parent reply other threads:[~2021-04-23 15:31 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-23 10:24 [PATCH] refs: remove EINVAL specification from the errno sideband in read_raw_ref_fn Han-Wen Nienhuys via GitGitGadget
2021-04-23 12:57 ` Jeff King
2021-04-23 15:25 ` Han-Wen Nienhuys
2021-04-23 15:31 ` [PATCH v2 0/3] refs: cleanup errno sideband ref related functions Han-Wen Nienhuys via GitGitGadget
2021-04-23 15:31 ` [PATCH v2 1/3] refs: remove EINVAL specification from the errno sideband in read_raw_ref_fn Han-Wen Nienhuys via GitGitGadget
2021-04-23 15:31 ` [PATCH v2 2/3] refs/files-backend: stop setting errno from lock_ref_oid_basic Han-Wen Nienhuys via GitGitGadget
2021-04-28 4:20 ` Junio C Hamano
2021-04-28 10:55 ` Han-Wen Nienhuys
2021-04-29 1:55 ` Junio C Hamano
2021-04-29 8:52 ` Han-Wen Nienhuys
2021-04-23 15:31 ` Han-Wen Nienhuys via GitGitGadget [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7fbc1c754f435c6d621734685ebee1bdc539c000.1619191907.git.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=git@vger.kernel.org \
--cc=hanwen@google.com \
--cc=hanwenn@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).