Subject: Re: [RFC/WIP PATCH 07/11] fetch-pack: use the configured transport protocol

From: Jeff King <[email-masked]>
Date: Wed, 27 May 2015 02:53:42 -0400

On Tue, May 26, 2015 at 03:01:11PM -0700, Stefan Beller wrote:

> diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
> index 4a6b340..32dc8b0 100644
> --- a/builtin/fetch-pack.c
> +++ b/builtin/fetch-pack.c
> @@ -127,6 +127,10 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
>  			args.update_shallow = 1;
>  			continue;
>  		}
> +		if (!strcmp("--transport-version", arg)) {
> +			args.version = strtol(arg + strlen("--transport-version"), NULL, 0);
> +			continue;
> +		}

You strcmp() the arg here, so there can't be anything at arg +
strlen(...), can there? Did you want:

  starts_with(arg, "--transports-version=")

here? Or better yet, skip_prefix().

> -	get_remote_heads(fd[0], NULL, 0, &ref, 0, NULL, &shallow);
> +
> +	switch (args.version) {
> +	default:
> +	case 2:
> +		get_remote_capabilities(fd[0], NULL, 0);
> +		request_capabilities(fd[1]);
> +		break;

Should the "default" case be to complain about an unknown version,
rather than fall-through to 2?

-Peff