On Tue, Aug 29, 2017 at 11:48:27PM -0700, Jonathan Nieder wrote: > The MRU cache that keeps track of recently used packs is represented > using two global variables: > > struct mru packed_git_mru_storage; > struct mru *packed_git_mru = &packed_git_mru_storage; > > Callers never assign to the packed_git_mru pointer, though, so we can > simplify by eliminating it and using &packed_git_mru_storage (renamed > to &packed_git_mru) directly. This variable is only used by the > packfile subsystem, making this a relatively uninvasive change (and > any new unadapted callers would trigger a compile error). > > Noticed while moving these globals to the object_store struct. Sounds reasonable. I think I had originally wanted to hide the implementation and make the MRU more opaque, hence exposing only the pointer. But since I ended up just letting people directly walk over the struct pointers to do iteration, it needs to expose the struct internals anyway, and this layer of indirection isn't useful. > --- > builtin/pack-objects.c | 4 ++-- > cache.h | 4 ++-- > packfile.c | 12 +++++------- > 3 files changed, 9 insertions(+), 11 deletions(-) The patch looks good to me. As an aside, the mru code could probably be simplified a bit by reusing the list implementation from list.h (both were added around the same time, and it wasn't worth creating a dependency then, but I think list.h is useful and here to stay at this point). It's definitely not critical to put that into this already-large series, though. Maybe we can use Junio's #leftoverbits tag. :) -Peff
