Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
> @@ -1722,8 +1722,6 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
> if (refs_read_raw_ref(refs, refname, oid, &sb_refname,
> &read_flags, failure_errno)) {
> *flags |= read_flags;
> - if (errno)
> - *failure_errno = errno;
Looks good.
The whole point of passing failure_errno down to refs_read_raw_ref()
is that we capture the reason of the failure there without having to
rely on errno at this point in the flow. Removal of this assignment
makes perfect sense.
But ...
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index b529fdf237e..43a3b882d7c 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -382,7 +382,6 @@ static int files_read_raw_ref(struct ref_store *ref_store, const char *refname,
> if (lstat(path, &st) < 0) {
> int ignore_errno;
> myerr = errno;
> - errno = 0;
> if (myerr != ENOENT)
> goto out;
> if (refs_read_raw_ref(refs->packed_ref_store, refname, oid,
> @@ -399,7 +398,6 @@ static int files_read_raw_ref(struct ref_store *ref_store, const char *refname,
> strbuf_reset(&sb_contents);
> if (strbuf_readlink(&sb_contents, path, st.st_size) < 0) {
> myerr = errno;
> - errno = 0;
> if (myerr == ENOENT || myerr == EINVAL)
> /* inconsistent with lstat; retry */
> goto stat_ref;
> @@ -469,6 +467,7 @@ static int files_read_raw_ref(struct ref_store *ref_store, const char *refname,
>
> strbuf_release(&sb_path);
> strbuf_release(&sb_contents);
> + errno = 0;
> return ret;
> }
... it is not clear to me if this part makes sense. If everybody
above the callstack uses failure_errno as the source of truth,
clearing errno here in this function should not be necessary and is
misleading to readers of the code, no?
