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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html