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

> Jeff King <peff@peff.net> 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 = 0; i < 5; i++) {
>                 size_t size;
>                 enum object_type type;
>                 oi.size_p = &size;
>                 oi.type_p = &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 = 0; i < 5; i++) {
>                 oi.size_p = &trait[i].size;
>                 oi.type_p = &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.