strbuf: add strbuf_vaddf

In a variable-args function, the code for writing into a strbuf is
non-trivial. We ended up cutting and pasting it in several places
because there was no vprintf-style function for strbufs (which in turn
was held up by a lack of va_copy).

Now that we have a fallback va_copy, we can add strbuf_vaddf, the
strbuf equivalent of vsprintf. And we can clean up the cut and paste
mess.

Signed-off-by: Jeff King <peff@peff.net>
Improved-by: Christian Couder <christian.couder@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/fsck.c b/fsck.c
index 3d05d4a794a4158a421ce1265422d241e28a5278..6f266c1ea4b8df503c211170741fa5b490a6a02a 100644 (file)
--- a/fsck.c
+++ b/fsck.c
@@ -347,26 +347,14 @@ int fsck_object(struct object *obj, int strict, fsck_error error_func)
 int fsck_error_function(struct object *obj, int type, const char *fmt, ...)
 {
        va_list ap;
 int fsck_error_function(struct object *obj, int type, const char *fmt, ...)
 {
        va_list ap;

-       int len;

        struct strbuf sb = STRBUF_INIT;
 
        strbuf_addf(&sb, "object %s:", obj->sha1?sha1_to_hex(obj->sha1):"(null)");
 
        va_start(ap, fmt);
        struct strbuf sb = STRBUF_INIT;
 
        strbuf_addf(&sb, "object %s:", obj->sha1?sha1_to_hex(obj->sha1):"(null)");
 
        va_start(ap, fmt);

-       len = vsnprintf(sb.buf + sb.len, strbuf_avail(&sb), fmt, ap);
+       strbuf_vaddf(&sb, fmt, ap);

        va_end(ap);
 
        va_end(ap);
 

-       if (len < 0)
-               len = 0;
-       if (len >= strbuf_avail(&sb)) {
-               strbuf_grow(&sb, len + 2);
-               va_start(ap, fmt);
-               len = vsnprintf(sb.buf + sb.len, strbuf_avail(&sb), fmt, ap);
-               va_end(ap);
-               if (len >= strbuf_avail(&sb))
-                       die("this should not happen, your snprintf is broken");
-       }
-

        error("%s", sb.buf);
        strbuf_release(&sb);
        return 1;
        error("%s", sb.buf);
        strbuf_release(&sb);
        return 1;

diff --git a/merge-recursive.c b/merge-recursive.c
index 16c2dbeab95a0f4099e3de8badf688bb4dee8189..2a4f739365e6ec72372b32ddb486aee6b21f4c22 100644 (file)
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -137,7 +137,6 @@ static void flush_output(struct merge_options *o)
 __attribute__((format (printf, 3, 4)))
 static void output(struct merge_options *o, int v, const char *fmt, ...)
 {
 __attribute__((format (printf, 3, 4)))
 static void output(struct merge_options *o, int v, const char *fmt, ...)
 {

-       int len;

        va_list ap;
 
        if (!show(o, v))
        va_list ap;
 
        if (!show(o, v))


@@ -148,21 +147,9 @@ static void output(struct merge_options *o, int v, const char *fmt, ...)
        strbuf_setlen(&o->obuf, o->obuf.len + o->call_depth * 2);
 
        va_start(ap, fmt);
        strbuf_setlen(&o->obuf, o->obuf.len + o->call_depth * 2);
 
        va_start(ap, fmt);

-       len = vsnprintf(o->obuf.buf + o->obuf.len, strbuf_avail(&o->obuf), fmt, ap);
+       strbuf_vaddf(&o->obuf, fmt, ap);

        va_end(ap);
 
        va_end(ap);
 

-       if (len < 0)
-               len = 0;
-       if (len >= strbuf_avail(&o->obuf)) {
-               strbuf_grow(&o->obuf, len + 2);
-               va_start(ap, fmt);
-               len = vsnprintf(o->obuf.buf + o->obuf.len, strbuf_avail(&o->obuf), fmt, ap);
-               va_end(ap);
-               if (len >= strbuf_avail(&o->obuf)) {
-                       die("this should not happen, your snprintf is broken");
-               }
-       }
-       strbuf_setlen(&o->obuf, o->obuf.len + len);

        strbuf_add(&o->obuf, "\n", 1);
        if (!o->buffer_output)
                flush_output(o);
        strbuf_add(&o->obuf, "\n", 1);
        if (!o->buffer_output)
                flush_output(o);

diff --git a/strbuf.c b/strbuf.c
index 07e8883ceb297bdde79f06ffa2d0519581ee60b9..77444a94df3d4a0cda6403957fd13ea262d3ab24 100644 (file)
--- a/strbuf.c
+++ b/strbuf.c
@@ -195,24 +195,29 @@ void strbuf_adddup(struct strbuf *sb, size_t pos, size_t len)
 
 void strbuf_addf(struct strbuf *sb, const char *fmt, ...)
 {
 
 void strbuf_addf(struct strbuf *sb, const char *fmt, ...)
 {

-       int len;

        va_list ap;
        va_list ap;

+       va_start(ap, fmt);
+       strbuf_vaddf(sb, fmt, ap);
+       va_end(ap);
+}
+
+void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap)
+{
+       int len;
+       va_list cp;

 
        if (!strbuf_avail(sb))
                strbuf_grow(sb, 64);
 
        if (!strbuf_avail(sb))
                strbuf_grow(sb, 64);

-       va_start(ap, fmt);
-       len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
-       va_end(ap);
+       va_copy(cp, ap);
+       len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, cp);
+       va_end(cp);

        if (len < 0)
        if (len < 0)

-               die("your vsnprintf is broken");
+               die("BUG: your vsnprintf is broken (returned %d)", len);

        if (len > strbuf_avail(sb)) {
                strbuf_grow(sb, len);
        if (len > strbuf_avail(sb)) {
                strbuf_grow(sb, len);

-               va_start(ap, fmt);

                len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
                len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);

-               va_end(ap);
-               if (len > strbuf_avail(sb)) {
-                       die("this should not happen, your snprintf is broken");
-               }
+               if (len > strbuf_avail(sb))
+                       die("BUG: your vsnprintf is broken (insatiable)");

        }
        strbuf_setlen(sb, sb->len + len);
 }
        }
        strbuf_setlen(sb, sb->len + len);
 }

diff --git a/strbuf.h b/strbuf.h
index 675a91f93821421aa79de51857b22a4960da8ec1..f722331470065f448197ea461ef3af06d3623ff2 100644 (file)
--- a/strbuf.h
+++ b/strbuf.h
@@ -120,6 +120,8 @@ extern void strbuf_addbuf_percentquote(struct strbuf *dst, const struct strbuf *
 
 __attribute__((format (printf,2,3)))
 extern void strbuf_addf(struct strbuf *sb, const char *fmt, ...);
 
 __attribute__((format (printf,2,3)))
 extern void strbuf_addf(struct strbuf *sb, const char *fmt, ...);

+__attribute__((format (printf,2,0)))
+extern void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap);

 
 extern size_t strbuf_fread(struct strbuf *, size_t, FILE *);
 /* XXX: if read fails, any partial read is undone */
 
 extern size_t strbuf_fread(struct strbuf *, size_t, FILE *);
 /* XXX: if read fails, any partial read is undone */

diff --git a/trace.c b/trace.c
index 35d388dce44a4a8e3d6053dfd6abcb652771818a..eda3f6d721dc3c70a858eaa247a3100111324fb4 100644 (file)
--- a/trace.c
+++ b/trace.c
@@ -64,28 +64,18 @@ static const char err_msg[] = "Could not trace into fd given by "
 
 void trace_printf(const char *fmt, ...)
 {
 
 void trace_printf(const char *fmt, ...)
 {

-       struct strbuf buf;
+       struct strbuf buf = STRBUF_INIT;

        va_list ap;
        va_list ap;

-       int fd, len, need_close = 0;
+       int fd, need_close = 0;

 
        fd = get_trace_fd(&need_close);
        if (!fd)
                return;
 
        set_try_to_free_routine(NULL);  /* is never reset */
 
        fd = get_trace_fd(&need_close);
        if (!fd)
                return;
 
        set_try_to_free_routine(NULL);  /* is never reset */

-       strbuf_init(&buf, 64);

        va_start(ap, fmt);
        va_start(ap, fmt);

-       len = vsnprintf(buf.buf, strbuf_avail(&buf), fmt, ap);
+       strbuf_vaddf(&buf, fmt, ap);

        va_end(ap);
        va_end(ap);

-       if (len >= strbuf_avail(&buf)) {
-               strbuf_grow(&buf, len - strbuf_avail(&buf) + 128);
-               va_start(ap, fmt);
-               len = vsnprintf(buf.buf, strbuf_avail(&buf), fmt, ap);
-               va_end(ap);
-               if (len >= strbuf_avail(&buf))
-                       die("broken vsnprintf");
-       }
-       strbuf_setlen(&buf, len);

 
        write_or_whine_pipe(fd, buf.buf, buf.len, err_msg);
        strbuf_release(&buf);
 
        write_or_whine_pipe(fd, buf.buf, buf.len, err_msg);
        strbuf_release(&buf);


@@ -96,28 +86,18 @@ void trace_printf(const char *fmt, ...)
 
 void trace_argv_printf(const char **argv, const char *fmt, ...)
 {
 
 void trace_argv_printf(const char **argv, const char *fmt, ...)
 {

-       struct strbuf buf;
+       struct strbuf buf = STRBUF_INIT;

        va_list ap;
        va_list ap;

-       int fd, len, need_close = 0;
+       int fd, need_close = 0;

 
        fd = get_trace_fd(&need_close);
        if (!fd)
                return;
 
        set_try_to_free_routine(NULL);  /* is never reset */
 
        fd = get_trace_fd(&need_close);
        if (!fd)
                return;
 
        set_try_to_free_routine(NULL);  /* is never reset */

-       strbuf_init(&buf, 64);

        va_start(ap, fmt);
        va_start(ap, fmt);

-       len = vsnprintf(buf.buf, strbuf_avail(&buf), fmt, ap);
+       strbuf_vaddf(&buf, fmt, ap);

        va_end(ap);
        va_end(ap);

-       if (len >= strbuf_avail(&buf)) {
-               strbuf_grow(&buf, len - strbuf_avail(&buf) + 128);
-               va_start(ap, fmt);
-               len = vsnprintf(buf.buf, strbuf_avail(&buf), fmt, ap);
-               va_end(ap);
-               if (len >= strbuf_avail(&buf))
-                       die("broken vsnprintf");
-       }
-       strbuf_setlen(&buf, len);

 
        sq_quote_argv(&buf, argv, 0);
        strbuf_addch(&buf, '\n');
 
        sq_quote_argv(&buf, argv, 0);
        strbuf_addch(&buf, '\n');