Re: [PATCH 01/39] pack: make packed_git_mru global a value instead of a pointer

[Jeff King <peff@peff.net>]

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