From: Pierre Habouzit <madcoder@debian.org>
To: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: [SUPERSEDES PATCH 2/7] nfv?asprintf are broken without va_copy, workaround them.
Date: Thu, 20 Sep 2007 10:43:11 +0200 [thread overview]
Message-ID: <20070920084311.GB2053@artemis.corp> (raw)
In-Reply-To: <20070920082701.GA2053@artemis.corp>
* 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>
---
This reinstates the trace_argv_printf API. The implementation is
stupid, but is rewritten in a latter commit. I didn't wanted to bother
optimizing it.
cache.h | 2 -
imap-send.c | 13 ++++++++
merge-recursive.c | 74 ++++++++++++++++++++-----------------------
trace.c | 90 ++++++++++++++++-------------------------------------
4 files changed, 74 insertions(+), 105 deletions(-)
diff --git a/cache.h b/cache.h
index c57ccd6..b127c43 100644
--- a/cache.h
+++ b/cache.h
@@ -587,8 +587,6 @@ 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, ...);
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..91548a5 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,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";
-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_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;
+ 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 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(format_str);
- free(trace_str);
if (need_close)
close(fd);
--
1.5.3.2.1036.gca8d2
next prev parent reply other threads:[~2007-09-20 8:43 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-09-19 22:42 quote/strbuf series, take 3 Pierre Habouzit
[not found] ` <1190241736-30449-2-git-send-email-madcoder@debian.org>
[not found] ` <1190241736-30449-3-git-send-email-madcoder@debian.org>
2007-09-20 4:27 ` [PATCH 2/7] nfv?asprintf are broken without va_copy, workaround them Junio C Hamano
2007-09-20 4:53 ` Christian Couder
2007-09-20 8:27 ` Pierre Habouzit
2007-09-20 8:43 ` Pierre Habouzit [this message]
2007-09-21 6:17 ` [SUPERSEDES PATCH " Junio C Hamano
2007-09-21 7:02 ` Pierre Habouzit
2007-09-20 8:44 ` [SUPERSEDES PATCH 4/7] sq_quote_argv and add_to_string rework with strbuf's Pierre Habouzit
[not found] ` <1190241736-30449-4-git-send-email-madcoder@debian.org>
[not found] ` <1190241736-30449-5-git-send-email-madcoder@debian.org>
[not found] ` <1190241736-30449-6-git-send-email-madcoder@debian.org>
[not found] ` <1190241736-30449-7-git-send-email-madcoder@debian.org>
[not found] ` <1190241736-30449-8-git-send-email-madcoder@debian.org>
2007-09-20 22:05 ` [PATCH 7/7] Avoid duplicating memory, and use xmemdupz instead of xstrdup Pierre Habouzit
2007-09-21 7:03 ` Pierre Habouzit
2007-09-21 7:39 ` [DON'T MERGE PATCH 7/7] Pierre Habouzit
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20070920084311.GB2053@artemis.corp \
--to=madcoder@debian.org \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).