Re: [PATCH 3/5] Add strbuf_printf() to do formatted printing to a strbuf.
- From
Kristian Høgsberg <krh@redhat.com>
- Date
- Jul 31, 2007, 14:23 UTC
- Message-ID
- <1185891786.11086.40.camel@hinata.boston.redhat.com>
- In-Reply-To
- <7vhcnlgpeo.fsf@assigned-by-dhcp.cox.net>
On Mon, 2007-07-30 at 21:36 -0700, Junio C Hamano wrote:
Show 11 quoted lines
> 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?
Show 8 quoted lines
> > +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