Re: errno oversight - Han-Wen Nienhuys

From: Han-Wen Nienhuys <hanwen@google.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git <git@vger.kernel.org>
Subject: Re: errno oversight
Date: Wed, 8 Dec 2021 14:05:10 +0100	[thread overview]
Message-ID: <CAFQ2z_MmWywH2AH3YaB+v67e-0jNsH9sz8dgqYenQNM6Gqz_Hw@mail.gmail.com> (raw)
In-Reply-To: <211208.86sfv3l0op.gmgdl@evledraar.gmail.com>

On Wed, Dec 8, 2021 at 1:51 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> > In refs.c in origin/next and origin/seen, we have the following fragment:
>
> It's in "master".

Correct, but it's also in seen.

> >                    if (refs_read_raw_ref(refs, refname, oid, &sb_refname,
> >                                       &read_flags, failure_errno)) {
> >                         *flags |= read_flags;
> >                         if (errno)
> >                                 *failure_errno = errno;
> >
> > overwriting failure_errno looks like an oversight?
>
> This is from my ef18119dec8 (refs API: add a version of
> refs_resolve_ref_unsafe() with "errno", 2021-10-16).
>
> I don't see the bug here, overwriting *failure_errno is the point of
> that variable. We're keeping the right errno value right after a
> failure, and passing it up to our caller.

The reftable code for refs_read_raw_ref places a sensible value into
its failure_errno argument. Then the "if (errno)" replaces it with
whatever the outcome of the last syscall was. I started the errno work
to get rid of this kind of global state manipulation; am I missing
something?

-- 
Han-Wen Nienhuys - Google Munich
I work 80%. Don't expect answers from me on Fridays.
--

Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich

Registergericht und -nummer: Hamburg, HRB 86891

Sitz der Gesellschaft: Hamburg

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado