From: Junio C Hamano <gitster@pobox.com>
Date: Sun, 18 Nov 2007 04:47:11 GMT
Subject: Re: [PATCH 3/3] send-pack: assign remote errors to each ref
Message-ID: <7vwssgxir4.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20071118023942.GA4560@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Sat, Nov 17, 2007 at 05:03:57PM -0800, Junio C Hamano wrote:
>
>> > +	for (ref = refs; ref; ref = ref->next) {
>> > +		const char *msg;
>> > +		if (prefixcmp(line, ref->name))
>> > +			continue;
>> 
>> It probably would not matter for sane repositories, but with
>> thousands of refs, strlen() and prefixcmp() may start to hurt:
>
> It is actually _just_ prefixcmp. Or do you mean the strlen we call in
> prefixcmp? If so, I think the right solution is to make prefixcmp
> faster.  :)

I was referring to strlen(ref->name) taken for all refs during
the loop.  Micro-optimized one finds the end of refname on the
"ng" line once before entering the loop.

> OK. Since it is already in next, do you want a style fixup patch?

I do not think it is particularly a big deal -- perhaps clean it
before we touch the vicinity of the code next time.  The same
goes for the "} else {" stuff.

>> Before receive_status() is called, can the refs already have the
>> error status and string set?
>
> Nothing else sets the string, so the latter is not possible (perhaps it
> should be "remote_error" for clarity). It is less clear that we are not
> ... if you look at do_send_pack, we
> only actually send the remote refs that are getting REF_STATUS_OK.

Ah, Ok.