On Thu, May 23, 2019 at 08:59:59AM +0000, Eric Wong wrote:

> > We never delete entries from the in-memory packed_git list; a reprepare
> > only adds to the list. You'd need to teach update_server_info() to
> > ignore packs which are no longer present (or switch to exec-ing a
> > separate update-server-info binary).
> 
> Ah, checking files_exists() and setting a bit seems sufficient.

Yes, though we do we even need to store the bit?

I.e.,

> @@ -199,12 +200,16 @@ static void init_pack_info(const char *infofile, int 
> force)
>                */
>               if (!p->pack_local)
>                       continue;
> +             if (!file_exists(p->pack_name)) {
> +                     p->pack_unlinked = 1;
> +                     continue;
> +             }
>               i++;
>       }
>       num_pack = i;
>       info = xcalloc(num_pack, sizeof(struct pack_info *));
>       for (i = 0, p = get_all_packs(the_repository); p; p = p->next) {
> -             if (!p->pack_local)
> +             if (!p->pack_local || p->pack_unlinked)
>                       continue;
>               assert(i < num_pack);
>               info[i] = xcalloc(1, sizeof(struct pack_info));

If we just check file_exists() in the second loop, then this is entirely
local to update_server_info(). And other users of packed_git do not have
to wonder who is responsible for setting that flag in the global list.

It does mean you'd over-allocate the array (and num_pack would have to
be adjusted down to "i" after the second loop), but that's not a big
deal.  I do think the whole two-loop thing would be more readable if we
simply grew it on the fly with ALLOC_GROW().

-Peff

Reply via email to