nfv?asprintf are broken without va_copy, workaround them.

* drop nfasprintf.
* move nfvasprintf into imap-send.c back, and let it work on a 8k buffer,
and die() in case of overflow. It should be enough for imap commands, if
someone cares about imap-send, he's welcomed to fix it properly.
* replace nfvasprintf use in merge-recursive with a copy of the strbuf_addf
logic, it's one place, we'll live with it.
To ease the change, output_buffer string list is replaced with a strbuf ;)
* rework trace.c to call vsnprintf itself.  It's used to format strerror()s
and git command names, it should never be more than a few octets long, let
it work on a 8k static buffer with vsnprintf or die loudly.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>

diff --git a/cache.h b/cache.h
index 8650d62334323898f5173dcfa5af463c18e40624..916ee5155bc0cb8f35c81f1c2809bb30162ebb83 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -585,8 +585,6 @@ extern void *alloc_object_node(void);
 extern void alloc_report(void);
 
 /* trace.c */
 extern void alloc_report(void);
 
 /* trace.c */

-extern int nfasprintf(char **str, const char *fmt, ...);
-extern int nfvasprintf(char **str, const char *fmt, va_list va);

 extern void trace_printf(const char *format, ...);
 extern void trace_argv_printf(const char **argv, int count, const char *format, ...);
 
 extern void trace_printf(const char *format, ...);
 extern void trace_argv_printf(const char **argv, int count, const char *format, ...);
 

diff --git a/imap-send.c b/imap-send.c
index 905d09746aa36f18d873605518fbeaca25d17f73..e95cdde062b0741fa8c73ddc87ada2fbfab8921f 100644 (file)
--- a/imap-send.c
+++ b/imap-send.c
@@ -105,6 +105,19 @@ static void free_generic_messages( message_t * );
 
 static int nfsnprintf( char *buf, int blen, const char *fmt, ... );
 
 
 static int nfsnprintf( char *buf, int blen, const char *fmt, ... );
 

+static int nfvasprintf(char **strp, const char *fmt, va_list ap)
+{
+       int len;
+       char tmp[8192];
+
+       len = vsnprintf(tmp, sizeof(tmp), fmt, ap);
+       if (len < 0)
+               die("Fatal: Out of memory\n");
+       if (len >= sizeof(tmp))
+               die("imap command overflow !\n");
+       *strp = xmemdupz(tmp, len);
+       return len;
+}

 
 static void arc4_init( void );
 static unsigned char arc4_getbyte( void );
 
 static void arc4_init( void );
 static unsigned char arc4_getbyte( void );

diff --git a/merge-recursive.c b/merge-recursive.c
index 14b56c2f20651711727169b6d1f28df66bfe775d..86767e6e8a37ce7874aa80b789cfef780fa9b136 100644 (file)
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -85,63 +85,58 @@ struct stage_data
        unsigned processed:1;
 };
 
        unsigned processed:1;
 };
 

-struct output_buffer
-{
-       struct output_buffer *next;
-       char *str;
-};
-

 static struct path_list current_file_set = {NULL, 0, 0, 1};
 static struct path_list current_directory_set = {NULL, 0, 0, 1};
 
 static int call_depth = 0;
 static int verbosity = 2;
 static int buffer_output = 1;
 static struct path_list current_file_set = {NULL, 0, 0, 1};
 static struct path_list current_directory_set = {NULL, 0, 0, 1};
 
 static int call_depth = 0;
 static int verbosity = 2;
 static int buffer_output = 1;

-static struct output_buffer *output_list, *output_end;
+static struct strbuf obuf = STRBUF_INIT;

 
 

-static int show (int v)
+static int show(int v)

 {
        return (!call_depth && verbosity >= v) || verbosity >= 5;
 }
 
 {
        return (!call_depth && verbosity >= v) || verbosity >= 5;
 }
 

-static void output(int v, const char *fmt, ...)
+static void flush_output(void)

 {
 {

-       va_list args;
-       va_start(args, fmt);
-       if (buffer_output && show(v)) {
-               struct output_buffer *b = xmalloc(sizeof(*b));
-               nfvasprintf(&b->str, fmt, args);
-               b->next = NULL;
-               if (output_end)
-                       output_end->next = b;
-               else
-                       output_list = b;
-               output_end = b;
-       } else if (show(v)) {
-               int i;
-               for (i = call_depth; i--;)
-                       fputs("  ", stdout);
-               vfprintf(stdout, fmt, args);
-               fputc('\n', stdout);
+       if (obuf.len) {
+               fputs(obuf.buf, stdout);
+               strbuf_reset(&obuf);

        }
        }

-       va_end(args);

 }
 
 }
 

-static void flush_output(void)
+static void output(int v, const char *fmt, ...)

 {
 {

-       struct output_buffer *b, *n;
-       for (b = output_list; b; b = n) {
-               int i;
-               for (i = call_depth; i--;)
-                       fputs("  ", stdout);
-               fputs(b->str, stdout);
-               fputc('\n', stdout);
-               n = b->next;
-               free(b->str);
-               free(b);
+       int len;
+       va_list ap;
+
+       if (!show(v))
+               return;
+
+       strbuf_grow(&obuf, call_depth * 2 + 2);
+       memset(obuf.buf + obuf.len, ' ', call_depth * 2);
+       strbuf_setlen(&obuf, obuf.len + call_depth * 2);
+
+       va_start(ap, fmt);
+       len = vsnprintf(obuf.buf + obuf.len, strbuf_avail(&obuf), fmt, ap);
+       va_end(ap);
+
+       if (len < 0)
+               len = 0;
+       if (len >= strbuf_avail(&obuf)) {
+               strbuf_grow(&obuf, len + 2);
+               va_start(ap, fmt);
+               len = vsnprintf(obuf.buf + obuf.len, strbuf_avail(&obuf), fmt, ap);
+               va_end(ap);
+               if (len >= strbuf_avail(&obuf)) {
+                       die("this should not happen, your snprintf is broken");
+               }

        }
        }

-       output_list = NULL;
-       output_end = NULL;
+       strbuf_setlen(&obuf, obuf.len + len);
+       strbuf_add(&obuf, "\n", 1);
+       if (!buffer_output)
+               flush_output();

 }
 
 static void output_commit_title(struct commit *commit)
 }
 
 static void output_commit_title(struct commit *commit)

