| Message ID | 95d64d73353d8689e3928b8c9444490d0cdebfc9.1619710329.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | refs: cleanup errno sideband ref related functions | expand |
> From: Han-Wen Nienhuys <hanwen@google.com> > > The only caller of refs_resolve_ref_unsafe_with_errno() is in > refs/files-backend.c, and it only cares about EISDIR and ENOTDIR. > > Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> > --- > refs.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/refs.c b/refs.c > index 6e746cb01f24..597e4d1f18f9 100644 > --- a/refs.c > +++ b/refs.c > @@ -1706,7 +1706,6 @@ static const char *refs_resolve_ref_unsafe_errno(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; > return NULL; > } > I don't think this is related to avoiding errno and conveying error information through an out param. But besides that, as it is, I'm not sure that this is correct. Even if EINVAL is not checked by the caller, setting errno to EINVAL here means avoiding exposing a potential EISDIR/ENOTDIR that a preceding call set. Same comment for the other instances.
On Thu, Jun 3, 2021 at 4:55 AM Jonathan Tan <jonathantanmy@google.com> wrote: > I don't think this is related to avoiding errno and conveying error > information through an out param. But besides that, as it is, I'm not > sure that this is correct. Even if EINVAL is not checked by the caller, > setting errno to EINVAL here means avoiding exposing a potential > EISDIR/ENOTDIR that a preceding call set. Same comment for the other > instances. You are right, but it's probably moot, because the follow-up commit stops using errno (making it impossible for EISDIR/ENOTDIR to haphazardly appear), but I dropped this commit from the series.
diff --git a/refs.c b/refs.c index 6e746cb01f24..597e4d1f18f9 100644 --- a/refs.c +++ b/refs.c @@ -1706,7 +1706,6 @@ static const char *refs_resolve_ref_unsafe_errno(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; return NULL; } @@ -1766,7 +1765,6 @@ static const char *refs_resolve_ref_unsafe_errno(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; return NULL; } @@ -1774,7 +1772,6 @@ static const char *refs_resolve_ref_unsafe_errno(struct ref_store *refs, } } - errno = ELOOP; return NULL; }