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

From: Jeff King <peff@peff.net>
Date: May 27, 2015, 06:53 UTC
Message-ID: <20150527065342.GA1756@peff.net>
In-Reply-To: <1432677675-5118-8-git-send-email-sbeller@google.com>

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

Show 12 quoted lines

> 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().

Show 8 quoted lines

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