Re: [SUPERSEDES PATCH 2/7] nfv?asprintf are broken without va_copy, workaround them.
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 21, 2007, 06:17 UTC
- Message-ID
- <7vejgsk0ev.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <20070920084311.GB2053@artemis.corp>
Pierre Habouzit <madcoder@debian.org> writes:
Show 22 quoted lines
> 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/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
> +static void flush_output(void)
> {
> + if (obuf.len) {
> + fputs(obuf.buf, stdout);
> + strbuf_reset(&obuf);
> }
> }This assumes obuf.buf has necessary indentations and line breaks, which is sensible. However...
Show 5 quoted lines
> +static void output(int v, const char *fmt, ...)
> {
> + if (show(v)) {
> + int len;
> + va_list ap;Yuck, this single if statement covers the entirety of the function. Let's do
if (!show(v)) return;
> + strbuf_grow(&obuf, call_depth); > + memset(obuf.buf + obuf.len, ' ', call_depth); > + strbuf_setlen(&obuf, obuf.len + call_depth);
Per depth indentation used to be two whitespaces.
> + va_start(ap, fmt); > + len = vsnprintf(obuf.buf, strbuf_avail(&obuf) + 1, fmt, ap); > + va_end(ap);
And you overwrite whatever used to be in the buffer, including the previous buffered message and indentation you added. Not nice...
I'll squash this on top of yours for now.
merge-recursive.c | 45 +++++++++++++++++++++++---------------------- 1 files changed, 23 insertions(+), 22 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c index 4e27549..86767e6 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -108,34 +108,35 @@ static void flush_output(void) static void output(int v, const char *fmt, ...) { - if (show(v)) { - int len; - va_list ap; + int len; + va_list ap; - strbuf_grow(&obuf, call_depth); - memset(obuf.buf + obuf.len, ' ', call_depth); - strbuf_setlen(&obuf, obuf.len + call_depth); + 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, strbuf_avail(&obuf) + 1, fmt, ap); + 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); - 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"); - } + 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(); } + strbuf_setlen(&obuf, obuf.len + len); + strbuf_add(&obuf, "\n", 1); + if (!buffer_output) + flush_output(); } static void output_commit_title(struct commit *commit)