On Wed, Sep 13, 2017 at 10:03:58AM +0200, Michael Haggerty wrote: > On 09/12/2017 07:31 PM, Jonathan Nieder wrote: > > From: Stefan Beller <sbel...@google.com> > > > > Pass DO_FOR_EACH_INCLUDE_BROKEN when iterating over replacement refs > > so that the iteration does not require opening the named objects from > > the object store. This avoids a dependency cycle between object access > > and replace ref iteration. > > > > Moreover the ref subsystem has not been migrated yet to access the > > object store via passed in repository objects. As a result, without > > this patch, iterating over replace refs in a repository other than > > the_repository it produces errors: > > > > error: refs/replace/3afabef75c627b894cccc3bcae86837abc7c32fe does not > > point to a valid object! > > > > Noticed while adapting the object store (and in particular its > > evaluation of replace refs) to handle arbitrary repositories. > > Have you checked that consumers of this API can handle broken > references? Aside from missing values, broken references can have > illegal names (though hopefully not unsafe in the sense of causing > filesystem traversal outside of `$GITDIR`).
It looks like there are only two callers. One complains about names that aren't 40-hex. The other ("git replace -l") parses the 40-hex in "long" mode, but will print anything in short mode (not that printing is very dangerous). I do have to wonder if for_each_replace_ref() should just do the 40-hex syntactic check itself (and issue a warning for other refs). It seems like that would be the point of calling for_each_replace_ref() instead of just for_each_ref_in("refs/replace") yourself, and both of the callers would benefit. -Peff