From: Chandra Pratap <chandrapratap3519@gmail.com>
Date: Wed, 17 Jun 2026 10:16:20 GMT
Subject: Re: [PATCH GSoC RFC v12 12/12] cat-file: make remote-object-info allow-list dynamic
Message-ID: <CA+J6zkTrBO9paxkMtnR1cDtD=LQT8dzbVNxgzzYNz_bpzrvcwQ@mail.gmail.com>
In-Reply-To: <CAN5EUNQHSd=0z26iG0gk24TEtgg1n8CC+H9bkqRACyErNgLxEA@mail.gmail.com>

On Tue, 9 Jun 2026 at 23:04, Pablo Sabater <pabloosabaterr@gmail.com> wrote:
> [snip]
> > > diff --git a/fetch-object-info.c b/fetch-object-info.c
> > > index 51a898430d..425929a269 100644
> > > --- a/fetch-object-info.c
> > > +++ b/fetch-object-info.c
> > > @@ -39,6 +39,12 @@ int fetch_object_info(const enum protocol_version version, struct object_info_ar
> > >         case protocol_v2:
> > >                 if (!server_supports_v2("object-info"))
> > >                         die(_("object-info capability is not enabled on the server"));
> > > +
> > > +               for (int i = args->object_info_options->nr - 1; i >= 0; i--)
> >
> > Isn't args->object_info_options->nr of type size_t? We should probably
> > do something
> > like:
> >
> > for (size_t i = 0; i < args->args->object_info_options->nr; i++)
> >
> > instead.
>
> Hi!
>
> void unsorted_string_list_delete_item(struct string_list *list, int i,
> int free_util)
> {
>         if (list->strdup_strings)
>                 free(list->items[i].string);
>         if (free_util)
>                 free(list->items[i].util);
>         list->items[i] = list->items[list->nr-1];
>         list->nr--;
> }
>
>
> I made it backwards because of "list->items[i] = list->items[list->nr
> - 1];" If we made it from 0..nr and we delete the first element, for
> the next iteration, the last element is at [0] but we are on [1] and
> that swapped element never gets evaluated.

Makes sense now.

> About size_t, yes, it is size_t but because we go backwards 0 - 1
> would fail, also unsorted_string_list_delete_item() signature has "int
> i". The options that can be on that list will be a small number so
> there should be no problem, should I cast it explicitly?

Yes, I think explicit casting with a short comment explaining why it is
fine to do so will be much better.

Thanks,
Chandra.