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) -- 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
