Re: [PATCH GSoC v2 4/6] fetch-object-info: parse type from server response

On Sun Aug 2, 2026 at 4:02 AM CEST, Junio C Hamano wrote:
> Jeff King <[email protected]> writes:
>
>> And I guess that's what started this conversation. The fundamental
>> difference is asking about one object (and using pointers to tell where
>> to put the answer) versus asking about N.
>
> Thanks for framing the trouble I had so cleanly.  Yes.
>
> The origin of the pointer pattern you mentioned, 9a49059022
> (sha1_object_info_extended(): expose a bit more info, 2011-05-12),
> designed the object_info structure to be passed as a set of extended
> parameters to sha1_object_info_extended().
>
> Instead of passing 'size_t *size_p' (which can be NULL) as a
> parameter to signal that (1) if NULL we are not interested in the
> value, and (2) if not NULL, that is where you are expected to write
> the answer, and having to keep adding such a pointer parameter
> every time we need to optionally ask the function for a different
> aspect of the object, it defined the function to take an object_info
> structure to allow us to add new members to the struct as the set
> of queries grows without having to change the function signature.
>
> As a set of extended parameters, of course, it was natural for the
> caller's variables that receive the answers to be pointed to by
> members in the struct.  So the pointers in the struct are
> justifiable, but strictly as parameters to the function.
>
> The troubling thing I saw in the patch (and I suspect it was not a
> problem introduced in this series, but by earlier changes that added
> other kinds of fields) is exactly as you identified.
>
> The pointers in this struct were meant to point at real variables or
> structure members that receive values from the function, and were
> never meant to be the final structure that receives and retains
> returned values.  If we need 5 calls to the function, we either:
>
>  (1) Have a single object_info, and a set of local variables that
>      are pointed at by the members of the object_info structure, and
>      have a loop that runs 5 times where each iteration calls the
>      function to store the returned values in local variables and
>      consumes them, i.e.
>
>         struct oid oid[5];
>         struct object_info oi;
>         for (int i =3D 0; i < 5; i++) {
>                 size_t size;
>                 enum object_type type;
>                 oi.size_p =3D &size;
>                 oi.type_p =3D &type;
>                 object_info_extended(oid[i], &oi);
>                 ... use 'size' and 'type' here ...
>         }
>
>      if you can consume and forget about the object in each
>      iteration, or
>
>  (2) Have a single object_info, and 5 sets of local variables.  A
>      loop runs 5 times; in the nth iteration of the loop,
>      object_info points at the nth set of local variables and the
>      function is called.  After the loop runs, we have 5 sets of
>      local variables populated and we use them, i.e.
>
>         struct oid oid[5];
>         struct { size_t size; enum object_type type; } trait[5];
>         struct object_info oi;
>         for (int i =3D 0; i < 5; i++) {
>                 oi.size_p =3D &trait[i].size;
>                 oi.type_p =3D &trait[i].type;
>                 object_info_extended(oid[i], &oi);
>         }
>         ... now you have 'size' and 'type' for all these 5 objects ...
>
>      if you have to return all 5 results to your caller.
>
> In either case, you do not need more than one object_info
> structure.  Having an array of object_info structures was what
> looked so weird to me.
>
>
> Thanks.

Thanks, I think I got it.

I have the doubt of whether this change is desired for this series as
prep or if I should keep on and later make a cleanup series as this
doesn't make a change for a user.

What I understood is that fetch_object_info shouldn't use object_info to
store the results, because it doesn't call read_object_info() like other
commands like 'info' do. Then, it should use its own data structure to
hold the results with flags like wants_size and wants_type. Something
like:

	struct object_info_results {
		enum object_type *types;
		size_t *sizes;
		unsigned *unrecognized;
		size_t nr;
		unsigned wants_size:1;
		unsigned wants_type:1;
	};

All three of the pointers are nr long.
This could be done in two patches, as I was going to do a prep to
prepare the current code (size only) and this patch would add type for
fetch_object_info().

At the start I read this:

> It could be something we may want to
> clean-up much later after all the dust settles from this year's
> GSoC.  I dunno.

So I'm a bit lost about what to do, I'm happy to make that in this
series or as a cleanup series later after GSoC which ends in a couple
weeks.

Whatever is preferred.

Thanks,
Pablo