From ab8632ae36d2e5faf524309696725b60ec18e588 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 25 Feb 2011 23:08:25 -0600 Subject: compat: provide a fallback va_copy definition va_copy is C99. We have avoided using va_copy many times in the past, which has led to a bunch of cut-and-paste. From everything I found searching the web, implementations have historically either provided va_copy or just let your code assume that simple assignment of worked. So my guess is that this will be sufficient, though we won't really know for sure until somebody reports a problem. Signed-off-by: Jeff King Improved-by: Erik Faye-Lund Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- compat/msvc.h | 1 - git-compat-util.h | 4 ++++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/compat/msvc.h b/compat/msvc.h index 023aba0238..a33b01c032 100644 --- a/compat/msvc.h +++ b/compat/msvc.h @@ -9,7 +9,6 @@ #define inline __inline #define __inline__ __inline #define __attribute__(x) -#define va_copy(dst, src) ((dst) = (src)) #define strncasecmp _strnicmp #define ftruncate _chsize diff --git a/git-compat-util.h b/git-compat-util.h index 9c23622ed5..00d41e4f0e 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -535,6 +535,10 @@ void git_qsort(void *base, size_t nmemb, size_t size, #define fstat_is_reliable() 1 #endif +#ifndef va_copy +#define va_copy(dst,src) (dst) = (src) +#endif + /* * Preserves errno, prints a message, but gives no warning for ENOENT. * Always returns the return value of unlink(2). -- cgit v1.2.1 From ebeb60900fbab569ed14f710a0a1abb1637ec792 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 25 Feb 2011 23:08:53 -0600 Subject: 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 Improved-by: Christian Couder Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- fsck.c | 14 +------------- merge-recursive.c | 15 +-------------- strbuf.c | 25 +++++++++++++++---------- strbuf.h | 2 ++ trace.c | 32 ++++++-------------------------- 5 files changed, 25 insertions(+), 63 deletions(-) diff --git a/fsck.c b/fsck.c index 3d05d4a794..6f266c1ea4 100644 --- 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 len; 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); - 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; diff --git a/merge-recursive.c b/merge-recursive.c index 16c2dbeab9..2a4f739365 100644 --- 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, ...) { - int len; 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); - len = vsnprintf(o->obuf.buf + o->obuf.len, strbuf_avail(&o->obuf), fmt, ap); + strbuf_vaddf(&o->obuf, fmt, 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); diff --git a/strbuf.c b/strbuf.c index 07e8883ceb..77444a94df 100644 --- 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, ...) { - int len; 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); - 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) - die("your vsnprintf is broken"); + die("BUG: your vsnprintf is broken (returned %d)", 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); - 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); } diff --git a/strbuf.h b/strbuf.h index 675a91f938..f722331470 100644 --- 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,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 */ diff --git a/trace.c b/trace.c index 35d388dce4..eda3f6d721 100644 --- 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, ...) { - struct strbuf buf; + struct strbuf buf = STRBUF_INIT; 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 */ - strbuf_init(&buf, 64); va_start(ap, fmt); - len = vsnprintf(buf.buf, strbuf_avail(&buf), fmt, ap); + strbuf_vaddf(&buf, fmt, 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); @@ -96,28 +86,18 @@ void trace_printf(const char *fmt, ...) void trace_argv_printf(const char **argv, const char *fmt, ...) { - struct strbuf buf; + struct strbuf buf = STRBUF_INIT; 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 */ - strbuf_init(&buf, 64); va_start(ap, fmt); - len = vsnprintf(buf.buf, strbuf_avail(&buf), fmt, ap); + strbuf_vaddf(&buf, fmt, 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'); -- cgit v1.2.1 From 26db0f2e3afc043e184a5e0ce5eb7c53aeb1f644 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Tue, 8 Mar 2011 02:33:44 -0600 Subject: compat: fall back on __va_copy if available Since an obvious implementation of va_list is to make it a pointer into the stack frame, implementing va_copy as "dst = src" will work on many systems. Platforms that use something different (e.g., a size-1 array of structs, to be assigned with *(dst) = *(src)) will need some other compatibility macro, though. Luckily, as the glibc manual hints, such systems tend to provide the __va_copy macro (introduced in GCC in March, 1997). By using that if it is available, we can cover our bases pretty well. Discovered by building with CC="gcc -std=c89" on an amd64 machine: $ make CC=c89 strbuf.o [...] strbuf.c: In function 'strbuf_vaddf': strbuf.c:211:2: error: incompatible types when assigning to type 'va_list' from type 'struct __va_list_tag *' make: *** [strbuf.o] Error 1 Explained-by: Junio C Hamano Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- git-compat-util.h | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/git-compat-util.h b/git-compat-util.h index 00d41e4f0e..f4cb0a9b01 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -536,7 +536,16 @@ void git_qsort(void *base, size_t nmemb, size_t size, #endif #ifndef va_copy -#define va_copy(dst,src) (dst) = (src) +/* + * Since an obvious implementation of va_list would be to make it a + * pointer into the stack frame, a simple assignment will work on + * many systems. But let's try to be more portable. + */ +#ifdef __va_copy +#define va_copy(dst, src) __va_copy(dst, src) +#else +#define va_copy(dst, src) ((dst) = (src)) +#endif #endif /* -- cgit v1.2.1