Jeff King <peff@xxxxxxxx> writes: > On Thu, Jul 15, 2021 at 02:02:40AM +0200, Ævar Arnfjörð Bjarmason wrote: > >> > Maybe that is splitting hairs, but I definitely try to err on the side >> > of caution and over-analysis when touching tricky code (and the >> > ref-backend code is in my experience one of the trickiest spots for >> > corner cases, races, etc). >> >> I'd entirely forgotten about this, but I had a patch to remove that >> "oid" call entirely, as it's really an unrelated bug/undesired behavior >> >> You also looked at it at the time & Michael Haggerty reviewed it: >> https://lore.kernel.org/git/20190315155959.12390-9-avarab@xxxxxxxxx/ >> >> The state of that patch was that I needed to get to some minor issues >> with it (commit message rewording, cleaning up some related callers), >> but the fundamental approach seemed good. >> >> I then split it off from the v4 of that series to get the non-tricky >> changes in: >> https://lore.kernel.org/git/20190328161434.19200-1-avarab@xxxxxxxxx/ >> >> I then just never got to picking it up again, I'll probably re-roll it & >> make it a part of this series, then we can remove this whole OID != NULL >> case and will be sure nothing fishy's going on. > > Yeah, that sounds like a good path forward. I do think the patch under > discussion here is probably the right thing to do. But it becomes all > the more obvious if lock_ref_oid_basic() ends up dropping that parameter > entirely. OK, so what's the final verdict on this step? It is unfortunate that when Ævar took over a topic from Han-Wen, this patch has been inserted as the very first step before the patches in the series, so until we know we are happy with it, it takes several other patches hostage. Thanks.
