Re: [PATCH GSoC RFC v12 12/12] cat-file: make remote-object-info allow-list dynamic

From: Pablo Sabater <pabloosabaterr@gmail.com>
Date: Jun 9, 2026, 17:34 UTC
Message-ID: <CAN5EUNQHSd=0z26iG0gk24TEtgg1n8CC+H9bkqRACyErNgLxEA@mail.gmail.com>
In-Reply-To: <CA+J6zkQ22en2HgH03EedKOfC+jLcHH2UbwpH0h_bDEAHR6B2pg@mail.gmail.com>

El mar, 9 jun 2026 a las 17:32, Chandra Pratap (<chandrapratap3519@gmail.com>) escribió:

Show 44 quoted lines

>
> On Mon, 8 Jun 2026 at 15:45, Pablo Sabater <pabloosabaterr@gmail.com> wrote:
> >
> > The static allow-list in expand_atom() is hardcoded to only allow
> > "objectname" and "objectsize" for remote queries. This works because
> > up to this point all servers will either support object-info with name
> > and size or they do not support them at all, but we cannot expect that
> > in a future different servers with different git versions to have the
> > same object-info capabilities. Therefore, the allow_list needs to be
> > dynamic depending on what does the server advertise.
> >
> > The client will now:
> >
> > 1. Request the protocol option that the placeholder refers to (i.e.
> >    "size" when "%(objectsize)").
> >
> > 2. Filters the request in fetch_object_info() dropping any option that
> >    the server does not advertise.
> >
> > 3. After the fetching, the options that haven't been dropped are the ones
> >    fetched and supported by the server, these supported options are
> >    mapped and remote_allowed_atoms is populated with the placeholders.
> >
> > 4. expand_atom() checks remote_allowed_atoms with the same behaviour as
> >    the static allow_list had.
> >
> > Move object_info_options out of get_remote_info so the caller which has
> > data can select what options will be requested instead of requesting
> > always size.
> > Move batch_object_write() out so there will always be an output even if
> > all the placeholders are not supported by the server (returns an empty
> > line).
> >
> > Include "type" in the object_info_options so once the server supports
> > it, the clients know already how to request it.
> >
> > Mentored-by: Karthik Nayak <karthik.188@gmail.com>
> > Mentored-by: Chandra Pratap <chandrapratap3519@gmail.com>
> > Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
> > ---
> >  builtin/cat-file.c  | 85 ++++++++++++++++++++++++++++++++---------------------
> >  fetch-object-info.c |  6 ++++
> >  2 files changed, 58 insertions(+), 33 deletions(-)
> >

[snip]

Show 18 quoted lines

> > 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.

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?

Show 16 quoted lines

>
> > +                       if (!server_supports_feature("object-info",
> > +                                                    args->object_info_options->items[i].string, 0))
> > +                               unsorted_string_list_delete_item(args->object_info_options, i, 0);
> > +
> >                 send_object_info_request(fd_out, args);
> >                 break;
> >         case protocol_v1:
> >
> > --
> > 2.54.0
>
> Other than these, the patch series LGTM for now.
>
> Thanks,
> Chandra.

Thanks,

Pablo.