Re: [PATCH] Change "refs/" references to symbolic constants

From: Andreas Ericsson <ae@op5.se>
Date: Oct 3, 2007, 11:30 UTC
Message-ID: <47037D44.8040404@op5.se>
In-Reply-To: <Pine.LNX.4.64.0710030503520.28395@racer.site>

Johannes Schindelin wrote:

Show 22 quoted lines

> Hi,
> 
> On Tue, 2 Oct 2007, Jeff King wrote:
> 
>> On Tue, Oct 02, 2007 at 05:22:23PM -0700, Junio C Hamano wrote:
>>
>>>>   strbuf_init(&url);
>>>>   strbuf_addf(&url, "%s/objects/pack/pack-%s.idx", repo->base, hex);
>>> Ugh, this typically calls snprintf() twice doesn't it?
>> Yes, it probably does. However, I think it is considerably easier to
>> read and more maintainable. Are you "ugh"ing because of the performance
>> impact (which should be negligible unless this is in a tight loop) or
>> because of the portability problems associated with va_copy?
> 
> I wonder, I wonder, if
> 
> 	strbuf_addstr(&url, repo->base);
> 	strbuf_addstr(&url, "/objects/pack/pack-");
> 	strbuf_addstr(&url, hex);
> 	strbuf_addstr(&url, ".idx");
> 
> would make anybody else but me happy...

strbuf_addstr_many(&url, repo->base, "/objects/pack/pack-", hex, ".idx", NULL);

is what I'd prefer. It's not overly complicated, requires no *printf(), and doesn't introduce any new portability issues (va_arg() is C89).

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231