Re: [PATCH 4/4] fetch-pack: mention server version with verbose output

[Jeff King <peff@peff.net>]

On Mon, Aug 13, 2012 at 02:09:32PM -0700, Junio C Hamano wrote:

> >> +	if ((agent_feature = server_feature("agent", &agent_len)) != NULL &&
> >> +	    5 < agent_len && agent_feature[5] == '=') {
> >>  		agent_supported = 1;
> >> +		if (args.verbose) {
> >> +			fprintf(stderr, "Server version is %.*s\n",
> >> +				agent_len - 6, agent_feature + 6);
> >> +		}
> >> +	}
> >
> > Yeah, this is exactly the kind of ugliness I was trying to avoid with my
> > allocating wrapper. Still, there is only one call site, so I do not care
> > overly much (and I as I've already said, I'm lukewarm on the final two
> > patches, anyway).
> 
> Actually, the above is vastly superiour compared to the allocating
> kind.  Be honest and think about it.  If the caller wants to
> allocate, it could, and it does not even have to count.  If the
> caller does not want to allocate, it does not have to pay the price.

My point is that the run-time allocation price is quite small, but the
readability cost of that ugly conditional with the magic "5" is
non-trivial. But they are apples and oranges, so it is hard to compare
their amounts directly.

-Peff