[PATCH 2/7] nfv?asprintf are broken without va_copy, workaround them.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



* 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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux