From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 16 Jun 2026 21:31:17 GMT
Subject: Re: [PATCH GSoC RFC v12 09/12] transport: add client support for object-info
Message-ID: <xmqqse6m18be.fsf@gitster.g>
In-Reply-To: <xmqq1pe62pgo.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

> Pablo Sabater <pabloosabaterr@gmail.com> writes:
>
> [jc: removed recipients from Cc: list whose addresses bounce]
>
>> From: Calvin Wan <calvinwan@google.com>
>>
>> Sometimes, it is beneficial to retrieve information about an object
>> without downloading it entirely. The server-side logic for this
>> functionality was implemented in commit "a2ba162cda (object-info:
>> ...
>> diff --git a/fetch-object-info.c b/fetch-object-info.c
>> ...
>> +int fetch_object_info(const enum protocol_version version, struct object_info_args *args,
>> +		      struct packet_reader *reader, struct object_info *object_info_data,
>> +		      const int stateless_rpc, const int fd_out)
>> +{
>> ...
>> +	for (size_t i = 0; packet_reader_read(reader) == PACKET_READ_NORMAL && i < args->oids->nr; i++) {
>> +		struct string_list object_info_values = STRING_LIST_INIT_DUP;
>> +
>> +		string_list_split(&object_info_values, reader->line, " ", -1);
>> +		if (0 <= size_index) {
>> +			if (!strcmp(object_info_values.items[1 + size_index].string, ""))
>> +				die("object-info: server does not recognize object %s",
>> +				    object_info_values.items[0].string);
>> +
>> +			if (strtoul_ul(object_info_values.items[1 + size_index].string, 10, object_info_data[i].sizep))
>
>
> Overly long lines need to be fixed, by using a shorter and crisper
> variable name in such a short scope, and line wrapping if needed.
>
> More importantly, on this line (wrapped):
>
> 			if (strtoul_ul(object_info_values.items[1 + size_index].string,
> 				       10, object_info_data[i].sizep))
>
> we notice object_info_data[i] is of type "struct object_info", which
> is 
>
>     struct object_info {
>             /* Request */
>             enum object_type *typep;
>             size_t *sizep;
>             off_t *disk_sizep;
> 	    ...
>
> but the last parameter strtoul_ul() takes is unsurprisingly a
> pointer to "unsigned long", not a pointer to "size_t".
>
> Which will break on 32-bit boxes where size_t is "unsigned int"
> that is 32-bit and different from "unsigned long".
>
> Perhaps something along this line?


Not quite.  This "size_t *sizep" has been very recently introduced
by Dscho in a topic that is in-flight.

The ps/cat-file-remote-object-info topic alone does not have this
type-mismatch problem.  Below needs to be addressed as an evil merge
at the integration side, so you have nothing to do.  I'll have to
tweak the merges.

Sorry for a false alarm.



> diff --git a/fetch-object-info.c b/fetch-object-info.c
> index 425929a269..5210e7d954 100644
> --- a/fetch-object-info.c
> +++ b/fetch-object-info.c
> @@ -75,14 +75,17 @@ int fetch_object_info(const enum protocol_version version, struct object_info_ar
>  
>  		string_list_split(&object_info_values, reader->line, " ", -1);
>  		if (0 <= size_index) {
> +			unsigned long sz;
>  			if (!strcmp(object_info_values.items[1 + size_index].string, ""))
>  				die("object-info: server does not recognize object %s",
>  				    object_info_values.items[0].string);
>  
> -			if (strtoul_ul(object_info_values.items[1 + size_index].string, 10, object_info_data[i].sizep))
> +			if (strtoul_ul(object_info_values.items[1 + size_index].string,
> +				       10, &sz))
>  				die("object-info: ref %s has invalid size %s",
>  				    object_info_values.items[0].string,
>  				    object_info_values.items[1 + size_index].string);
> +			*object_info_data[i].sizep = sz;
>  		}
>  
>  		string_list_clear(&object_info_values, 0);