From: Han-Wen Nienhuys <hanwen@xxxxxxxxxx>
This lets us use the explicit errno output parameter in refs_resolve_ref_unsafe.
Some of our callers explicitly do not care about the errno, rather
than understanding NULL let's have them declare that they don't care
by passing in an "ignore_errno". There's only three of them, and using
that pattern will make it more obvious that they want to throw away
data, let's also add a comment to one of the callers about why we'd
like to ignore the errno.
Let's not extend that to refs_resolve_ref_unsafe() itself for now, it
has a large set of legacy callers, so we're faking up the old "errno"
behavior for it. We can convert those callers to
refs_resolve_ref_unsafe_with_errno() later.
We are leaving out out the refs_read_special_head() in
refs_read_raw_ref() for now, as noted in the next commit moving it to
"failure_errno" will require some special consideration.
We're intentionally mis-indenting the argument list of the new
refs_resolve_ref_unsafe_with_errno(), it will be non-static in a
subsequent commit, doing it this way makes that diff smaller.
Signed-off-by: Han-Wen Nienhuys <hanwen@xxxxxxxxxx>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
---
refs.c | 61 ++++++++++++++++++++++++++++++-------------
refs/files-backend.c | 10 ++++---
refs/packed-backend.c | 7 ++---
refs/refs-internal.h | 6 ++---
4 files changed, 56 insertions(+), 28 deletions(-)
diff --git a/refs.c b/refs.c
index da0cc82ca66..4c782fa2978 100644
--- a/refs.c
+++ b/refs.c
@@ -1672,30 +1672,33 @@ static int refs_read_special_head(struct ref_store *ref_store,
return result;
}
-int refs_read_raw_ref(struct ref_store *ref_store,
- const char *refname, struct object_id *oid,
- struct strbuf *referent, unsigned int *type)
+int refs_read_raw_ref(struct ref_store *ref_store, const char *refname,
+ struct object_id *oid, struct strbuf *referent,
+ unsigned int *type, int *failure_errno)
{
+ assert(failure_errno);
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, &errno);
+ type, failure_errno);
}
-/* This function needs to return a meaningful errno on failure */
-const char *refs_resolve_ref_unsafe(struct ref_store *refs,
- const char *refname,
- int resolve_flags,
- struct object_id *oid, int *flags)
+static const char *refs_resolve_ref_unsafe_with_errno(struct ref_store *refs,
+ const char *refname,
+ int resolve_flags,
+ struct object_id *oid,
+ int *flags, int *failure_errno)
{
static struct strbuf sb_refname = STRBUF_INIT;
struct object_id unused_oid;
int unused_flags;
int symref_count;
+ assert(failure_errno);
+
if (!oid)
oid = &unused_oid;
if (!flags)
@@ -1706,7 +1709,7 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
if (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||
!refname_is_safe(refname)) {
- errno = EINVAL;
+ *failure_errno = EINVAL;
return NULL;
}
@@ -1724,8 +1727,8 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
for (symref_count = 0; symref_count < SYMREF_MAXDEPTH; symref_count++) {
unsigned int read_flags = 0;
- if (refs_read_raw_ref(refs, refname,
- oid, &sb_refname, &read_flags)) {
+ if (refs_read_raw_ref(refs, refname, oid, &sb_refname,
+ &read_flags, failure_errno)) {
*flags |= read_flags;
/* In reading mode, refs must eventually resolve */
@@ -1737,9 +1740,9 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
* may show errors besides ENOENT if there are
* similarly-named refs.
*/
- if (errno != ENOENT &&
- errno != EISDIR &&
- errno != ENOTDIR)
+ if (*failure_errno != ENOENT &&
+ *failure_errno != EISDIR &&
+ *failure_errno != ENOTDIR)
return NULL;
oidclr(oid);
@@ -1766,7 +1769,7 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
if (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||
!refname_is_safe(refname)) {
- errno = EINVAL;
+ *failure_errno = EINVAL;
return NULL;
}
@@ -1774,10 +1777,24 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
}
}
- errno = ELOOP;
+ *failure_errno = ELOOP;
return NULL;
}
+const char *refs_resolve_ref_unsafe(struct ref_store *refs, const char *refname,
+ int resolve_flags, struct object_id *oid,
+ int *flags)
+{
+ int failure_errno = 0;
+ const char *refn;
+ refn = refs_resolve_ref_unsafe_with_errno(refs, refname, resolve_flags,
+ oid, flags, &failure_errno);
+ if (!refn)
+ /* For unmigrated legacy callers */
+ errno = failure_errno;
+ return refn;
+}
+
/* backend functions */
int refs_init_db(struct strbuf *err)
{
@@ -2228,6 +2245,13 @@ int refs_verify_refname_available(struct ref_store *refs,
strbuf_grow(&dirname, strlen(refname) + 1);
for (slash = strchr(refname, '/'); slash; slash = strchr(slash + 1, '/')) {
+ /*
+ * Just saying "Is a directory" when we e.g. can't
+ * lock some multi-level ref isn't very informative,
+ * the user won't be told *what* is a directory, so
+ * let's not use strerror() below.
+ */
+ int ignore_errno;
/* Expand dirname to the new prefix, not including the trailing slash: */
strbuf_add(&dirname, refname + dirname.len, slash - refname - dirname.len);
@@ -2239,7 +2263,8 @@ int refs_verify_refname_available(struct ref_store *refs,
if (skip && string_list_has_string(skip, dirname.buf))
continue;
- if (!refs_read_raw_ref(refs, dirname.buf, &oid, &referent, &type)) {
+ if (!refs_read_raw_ref(refs, dirname.buf, &oid, &referent,
+ &type, &ignore_errno)) {
strbuf_addf(err, _("'%s' exists; cannot create '%s'"),
dirname.buf, refname);
goto cleanup;
diff --git a/refs/files-backend.c b/refs/files-backend.c
index b3de3633969..f962495e456 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -381,10 +381,11 @@ static int files_read_raw_ref(struct ref_store *ref_store, const char *refname,
goto out;
if (lstat(path, &st) < 0) {
+ int ignore_errno;
if (errno != ENOENT)
goto out;
- if (refs_read_raw_ref(refs->packed_ref_store, refname,
- oid, referent, type)) {
+ if (refs_read_raw_ref(refs->packed_ref_store, refname, oid,
+ referent, type, &ignore_errno)) {
errno = ENOENT;
goto out;
}
@@ -418,13 +419,14 @@ static int files_read_raw_ref(struct ref_store *ref_store, const char *refname,
/* Is it a directory? */
if (S_ISDIR(st.st_mode)) {
+ int ignore_errno;
/*
* Even though there is a directory where the loose
* ref is supposed to be, there could still be a
* packed ref:
*/
- if (refs_read_raw_ref(refs->packed_ref_store, refname,
- oid, referent, type)) {
+ if (refs_read_raw_ref(refs->packed_ref_store, refname, oid,
+ referent, type, &ignore_errno)) {
errno = EISDIR;
goto out;
}
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 47247a14917..52cdc94a26e 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1347,6 +1347,7 @@ int is_packed_transaction_needed(struct ref_store *ref_store,
ret = 0;
for (i = 0; i < transaction->nr; i++) {
struct ref_update *update = transaction->updates[i];
+ int failure_errno;
unsigned int type;
struct object_id oid;
@@ -1357,9 +1358,9 @@ int is_packed_transaction_needed(struct ref_store *ref_store,
*/
continue;
- if (!refs_read_raw_ref(ref_store, update->refname,
- &oid, &referent, &type) ||
- errno != ENOENT) {
+ if (!refs_read_raw_ref(ref_store, update->refname, &oid,
+ &referent, &type, &failure_errno) ||
+ failure_errno != ENOENT) {
/*
* We have to actually delete that reference
* -> this transaction is needed.
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index ddd6d7f8ebd..7ade286260c 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -149,9 +149,9 @@ struct ref_update {
const char refname[FLEX_ARRAY];
};
-int refs_read_raw_ref(struct ref_store *ref_store,
- const char *refname, struct object_id *oid,
- struct strbuf *referent, unsigned int *type);
+int refs_read_raw_ref(struct ref_store *ref_store, const char *refname,
+ struct object_id *oid, struct strbuf *referent,
+ unsigned int *type, int *failure_errno);
/*
* Write an error to `err` and return a nonzero value iff the same
--
2.33.0.662.g438caf9576d
