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

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

 



Pierre Habouzit <madcoder@xxxxxxxxxx> writes:

> 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...

> +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)
-
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