Re: [PATCH 2/3] connect.c: save symref info from server capabilities

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 6, 2013, 17:56 UTC
Message-ID: <xmqqob85ygt8.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20130906155655.GG12966@inner.h.apk.li>

Andreas Krey <a.krey@gmx.de> writes:

Show 43 quoted lines

> Signed-off-by: Andreas Krey <a.krey@gmx.de>
> ---
>  connect.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/connect.c b/connect.c
> index a0783d4..98c4868 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -72,8 +72,8 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
>  	for (;;) {
>  		struct ref *ref;
>  		unsigned char old_sha1[20];
> -		char *name;
> -		int len, name_len;
> +		char *name, *symref;
> +		int len, name_len, symref_len;
>  		char *buffer = packet_buffer;
>  
>  		len = packet_read(in, &src_buf, &src_len,
> @@ -94,9 +94,12 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
>  		name = buffer + 41;
>  
>  		name_len = strlen(name);
> +		symref = 0;
>  		if (len != name_len + 41) {
>  			free(server_capabilities);
>  			server_capabilities = xstrdup(name + name_len + 1);
> +			symref = parse_feature_value(server_capabilities,
> +						     "symref", &symref_len);
>  		}
>  		if (extra_have &&
> @@ -108,6 +111,10 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
>  		if (!check_ref(name, name_len, flags))
>  			continue;
>  		ref = alloc_ref(buffer + 41);
> +		if (symref) {
> +			ref->symref = xcalloc(symref_len + 1, 1);
> +			strncpy(ref->symref, symref, symref_len);
> +		}
>  		hashcpy(ref->old_sha1, old_sha1);
>  		*list = ref;
>  		list = &ref->next;

This looks utterly wrong. HEAD may happen to be the first ref that is advertised and hence capability list typically comes on it, but that does not necessarily have to be the case from the protocol's correctness point of view.

I think this function should do this instead.

    - inside the loop, collect the "symref=..." capabilities;

    - after the loop, look at the "symref=..." capabilities, and
      among the refs the loop added to the *list, find the "HEAD"
      ref and set its ->symref to point at an appropirate ref.