On Mon, Aug 23, 2021 at 2:13 PM Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote: > > Since my "refs API: remove OID argument to reflog_expire()" we don't > have the "oid" as part of the reflog_expire() signature. Instead the > reflog_expire() should pass the OID of the tip of the "locked" ref to > the prepare_fn(). > > In files_reflog_expire() we do that by getting the OID from > lock_ref_oid_basic(). I'm assuming (but am not familiar enough with > reftable...) that by the time we get here we've got a locked ref > already in some way, so let's just use > refs_resolve_ref_unsafe_with_errno() to lookup the current OID of that > presumably-locked ref. I quickly looked at the files code, but I don't understand why the OID needs to be passed-in (before your refactoring): in builtin/reflog.c (before), the current OID is read, with any protection. This means that its value can't be trusted. After your refactoring, you lock the ref. I guess in the files backend this protects against non-atomic update of (ref, reflog) racing with a concurrent reflog expiry? In reftable, the (ref,reflog) update is atomic, so there is no need for locking to properly sequence operations. -- 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