diff --git a/trace.c b/trace.c
index 7961a27a2ed4f32c766dabdf12c4115c3d3b36ba..91548a56eceb56cfcb0521fc0313afe51dff0353 100644 (file)
--- a/trace.c
+++ b/trace.c
@@ -25,33 +25,6 @@
 #include "cache.h"
 #include "quote.h"
 
 #include "cache.h"
 #include "quote.h"
 

-/* Stolen from "imap-send.c". */
-int nfvasprintf(char **strp, const char *fmt, va_list ap)
-{
-       int len;
-       char tmp[1024];
-
-       if ((len = vsnprintf(tmp, sizeof(tmp), fmt, ap)) < 0 ||
-           !(*strp = xmalloc(len + 1)))
-               die("Fatal: Out of memory\n");
-       if (len >= (int)sizeof(tmp))
-               vsprintf(*strp, fmt, ap);
-       else
-               memcpy(*strp, tmp, len + 1);
-       return len;
-}
-
-int nfasprintf(char **str, const char *fmt, ...)
-{
-       int rc;
-       va_list args;
-
-       va_start(args, fmt);
-       rc = nfvasprintf(str, fmt, args);
-       va_end(args);
-       return rc;
-}
-

 /* Get a trace file descriptor from GIT_TRACE env variable. */
 static int get_trace_fd(int *need_close)
 {
 /* Get a trace file descriptor from GIT_TRACE env variable. */
 static int get_trace_fd(int *need_close)
 {


@@ -89,63 +62,54 @@ static int get_trace_fd(int *need_close)
 static const char err_msg[] = "Could not trace into fd given by "
        "GIT_TRACE environment variable";
 
 static const char err_msg[] = "Could not trace into fd given by "
        "GIT_TRACE environment variable";
 

-void trace_printf(const char *format, ...)
+void trace_printf(const char *fmt, ...)

 {
 {

-       char *trace_str;
-       va_list rest;
-       int need_close = 0;
-       int fd = get_trace_fd(&need_close);
+       char buf[8192];
+       va_list ap;
+       int fd, len, need_close = 0;

 
 

+       fd = get_trace_fd(&need_close);

        if (!fd)
                return;
 
        if (!fd)
                return;
 

-       va_start(rest, format);
-       nfvasprintf(&trace_str, format, rest);
-       va_end(rest);
-
-       write_or_whine_pipe(fd, trace_str, strlen(trace_str), err_msg);
-
-       free(trace_str);
+       va_start(ap, fmt);
+       len = vsnprintf(buf, sizeof(buf), fmt, ap);
+       va_end(ap);
+       if (len >= sizeof(buf))
+               die("unreasonnable trace length");
+       write_or_whine_pipe(fd, buf, len, err_msg);

 
        if (need_close)
                close(fd);
 }
 
 
        if (need_close)
                close(fd);
 }
 

-void trace_argv_printf(const char **argv, int count, const char *format, ...)
+void trace_argv_printf(const char **argv, int count, const char *fmt, ...)

 {
 {

-       char *argv_str, *format_str, *trace_str;
-       size_t argv_len, format_len, trace_len;
-       va_list rest;
-       int need_close = 0;
-       int fd = get_trace_fd(&need_close);
+       char buf[8192];
+       va_list ap;
+       char *argv_str;
+       size_t argv_len;
+       int fd, len, need_close = 0;

 
 

+       fd = get_trace_fd(&need_close);

        if (!fd)
                return;
 
        if (!fd)
                return;
 

+       va_start(ap, fmt);
+       len = vsnprintf(buf, sizeof(buf), fmt, ap);
+       va_end(ap);
+       if (len >= sizeof(buf))
+               die("unreasonnable trace length");
+

        /* Get the argv string. */
        argv_str = sq_quote_argv(argv, count);
        argv_len = strlen(argv_str);
 
        /* Get the argv string. */
        argv_str = sq_quote_argv(argv, count);
        argv_len = strlen(argv_str);
 

-       /* Get the formated string. */
-       va_start(rest, format);
-       nfvasprintf(&format_str, format, rest);
-       va_end(rest);
-
-       /* Allocate buffer for trace string. */
-       format_len = strlen(format_str);
-       trace_len = argv_len + format_len + 1; /* + 1 for \n */
-       trace_str = xmalloc(trace_len + 1);
-
-       /* Copy everything into the trace string. */
-       strncpy(trace_str, format_str, format_len);
-       strncpy(trace_str + format_len, argv_str, argv_len);
-       strcpy(trace_str + trace_len - 1, "\n");
-
-       write_or_whine_pipe(fd, trace_str, trace_len, err_msg);
+       write_or_whine_pipe(fd, buf, len, err_msg);
+       write_or_whine_pipe(fd, argv_str, argv_len, err_msg);
+       write_or_whine_pipe(fd, "\n", 1, err_msg);

 
        free(argv_str);
 
        free(argv_str);

-       free(format_str);
-       free(trace_str);

 
        if (need_close)
                close(fd);
 
        if (need_close)
                close(fd);