On Sun, Oct 02, 2016 at 11:38:56AM -0400, Jeff King wrote:
> On Sun, Oct 02, 2016 at 11:07:39AM +0200, René Scharfe wrote:
>
> > diff --git a/builtin/count-objects.c b/builtin/count-objects.c
> > index ba92919..b2afe36 100644
> > --- a/builtin/count-objects.c
> > +++ b/builtin/count-objects.c
> > @@ -73,6 +73,12 @@ static int count_cruft(const char *basename, const char *path, void *data)
> > return 0;
> > }
> >
> > +static int print_alt_odb(struct alternate_object_database *alt, void *data)
> > +{
> > + puts(alt->base);
> > + return 0;
> > +}
>
> This turns out to be wrong, because alt->base _isn't_ just the base;
> it's also the scratch buffer we write into to form pathnames. So if
> we've used the buffer to look up an object, we'll get that object name
> here, not just the base.
>
> It tends to work for your command because we do nothing except list the
> alternates and exit, but I'm not sure if there are code paths which
> might access an object.
>
> I think giving a known state to the callback should be the
> responsibility of foreach_alt_odb(), though.
Actually, it looks like there are a lot of opportunities for cleanups in
that area, and maybe some bugfixes (e.g., we should consistently be
using fspathcmp and not memcmp to check for duplicate paths). I started
on a series, but don't have anything to show yet (just FYI in case you
were thinking about doing the same).
-Peff