On Wed, Sep 19, 2007 at 12:46:04PM +0000, Edgar Toernig wrote:
> Pierre Habouzit wrote:
> >
> > +void strbuf_addvf(struct strbuf *sb, const char *fmt, va_list ap)
> > +{
> > +	int len;
> > +
> > +	len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
> > +	if (len < 0) {
> > +		len = 0;
> > +	}
> > +	if (len > strbuf_avail(sb)) {
> > +		strbuf_grow(sb, len);
> > +		len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
> > +		if (len > strbuf_avail(sb)) {
> > +			die("this should not happen, your snprintf is broken");
> > +		}
> > +	}
> > +	strbuf_setlen(sb, sb->len + len);
> > +}
> 
> The second vsnprintf won't work as the first one consumed all args
> from va_list ap.  You need to va_copy the ap.  But iirc va_copy poses
> compatibility issues.  Unless va_copy is made available somehow,
> I would suggest to let the caller know that the buffer was too small
> (but isn't any more) and it has to call the function again:

  That's what I thought, and then nfvasprintf in trace.c suffers from
the same issue, as I copied the code from there.

> 	do {
> 		va_start(ap, fmt);
> 		again = strbuf_addvf(sb, fmt, ap);
> 		va_end(ap);
> 	} while (again);

  in fact doing it twice is enough but either way I don't like to impose
that to the caller :/ I mean it's totally stupid to have to do that on a
strbuf. of course we could provide a macro doing that ...
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org