[PATCH 2/2] perf tools: Fix strbuf_addf() when the buffer needs to grow

[PATCH 2/2] perf tools: Fix strbuf_addf() when the buffer needs to grow

[Namhyung Kim]

This was found during chasing down the header output regression.
The strbuf_addf() was checking buffer length with a result of
vscnprintf() which cannot be greater than that of strbuf_avail().

Since numa topology and pmu mapping info in header were converted
to use strbuf, it sometimes caused uninteresting behaviors with the
broken strbuf.

Fix it by using vsnprintf() which returns desired output string
length regardless of the available buffer size and grow the buffer
if needed.

Reported-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/strbuf.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/strbuf.c b/tools/perf/util/strbuf.c
index 2eeb51baf077..cfa906882e2c 100644
--- a/tools/perf/util/strbuf.c
+++ b/tools/perf/util/strbuf.c
@@ -90,17 +90,17 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...)
 	if (!strbuf_avail(sb))
 		strbuf_grow(sb, 64);
 	va_start(ap, fmt);
-	len = vscnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
+	len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
 	va_end(ap);
 	if (len < 0)
-		die("your vscnprintf is broken");
+		die("your vsnprintf is broken");
 	if (len > strbuf_avail(sb)) {
 		strbuf_grow(sb, len);
 		va_start(ap, fmt);
-		len = vscnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
+		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");
+			die("this should not happen, your vsnprintf is broken");
 		}
 	}
 	strbuf_setlen(sb, sb->len + len);
-- 
1.7.9.2

Re: [PATCH 2/2] perf tools: Fix strbuf_addf() when the buffer needs to grow

[Andrew Jones]

On Tue, Oct 23, 2012 at 10:44:50PM +0900, Namhyung Kim wrote:
> This was found during chasing down the header output regression.
> The strbuf_addf() was checking buffer length with a result of
> vscnprintf() which cannot be greater than that of strbuf_avail().
> 
> Since numa topology and pmu mapping info in header were converted
> to use strbuf, it sometimes caused uninteresting behaviors with the
> broken strbuf.
> 
> Fix it by using vsnprintf() which returns desired output string
> length regardless of the available buffer size and grow the buffer
> if needed.
> 
> Reported-by: Andrew Jones <drjones@redhat.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/strbuf.c |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/util/strbuf.c b/tools/perf/util/strbuf.c
> index 2eeb51baf077..cfa906882e2c 100644
> --- a/tools/perf/util/strbuf.c
> +++ b/tools/perf/util/strbuf.c
> @@ -90,17 +90,17 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...)
>  	if (!strbuf_avail(sb))
>  		strbuf_grow(sb, 64);
>  	va_start(ap, fmt);
> -	len = vscnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
> +	len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
>  	va_end(ap);
>  	if (len < 0)
> -		die("your vscnprintf is broken");
> +		die("your vsnprintf is broken");
>  	if (len > strbuf_avail(sb)) {
>  		strbuf_grow(sb, len);
>  		va_start(ap, fmt);
> -		len = vscnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
> +		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");
> +			die("this should not happen, your vsnprintf is broken");
>  		}
>  	}
>  	strbuf_setlen(sb, sb->len + len);
> -- 
> 1.7.9.2
>

Tested-by: Andrew Jones <drjones@redhat.com> 

[GIT PULL 0/2] perf/urgent fixes

[Arnaldo Carvalho de Melo]

Hi Ingo,

	Please consider pulling,

- Arnaldo

The following changes since commit 0d855354ea351bec6b222e9fea86a876cfafdcb6:

  perf, powerpc: Fix hw breakpoints returning -ENOSPC (2012-10-30 10:07:58 +0100)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux tags/perf-urgent-for-mingo

for you to fetch changes up to f787d9519fb10411f2948f5b9957a1669879ba84:

  perf tools: Fix strbuf_addf() when the buffer needs to grow (2012-10-30 10:32:56 -0200)

----------------------------------------------------------------
perf/urgent fixes

.  Fix numa topology printing, from Namhyung Kim.

Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

