On Thu, Jan 21, 2021 at 5:14 PM Han-Wen Nienhuys <hanwen@xxxxxxxxxx> wrote:
>
> On Thu, Jan 21, 2021 at 4:55 PM Ævar Arnfjörð Bjarmason
> <avarab@xxxxxxxxx> wrote:
> > I must say I do find it a bit more concerning in the case of the
> > reftable series. I.e. the git_reftable_read_raw_ref() function seems
> > like a faithful copy of the general logic (at least this part) of the
> > refs_resolve_ref_unsafe() function in refs.c.
> >
> > That I can remove error checking/handling from this place means existing
> > general logic was faithfully copied without checking that we have a test
> > for it, or adding one. Or perhaps given the history of this codebase
> > there's out-of-tree tests for it somewhere?
>
> One of the earlier iterations of this code didn't have the errno
> handling, and it caused a bug that I tracked down to the errno
> handling. I'm fairly sure of this, because I remember being perplexed
> and flabbergasted at using errno as an out-of-band API mechanism. The
> first iteration of reftable code was about a year ago, so if you
> checkout the Git source code from that time, and remove the errno
> assignments, you would be able to see which test exercised this code
> path.
>
> Or you could check where errno is read-back (I believe ENOENT is the
> case that has special meaning)
In refs.c, there is behavior that depends on errno,
if (refs_read_raw_ref(refs, refname,
oid, &sb_refname, &read_flags)) {
..
/*
* Otherwise a missing ref is OK. But the files backend
* may show errors besides ENOENT if there are
* similarly-named refs.
*/
if (errno != ENOENT &&
errno != EISDIR &&
errno != ENOTDIR)
return NULL;
blame tells me it has something to do with dir/file conflicts and
commit a1c1d8170db.
--
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
