On Thu, Jun 06, 2024 at 07:28:52AM +0200, Patrick Steinhardt wrote: > the ref storage migration was merged to `next`, but got reverted due to > some additional findings by Peff and/or Coverity. > > Changes compared to v4: > > - Adapt comment of `ref_store_init()` to the new parameter. > > - Fix use of an uninitialized return value in `for_each_root_ref()`. > > - Fix overwrite of ret code in `files_ref_store_remove_on_disk()`. > > - Adapt an error message to more clearly point out that deletion of > "refs/" directory failed in `reftable_be_remove_on_disk()`. > > - Fix a leak when `mkdtemp()` fails. These all looked good to me (though I did not carefully read the original topic, so was just looking at the parts I mentioned earlier). > 6: f7577a0ab3 ! 6: 86cf0c84b1 refs/files: extract function to iterate through root refs > @@ refs/files-backend.c: static void add_root_refs(struct files_ref_store *refs, > strbuf_setlen(&refname, dirnamelen); > } > + > ++ ret = 0; > ++ > +done: > strbuf_release(&refname); > strbuf_release(&path); Since the context doesn't show much, I wondered whether there was any case where we'd overwrite an earlier "ret" here. But nope, we always jump to "done" after finding "ret" contains a non-zero value. So setting it to zero here is the right thing. -Peff
