On Thu, Jan 21, 2021 at 5:14 PM Han-Wen Nienhuys [off-list ref] wrote:

On Thu, Jan 21, 2021 at 4:55 PM Ævar Arnfjörð Bjarmason
[off-list ref] wrote:

quoted

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