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. -Peff
