"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> From: Han-Wen Nienhuys <hanwen@xxxxxxxxxx>
>
> The EINVAL error from parse_loose_ref_contents is used in files-backend
> to create a custom error message.
>
> Signed-off-by: Han-Wen Nienhuys <hanwen@xxxxxxxxxx>
> Reviewed-By: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
I think a -By in the trailer is spelled with lowercase, i.e. "Reviewed-by".
> ---
> refs.c | 8 +++++---
> refs/files-backend.c | 13 +++++++++----
> refs/refs-internal.h | 6 ++++--
> 3 files changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 25d80e544d0..eca7310e7a4 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1653,7 +1653,8 @@ int for_each_fullref_in_prefixes(const char *namespace,
>
> static int refs_read_special_head(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 strbuf full_path = STRBUF_INIT;
> struct strbuf content = STRBUF_INIT;
> @@ -1663,7 +1664,8 @@ static int refs_read_special_head(struct ref_store *ref_store,
> if (strbuf_read_file(&content, full_path.buf, 0) < 0)
> goto done;
>
> - result = parse_loose_ref_contents(content.buf, oid, referent, type);
> + result = parse_loose_ref_contents(content.buf, oid, referent, type,
> + failure_errno);
>
> done:
> strbuf_release(&full_path);
> @@ -1683,7 +1685,7 @@ int refs_read_raw_ref(struct ref_store *ref_store, const char *refname,
>
> if (!strcmp(refname, "FETCH_HEAD") || !strcmp(refname, "MERGE_HEAD")) {
> return refs_read_special_head(ref_store, refname, oid, referent,
> - type);
> + type, failure_errno);
> }
>
> return ref_store->be->read_raw_ref(ref_store, refname, oid, referent,
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index f38c9703504..b43ec4c66cb 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -455,9 +455,13 @@ stat_ref:
> strbuf_rtrim(&sb_contents);
> buf = sb_contents.buf;
>
> - ret = parse_loose_ref_contents(buf, oid, referent, type);
> -
> + ret = parse_loose_ref_contents(buf, oid, referent, type, failure_errno);
> + errno = *failure_errno;
All of the above makes sense.
> out:
> + /* Collect all types of failures from errno. Many system calls in this
> + * function can fail with ENOTDIR/EISDIR, and we want to collect all of
> + * them.
> + */
Style.
> *failure_errno = errno;
> strbuf_release(&sb_path);
> strbuf_release(&sb_contents);
> @@ -465,7 +469,8 @@ out:
> }
>
> int parse_loose_ref_contents(const char *buf, struct object_id *oid,
> - struct strbuf *referent, unsigned int *type)
> + struct strbuf *referent, unsigned int *type,
> + int *failure_errno)
> {
> const char *p;
> if (skip_prefix(buf, "ref:", &buf)) {
> @@ -484,7 +489,7 @@ int parse_loose_ref_contents(const char *buf, struct object_id *oid,
> if (parse_oid_hex(buf, oid, &p) ||
> (*p != '\0' && !isspace(*p))) {
> *type |= REF_ISBROKEN;
> - errno = EINVAL;
> + *failure_errno = EINVAL;
> return -1;
> }
OK.
> return 0;
> diff --git a/refs/refs-internal.h b/refs/refs-internal.h
> index 54f57c6a2df..bf581e70cf6 100644
> --- a/refs/refs-internal.h
> +++ b/refs/refs-internal.h
> @@ -689,10 +689,12 @@ struct ref_store {
> };
>
> /*
> - * Parse contents of a loose ref file.
> + * Parse contents of a loose ref file. *failure_errno maybe be set to EINVAL for
> + * invalid contents.
> */
OK.
> int parse_loose_ref_contents(const char *buf, struct object_id *oid,
> - struct strbuf *referent, unsigned int *type);
> + struct strbuf *referent, unsigned int *type,
> + int *failure_errno);
>
> /*
> * Fill in the generic part of refs and add it to our collection of
