Re: [PATCH GSoC v3 4/8] fetch-object-info: use dedicated struct for the results

[Junio C Hamano <gitster@pobox.com>]

Pablo Sabater <pabloosabaterr@gmail.com> writes:

> fetch_object_info() collects information about N objects, but it stores
> the results in an array of object_info. That struct holds the extended
> parameters of read_object_info() (The optional outputs the caller wants
> filled). Its pointers tell that function where to write the answers for
> a single object. object_info is not meant to be the final storage, and
> since fetch_object_info() does not call read_object_info(), there is no
> reason to use it. Using it means allocating one scalar per object per
> attribute just to have those pointers somewhere to point at.
>
> Add struct fetch_object_info_results. The caller sets the wants_* flags
> to say what it is interested in, and fetch_object_info() allocates one
> array per attribute. A set wants_* flag means "asked for", while a
> non-NULL array means "available". The caller releases the arrays with
> free_fetch_object_info_results().
>
> The object_info_options string list is no longer needed. Filtering
> against the server's advertisement now sets local ask_* flags, and
> send_object_info_request() turns those into the v2 protocol option
> strings. remote_atom_map[] existed only to map those strings back into
> atom names, so drop it and build remote_allowed_atoms from the result
> arrays.
>
> free_object_info_contents() loses its only caller and is dropped.
>
> Helped-by: Jeff King <peff@peff.net>
> Helped-by: Junio C Hamano <gitster@pobox.com>
> 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  | 59 +++++++++--------------------------
>  fetch-object-info.c | 90 ++++++++++++++++++++++++++++-------------------------
>  fetch-object-info.h | 28 ++++++++++++-----
>  object-file.c       | 10 ------
>  odb.h               |  3 --
>  transport.c         |  3 +-
>  transport.h         |  5 +--
>  7 files changed, 88 insertions(+), 110 deletions(-)

The direction this step wants to take us looks good, but at this
point we only support "size" and the client side starts parsing
"type" only in [6/8], while the server side starts advertising
"type" only in [7/8].  If the software at this step talks to a newer
server that does support "type", it will hit BUG() if the user
requests %(objecttype), no?  IOW, introduction of "ask_type" smells
a bit premature.