Re: [PATCH v2 3/7] transport: extract common fetch_pack() call

From: Junio C Hamano <gitster@pobox.com>
Date: May 18, 2020, 18:40 UTC
Message-ID: <xmqqpnb12iit.fsf@gitster.c.googlers.com>
In-Reply-To: <3a42575bd5d124b6b2e536b1511107ebf5ec1091.1589816719.git.liu.denton@gmail.com>

Denton Liu <liu.denton@gmail.com> writes:

Show 5 quoted lines

> In the switch statement, the difference between the `protocol_v2` and
> `protocol_v{1,0}` arms is a preparatory call to die_if_server_options() in
> the latter. The fetch_pack() call is identical in both arms. However,
> since this fetch_pack() call has so many parameters, it is not
> immediately obvious that the call is identical in both cases.

Sure.

	if (data->version < protocol_v2)
		die_if_server_options(transport);
	else if (data->version == protocol_unknown_version)
		BUG("...");

	refs = fetch_pack(...);

might have been even easier to follow, but that's OK.

Show 26 quoted lines

>  	switch (data->version) {
>  	case protocol_v2:
> -		refs = fetch_pack(&args, data->fd,
> -				  refs_tmp ? refs_tmp : transport->remote_refs,
> -				  to_fetch, nr_heads, &data->shallow,
> -				  &transport->pack_lockfile, data->version);
> +		/* do nothing */
>  		break;
>  	case protocol_v1:
>  	case protocol_v0:
>  		die_if_server_options(transport);
> -		refs = fetch_pack(&args, data->fd,
> -				  refs_tmp ? refs_tmp : transport->remote_refs,
> -				  to_fetch, nr_heads, &data->shallow,
> -				  &transport->pack_lockfile, data->version);
>  		break;
>  	case protocol_unknown_version:
>  		BUG("unknown protocol version");
>  	}
> +	refs = fetch_pack(&args, data->fd,
> +			  refs_tmp ? refs_tmp : transport->remote_refs,
> +			  to_fetch, nr_heads, &data->shallow,
> +			  &transport->pack_lockfile, data->version);
>  
>  	close(data->fd[0]);
>  	close(data->fd[1]);