----------------------------------------------------------------
Namhyung Kim (2):
      perf header: Fix numa topology printing
      perf tools: Fix strbuf_addf() when the buffer needs to grow

 tools/perf/util/header.c |    2 ++
 tools/perf/util/strbuf.c |    8 ++++----
 2 files changed, 6 insertions(+), 4 deletions(-)

[PATCH 1/2] perf header: Fix numa topology printing

[Arnaldo Carvalho de Melo]

From: Namhyung Kim <namhyung@kernel.org>

Andrew reported that the commit 7e94cfcc9d20 ("perf header: Use pre-
processed session env when printing") regresses the header output.  It
was because of a missed string pointer calculation in the loop.

Reported-by: Andrew Jones <drjones@redhat.com>
Tested-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andrew Jones <drjones@redhat.com>
Link: http://lkml.kernel.org/r/1350999890-6920-1-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 7daad23..566b84c 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -1378,6 +1378,8 @@ static void print_numa_topology(struct perf_header *ph, int fd __maybe_unused,
 
 		str = tmp + 1;
 		fprintf(fp, "# node%u cpu list : %s\n", c, str);
+
+		str += strlen(str) + 1;
 	}
 	return;
 error:
-- 
1.7.9.2.358.g22243

[PATCH 2/2] perf tools: Fix strbuf_addf() when the buffer needs to grow

[Arnaldo Carvalho de Melo]

From: Namhyung Kim <namhyung@kernel.org>

This was found during chasing down the header output regression.  The
strbuf_addf() was checking buffer length with a result of vscnprintf()
which cannot be greater than that of strbuf_avail().

Since numa topology and pmu mapping info in header were converted to use
strbuf, it sometimes caused uninteresting behaviors with the broken
strbuf.

Fix it by using vsnprintf() which returns desired output string length
regardless of the available buffer size and grow the buffer if needed.

Reported-by: Andrew Jones <drjones@redhat.com>
Tested-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andrew Jones <drjones@redhat.com>
Link: http://lkml.kernel.org/r/1350999890-6920-2-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/strbuf.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/strbuf.c b/tools/perf/util/strbuf.c
index 2eeb51b..cfa9068 100644
--- a/tools/perf/util/strbuf.c
+++ b/tools/perf/util/strbuf.c
@@ -90,17 +90,17 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...)
 	if (!strbuf_avail(sb))
 		strbuf_grow(sb, 64);
 	va_start(ap, fmt);
-	len = vscnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
+	len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
 	va_end(ap);
 	if (len < 0)
-		die("your vscnprintf is broken");
+		die("your vsnprintf is broken");
 	if (len > strbuf_avail(sb)) {
 		strbuf_grow(sb, len);
 		va_start(ap, fmt);
-		len = vscnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
+		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");
+			die("this should not happen, your vsnprintf is broken");
 		}
 	}
 	strbuf_setlen(sb, sb->len + len);
-- 
1.7.9.2.358.g22243

Re: [GIT PULL 0/2] perf/urgent fixes

[Ingo Molnar]

* Arnaldo Carvalho de Melo <acme@infradead.org> wrote:

> Hi Ingo,
> 
> 	Please consider pulling,
> 
> - Arnaldo
> 
> The following changes since commit 0d855354ea351bec6b222e9fea86a876cfafdcb6:
> 
>   perf, powerpc: Fix hw breakpoints returning -ENOSPC (2012-10-30 10:07:58 +0100)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux tags/perf-urgent-for-mingo
> 
> for you to fetch changes up to f787d9519fb10411f2948f5b9957a1669879ba84:
> 
>   perf tools: Fix strbuf_addf() when the buffer needs to grow (2012-10-30 10:32:56 -0200)
> 
> ----------------------------------------------------------------
> perf/urgent fixes
> 
> .  Fix numa topology printing, from Namhyung Kim.
> 
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> ----------------------------------------------------------------
> Namhyung Kim (2):
>       perf header: Fix numa topology printing
>       perf tools: Fix strbuf_addf() when the buffer needs to grow
> 
>  tools/perf/util/header.c |    2 ++
>  tools/perf/util/strbuf.c |    8 ++++----
>  2 files changed, 6 insertions(+), 4 deletions(-)

Pulled, thanks Arnaldo!

	Ingo