* 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 API's so that only one of the trace functions takes a vararg. 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@xxxxxxxxxx> --- cache.h | 4 +-- exec_cmd.c | 3 +- git.c | 8 +++--- imap-send.c | 13 +++++++++ merge-recursive.c | 74 +++++++++++++++++++++++--------------------------- trace.c | 78 +++++++++++----------------------------------------- 6 files changed, 71 insertions(+), 109 deletions(-) diff --git a/cache.h b/cache.h index c57ccd6..9863917 100644 --- a/cache.h +++ b/cache.h @@ -587,10 +587,8 @@ extern void *alloc_object_node(void); 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_argv(const char **argv, int count); /* convert.c */ /* returns 1 if *dst was used */ diff --git a/exec_cmd.c b/exec_cmd.c index 9b74ed2..c0f954e 100644 --- a/exec_cmd.c +++ b/exec_cmd.c @@ -97,7 +97,8 @@ int execv_git_cmd(const char **argv) tmp = argv[0]; argv[0] = git_command; - trace_argv_printf(argv, -1, "trace: exec:"); + trace_printf("trace: exec:"); + trace_argv(argv, -1); /* execve() can only ever return if it fails */ execve(git_command, (char **)argv, environ); diff --git a/git.c b/git.c index 6773c12..28e4b7f 100644 --- a/git.c +++ b/git.c @@ -226,9 +226,8 @@ static int handle_alias(int *argcp, const char ***argv) if (!strcmp(alias_command, new_argv[0])) die("recursive alias: %s", alias_command); - trace_argv_printf(new_argv, count, - "trace: alias expansion: %s =>", - alias_command); + trace_printf("trace: alias expansion: %s =>", alias_command); + trace_argv(new_argv, count); new_argv = xrealloc(new_argv, sizeof(char*) * (count + *argcp + 1)); @@ -285,7 +284,8 @@ static int run_command(struct cmd_struct *p, int argc, const char **argv) if (!work_tree || chdir(work_tree)) die("%s must be run in a work tree", p->cmd); } - trace_argv_printf(argv, argc, "trace: built-in: git"); + trace_printf("trace: built-in: git"); + trace_argv(argv, argc); status = p->fn(argc, argv, prefix); if (status) diff --git a/imap-send.c b/imap-send.c index 905d097..e95cdde 100644 --- 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 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 ); diff --git a/merge-recursive.c b/merge-recursive.c index 14b56c2..4e27549 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -85,63 +85,57 @@ struct stage_data 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 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; } -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); + if (show(v)) { + int len; + va_list ap; + + strbuf_grow(&obuf, call_depth); + memset(obuf.buf + obuf.len, ' ', call_depth); + strbuf_setlen(&obuf, obuf.len + call_depth); + + va_start(ap, fmt); + len = vsnprintf(obuf.buf, strbuf_avail(&obuf) + 1, fmt, ap); + va_end(ap); + + if (len < 0) + len = 0; + if (len > strbuf_avail(&obuf)) { + strbuf_grow(&obuf, len); + va_start(ap, fmt); + len = vsnprintf(obuf.buf, strbuf_avail(&obuf) + 1, fmt, ap); + va_end(ap); + if (len > strbuf_avail(&obuf)) { + die("this should not happen, your snprintf is broken"); + } + } + + strbuf_setlen(&obuf, obuf.len + len); + if (!buffer_output) + flush_output(); } - output_list = NULL; - output_end = NULL; } static void output_commit_title(struct commit *commit) diff --git a/trace.c b/trace.c index 7961a27..be1e274 100644 --- a/trace.c +++ b/trace.c @@ -25,33 +25,6 @@ #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) { @@ -89,36 +62,35 @@ static int get_trace_fd(int *need_close) 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; - 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); } -void trace_argv_printf(const char **argv, int count, const char *format, ...) +void trace_argv(const char **argv, int count) { - char *argv_str, *format_str, *trace_str; - size_t argv_len, format_len, trace_len; - va_list rest; + char *argv_str; + size_t argv_len; int need_close = 0; int fd = get_trace_fd(&need_close); + fd = get_trace_fd(&need_close); if (!fd) return; @@ -126,26 +98,10 @@ void trace_argv_printf(const char **argv, int count, const char *format, ...) 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, argv_str, argv_len, err_msg); + write_or_whine_pipe(fd, "\n", 1, err_msg); free(argv_str); - free(format_str); - free(trace_str); if (need_close) close(fd); -- 1.5.3.2.1036.gccf7 - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html