Subject: Re: [PATCH v5 5/6] transport: add client support for object-info

From: Junio C Hamano <[email-masked]>
Date: Fri, 29 Jul 2022 11:06:16 -0700

Calvin Wan <[email-masked]> writes:

> +void send_object_info_request(int fd_out, struct object_info_args *args)
> +{
> +	struct strbuf req_buf = STRBUF_INIT;
> +	size_t i;
> +
> +	write_command_and_capabilities(&req_buf, args->server_options, "object-info");
> +
> +	if (unsorted_string_list_has_string(args->object_info_options, "size"))
> +		packet_buf_write(&req_buf, "size");
> +
> +	if (unsorted_string_list_has_string(args->object_info_options, "type"))
> +		packet_buf_write(&req_buf, "type");
> +
> +	if (args->oids) {
> +		for (i = 0; i < args->oids->nr; i++)
> +			packet_buf_write(&req_buf, "oid %s", oid_to_hex(&args->oids->oid[i]));
> +	}

If !args->oids then we say "we want to request object-info to learn
size and type for the following objects: oh, there are no objects we
are interested in".  I wonder if an early return

	if (!args->oids)
		return;

at the beginning of the function that turns it into a benign no-op,
may make more sense?  Calling "send_X()" helper and seeing nothing
come out on the wire might make it look awkward, though.

> @@ -363,10 +437,12 @@ static int fetch_refs_via_pack(struct transport *transport,
>  			       int nr_heads, struct ref **to_fetch)
>  {
>  	int ret = 0;
> +	size_t i;
>  	struct git_transport_data *data = transport->data;
>  	struct ref *refs = NULL;
>  	struct fetch_pack_args args;
>  	struct ref *refs_tmp = NULL;
> +	struct ref *object_info_refs = xcalloc(1, sizeof (struct ref));

Style: no SP between "sizeof" and "(".