Re: [PATCH 3/3] send-pack: assign remote errors to each ref

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 18, 2007, 04:47 UTC
Message-ID: <7vwssgxir4.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20071118023942.GA4560@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 13 quoted lines

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

Show 7 quoted lines

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