On Mon, 2007-07-30 at 21:36 -0700, Junio C Hamano wrote:
quoted text
> Kristian Høgsberg <krh@redhat.com> writes:
> 
> > +static void inline strbuf_grow(struct strbuf *sb, size_t extra)
> > +{
> > +	while (sb->alloc < sb->len + extra)
> >  		sb->alloc = sb->alloc * 3 / 2 + 16;
> > +	sb->buf = xrealloc(sb->buf, sb->alloc);
> > +}
> 
> Somehow this while () loop to compute the growth factor bothers
> me but that is probably a minor detail.

Think of it as a more efficient way of adding one character at a time :)
And it's logarithmic in the number of extra bytes.  By the way, I
normally just double the size in these cases, which gives you amortized
linear performance for adding to the buffer.  What's behind the * 3 / 2
idea?

quoted text
> > +void strbuf_printf(struct strbuf *sb, const char *fmt, ...)
> > +{
> > +	char one_line[2048];
> > +	va_list args;
> > +	int len;
> 
> Such a nice abstraction so far, and then at the highest level of
> callchain we have this hardcoded limit?

Yeah, I know, it sucks.  I'd like to just run vsnprintf with a 0-sized
buffer to get the length, and then grow the buffer by that much, but
that's not very portable as far as I know.  Another approach is to just
vsnprintf into the one_line buffer and copy it into the strbuf if it
doesn't overflow.  If it does overflow, grow the buffer with that amount
and vsnprintf into the buffer.  I don't think that's portable either,
since vsnprintf can't be relied upon to return the number of characters
it would have printed.  One thing that would work, but is much more
involved is to lift the vsnprintf implementation from something like
glib.

So, I dunno... given the options, I opted for a simple solution with
some limitations.  Given how many other functions in git work on a fixed
sized static buffer (for example, the fmt_ident case mentioned in
another email), I wouldn't think this is a problem

Kristian


-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html