From: Jeff King <peff@peff.net>
Date: Wed, 27 May 2015 06:45:21 GMT
Subject: Re: [RFC/WIP PATCH 06/11] remote.h: add get_remote_capabilities, request_capabilities
Message-ID: <20150527064521.GD885@peff.net>
In-Reply-To: <1432677675-5118-7-git-send-email-sbeller@google.com>

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

> +void get_remote_capabilities(int in, char *src_buf, size_t src_len)
> +{
> +	struct strbuf capabilities_string = STRBUF_INIT;
> +	for (;;) {
> +		int len;
> +		char *line = packet_buffer;
> +		const char *arg;
> +
> +		len = packet_read(in, &src_buf, &src_len,
> +				  packet_buffer, sizeof(packet_buffer),
> +				  PACKET_READ_GENTLE_ON_EOF |
> +				  PACKET_READ_CHOMP_NEWLINE);
> +		if (len < 0)
> +			die_initial_contact(0);
> +
> +		if (!len)
> +			break;
> +
> +		if (len > 4 && skip_prefix(line, "ERR ", &arg))
> +			die("remote error: %s", arg);
> +
> +		if (starts_with(line, "capability:")) {
> +			strbuf_addstr(&capabilities_string, line + strlen("capability:"));
> +			strbuf_addch(&capabilities_string, ' ');
> +		}
> +	}

I think this is the reverse case of next_capabilities in the upload-pack
side, so I'll make the reverse suggestion. :) Would it make things nicer
if both v1 and v2 parsed the capabilities into a string_list?

> +	free(server_capabilities);
> +	server_capabilities = xmalloc(capabilities_string.len + 1);
> +	server_capabilities = strbuf_detach(&capabilities_string, NULL);

Is this xmalloc line left over? The strbuf_detach will write over it.

> +	strbuf_release(&capabilities_string);

No need to release if we just detached.

> +int request_capabilities(int out)
> +{
> +	fprintf(stderr, "request_capabilities\n");

Debug cruft, I presume.

> +	// todo: send our capabilities
> +	packet_write(out, "capability:foo");

No C99 comments. But I think this is just a placeholder. :)

-Peff