Re: [PATCH GSoC 2/5] fetch-object-info: parse type from server response

Karthik Nayak <[email protected]> Wed, 29 Jul 2026 17:47:17 -0500
Newsgroups	org.kernel.vger.git
Message-ID	<CAOLa=ZQbf2vdYOkFBHQ2Kkk=Yc0uSZXEPy+FQPyzp9C8=sUFEQ@mail.gmail.com>
Pablo Sabater <[email protected]> writes:

> The server can handle type requests but does not advertise the
> capability yet. Prepare the client to know how to parse the server
> response once the server advertises the capability.
>

Nit, I was a bit confused by the flow from the first to the second
sentence. Maybe:

    The server can handle type requests but does not advertise the
    capability yet. As a percursor, prepare the client to know how to
    parse the server response.

> Mentored-by: Karthik Nayak <[email protected]>
> Mentored-by: Chandra Pratap <[email protected]>
> Signed-off-by: Pablo Sabater <[email protected]>
> ---
>  fetch-object-info.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/fetch-object-info.c b/fetch-object-info.c
> index ba7e179c44..cf6b94afb8 100644
> --- a/fetch-object-info.c
> +++ b/fetch-object-info.c
> @@ -50,6 +50,7 @@ int fetch_object_info(const enum protocol_version version, struct object_info_ar
>  		      const int stateless_rpc, const int fd_out)
>  {
>  	int size_index = -1;
> +	int type_index = -1;
>
>  	switch (version) {
>  	case protocol_v2:
> @@ -101,8 +102,13 @@ int fetch_object_info(const enum protocol_version version, struct object_info_ar
>  			for (size_t j = 0; j < args->oids->nr; j++)
>  				object_info_data[j].sizep =
>  					xcalloc(1, sizeof(*object_info_data[j].sizep));
> +		} else if (!strcmp(reader->line, "type")) {
> +			type_index = (int)i;
> +			for (size_t j = 0; j < args->oids->nr; j++)
> +				object_info_data[j].typep =
> +					xcalloc(1, sizeof(*object_info_data[j].typep));
>  		} else {
> -			BUG("only size is supported");
> +			BUG("unexpected object-info option: %s", reader->line);
>  		}
>  	}
>
> @@ -148,6 +154,10 @@ int fetch_object_info(const enum protocol_version version, struct object_info_ar
>  			    object_info_values.items[0].string,
>  			    object_info_values.items[size_index + 1].string);
>
> +		if (type_index >= 0)
> +			*object_info_data[i].typep =
> +				type_from_string(object_info_values.items[type_index + 1].string);
> +

For size, passing the value as a number makes sense, since the value
represents the field directly.

For type however, we're passing in a value, while the actual field is a
string. We rely on `type_from_string()` to make that translation for us,
which internatlly depends on `object_type_strings[]`. What if there is a
mismatch between the server and the client? Shouldn't we be sending in
the string itself?

>  		string_list_clear(&object_info_values, 0);
>  	}
>  	check_stateless_delimiter(stateless_rpc, reader, "stateless delimiter expected");
>
> --
> 2.54.0
signature.asc (application/pgp-signature, 690 B)