From: Edgar Toernig <froese@gmx.de>
Date: Wed, 19 Sep 2007 12:46:04 GMT
Subject: Re: [PATCH 1/5] strbuf API additions and enhancements.
Message-ID: <20070919144604.7deca4f7.froese@gmx.de>
In-Reply-To: <20070918224119.17650344AB3@madism.org>

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:

int 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)
		return 0;
	if (len > strbuf_avail(sb)) {
		strbuf_grow(sb, len);
		return -1;
	}
	strbuf_setlen(sb, sb->len + len);
	return 0;
}

The caller:

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

va_copy would be nicer though...

Ciao, ET.