From: Andreas Krey <a.krey@gmx.de>
Date: Fri, 06 Sep 2013 19:25:15 GMT
Subject: Re: [PATCH 2/3] connect.c: save symref info from server capabilities
Message-ID: <20130906192515.GI12966@inner.h.apk.li>
In-Reply-To: <xmqqob85ygt8.fsf@gitster.dls.corp.google.com>

On Fri, 06 Sep 2013 10:56:51 +0000, Junio C Hamano wrote:
> Andreas Krey <a.krey@gmx.de> writes:
> 
...
> > +		if (symref) {
> > +			ref->symref = xcalloc(symref_len + 1, 1);
> > +			strncpy(ref->symref, symref, symref_len);
> > +		}
...
> 
> 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.

Ok, then I misunderstood that part. I thought we'd be going to
put the symref (if any) into 'capabilities' on the respective ref,
but putting them all in one capability list sounds saner all in all.

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

Fair game. There goes the parse_feature_value; will have to iterate
another way (or look them ("symref=#{name}:") up instead of collecting
them into a hash beforehand).

Can I assume that the only capability list is always on the
first ref sent (as it is now)?

(And besides, is there something more suitable for the 
 xcalloc/strncpy combination?)

Andreas

-- 
"Totally trivial. Famous last words."
From: Linus Torvalds <torvalds@*.org>
Date: Fri, 22 Jan 2010 07:29:21 -0800