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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 29, 2022, 18:06 UTC
Message-ID: <xmqqbkt77q07.fsf@gitster.g>
In-Reply-To: <20220728230210.2952731-6-calvinwan@google.com>

Calvin Wan <calvinwan@google.com> writes:

Show 17 quoted lines

> +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.

Show 10 quoted lines

> @@ -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 "(".