Re: [RFCv2 10/16] transport: connect_setup appends protocol version number

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 2, 2015, 21:37 UTC
Message-ID: <xmqq8uc123n5.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1433203338-27493-11-git-send-email-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

Show 19 quoted lines

> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>
> Notes:
>     name it to_free
>
>  transport.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/transport.c b/transport.c
> index 651f0ac..b49fc60 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -496,15 +496,28 @@ static int set_git_option(struct git_transport_options *opts,
>  static int connect_setup(struct transport *transport, int for_push, int verbose)
>  {
>  	struct git_transport_data *data = transport->data;
> +	const char *remote_program;
> +	char *to_free = 0;

	char *to_free = NULL;

Show 9 quoted lines

> +	remote_program = (for_push ? data->options.receivepack
> +				   : data->options.uploadpack);
> +
> +	if (transport->smart_options->transport_version >= 2) {
> +		to_free = xmalloc(strlen(remote_program) + 12);
> +		sprintf(to_free, "%s-%d", remote_program,
> +			transport->smart_options->transport_version);
> +		remote_program = to_free;
> +	}

Hmph, so everybody else thinks it is interacting with 'upload-pack', and this is the only function that knows it is actually talking with 'upload-pack-2'?

I am wondering why there isn't a separate helper function that munges data->options.{uploadpack,receivepack} fields based on the value of transport_version that is called _before_ this function is called.

Also, how does this interact with the name of the program the end user can specify via "fetch --upload-pack=<program name>" option?

Show 10 quoted lines

>  	data->conn = git_connect(data->fd, transport->url,
> -				 for_push ? data->options.receivepack :
> -				 data->options.uploadpack,
> +				 remote_program,
>  				 verbose ? CONNECT_VERBOSE : 0);
>  
> +	free(to_free);
> +
>  	return 0;
>  }