[PATCH 1/2] perf: fix build error

[PATCH 1/2] perf: fix build error

[Kirill A. Shutemov]

From: "Kirill A. Shutemov" <kirill@shutemov.name>

util/parse-events.c:29:5: error: redundant redeclaration of ‘parse_events_parse’ [-Werror=redundant-decls]
In file included from util/parse-events.c:14:0:
util/parse-events-bison.h:99:5: note: previous declaration of ‘parse_events_parse’ was here
cc1: all warnings being treated as errors

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 tools/perf/util/parse-events.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 1aa721d..ce61cba 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -26,7 +26,6 @@ struct event_symbol {
 #ifdef PARSER_DEBUG
 extern int parse_events_debug;
 #endif
-int parse_events_parse(void *data, void *scanner);
 
 static struct event_symbol event_symbols_hw[PERF_COUNT_HW_MAX] = {
 	[PERF_COUNT_HW_CPU_CYCLES] = {
-- 
1.7.11.2

[PATCH 2/2] perf: use XSI-complaint version of strerror_r() instead of GNU-specific

[Kirill A. Shutemov]

From: "Kirill A. Shutemov" <kirill@shutemov.name>

Perf uses GNU-specific version of strerror_r(). The GNU-specific
strerror_r() returns a pointer to a string containing the error message.
This may be either a pointer to a string that the function stores in
buf, or a pointer to some (immutable) static string (in which case buf
is unused).

In glibc-2.16 GNU version was marked with attribute warn_unused_result.
It triggers few warnings in perf:

util/target.c: In function ‘perf_target__strerror’:
util/target.c:114:13: error: ignoring return value of ‘strerror_r’, declared with attribute warn_unused_result [-Werror=unused-result]
ui/browsers/hists.c: In function ‘hist_browser__dump’:
ui/browsers/hists.c:981:13: error: ignoring return value of ‘strerror_r’, declared with attribute warn_unused_result [-Werror=unused-result]

They are bugs.

The right way to fix it is to switch to XSI-compliant version.
Unfortunately, glibc doesn't allow to get XSI-complaint strerror_r() and
_GNU_SOURCE at the same time.

Let's bundle XSI-complaint version to perf and use it.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 tools/perf/ui/browsers/hists.c |  2 +-
 tools/perf/util/string.c       | 25 +++++++++++++++++++++++++
 tools/perf/util/target.c       |  3 ++-
 tools/perf/util/util.h         |  1 +
 4 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 482f051..8bd1780 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -978,7 +978,7 @@ static int hist_browser__dump(struct hist_browser *browser)
 	fp = fopen(filename, "w");
 	if (fp == NULL) {
 		char bf[64];
-		strerror_r(errno, bf, sizeof(bf));
+		xsi_strerror_r(errno, bf, sizeof(bf));
 		ui_helpline__fpush("Couldn't write to %s: %s", filename, bf);
 		return -1;
 	}
diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
index 199bc4d..71b68b9 100644
--- a/tools/perf/util/string.c
+++ b/tools/perf/util/string.c
@@ -1,5 +1,6 @@
 #include "util.h"
 #include "string.h"
+#include <linux/kernel.h>
 
 #define K 1024LL
 /*
@@ -335,3 +336,27 @@ char *rtrim(char *s)
 
 	return s;
 }
+
+/**
+ * xsi_strerror_r() - XSI-compliant version of strerror_r()
+ */
+int xsi_strerror_r(int errnum, char *buf, size_t buflen)
+{
+	const char *p;
+	size_t len;
+
+	/* GNU-specific version of strerror_r() */
+	p = strerror_r(errnum, buf, buflen);
+
+	/* glibc use the buffer only if errnum is not correct */
+	if (p == buf)
+		return EINVAL;
+
+	len = strlen(p);
+	if (buflen > 0) {
+		char *c = mempcpy(buf, p, min(buflen - 1, len));
+		*c = '\0';
+	}
+
+	return buflen > len ? 0 : ERANGE;
+}
diff --git a/tools/perf/util/target.c b/tools/perf/util/target.c
index 1064d5b..0ec1a30 100644
--- a/tools/perf/util/target.c
+++ b/tools/perf/util/target.c
@@ -8,6 +8,7 @@
 
 #include "target.h"
 #include "debug.h"
+#include "util.h"
 
 #include <pwd.h>
 #include <string.h>
@@ -111,7 +112,7 @@ int perf_target__strerror(struct perf_target *target, int errnum,
 	const char *msg;
 
 	if (errnum >= 0) {
-		strerror_r(errnum, buf, buflen);
+		xsi_strerror_r(errnum, buf, buflen);
 		return 0;
 	}
 
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index b13c733..3e85d63 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -265,5 +265,6 @@ bool is_power_of_2(unsigned long n)
 size_t hex_width(u64 v);
 
 char *rtrim(char *s);
+int xsi_strerror_r(int errnum, char *buf, size_t buflen);
 
 #endif
-- 
1.7.11.2

Re: [PATCH 2/2] perf: use XSI-complaint version of strerror_r() instead of GNU-specific

[Ulrich Drepper]

On Mon, Jul 23, 2012 at 11:00 AM, Kirill A. Shutemov
<kirill@shutemov.name> wrote:
> The right way to fix it is to switch to XSI-compliant version.

And why exactly would this be "the right way"?  Just fix the use of
strerror_r or use strerror_l.

Re: [PATCH 2/2] perf: use XSI-complaint version of strerror_r() instead of GNU-specific

[Kirill A. Shutemov]

On Mon, Jul 23, 2012 at 02:00:33PM -0400, Ulrich Drepper wrote:
> On Mon, Jul 23, 2012 at 11:00 AM, Kirill A. Shutemov
> <kirill@shutemov.name> wrote:
> > The right way to fix it is to switch to XSI-compliant version.
> 
> And why exactly would this be "the right way"?  Just fix the use of
> strerror_r or use strerror_l.

Okay. What about this:

>From 8b76ea28a09ebc72c7bdcc1d92e80a63a5cdaf1c Mon Sep 17 00:00:00 2001
From: "Kirill A. Shutemov" <kirill@shutemov.name>
Date: Mon, 23 Jul 2012 17:41:05 +0300
Subject: [PATCH] perf: fix strerror_r() usage
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Perf uses GNU-specific version of strerror_r(). The GNU-specific
strerror_r() returns a pointer to a string containing the error message.
This may be either a pointer to a string that the function stores in
buf, or a pointer to some (immutable) static string (in which case buf
is unused).

In glibc-2.16 GNU version was marked with attribute warn_unused_result.
It triggers few warnings in perf:

util/target.c: In function ‘perf_target__strerror’:
util/target.c:114:13: error: ignoring return value of ‘strerror_r’, declared with attribute warn_unused_result [-Werror=unused-result]
ui/browsers/hists.c: In function ‘hist_browser__dump’:
ui/browsers/hists.c:981:13: error: ignoring return value of ‘strerror_r’, declared with attribute warn_unused_result [-Werror=unused-result]

They are bugs.

Let's fix strerror_r() usage.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 tools/perf/ui/browsers/hists.c | 4 ++--
 tools/perf/util/target.c       | 9 ++++++++-
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 482f051..413bd62 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -978,8 +978,8 @@ static int hist_browser__dump(struct hist_browser *browser)
 	fp = fopen(filename, "w");
 	if (fp == NULL) {
 		char bf[64];
-		strerror_r(errno, bf, sizeof(bf));
-		ui_helpline__fpush("Couldn't write to %s: %s", filename, bf);
+		const char *err = strerror_r(errno, bf, sizeof(bf));
+		ui_helpline__fpush("Couldn't write to %s: %s", filename, err);
 		return -1;
 	}
 
diff --git a/tools/perf/util/target.c b/tools/perf/util/target.c
index 1064d5b..45ab408 100644
--- a/tools/perf/util/target.c
+++ b/tools/perf/util/target.c
@@ -111,7 +111,14 @@ int perf_target__strerror(struct perf_target *target, int errnum,
 	const char *msg;
 
 	if (errnum >= 0) {
-		strerror_r(errnum, buf, buflen);
+		const char *err = strerror_r(errnum, buf, buflen);
+
+		if (err != buf && buflen > 0) {
+			size_t len = strlen(err);
+			char *c = mempcpy(buf, err, min(buflen - 1, len));
+			*c = '\0';
+		}
+
 		return 0;
 	}
 
-- 
 Kirill A. Shutemov

Re: [PATCH 2/2] perf: use XSI-complaint version of strerror_r() instead of GNU-specific

[Ulrich Drepper]

On Mon, Jul 23, 2012 at 4:31 PM, Kirill A. Shutemov
<kirill@shutemov.name> wrote:
> +               const char *err = strerror_r(errnum, buf, buflen);
> +
> +               if (err != buf && buflen > 0) {
> +                       size_t len = strlen(err);
> +                       char *c = mempcpy(buf, err, min(buflen - 1, len));
> +                       *c = '\0';
> +               }

No need to check for err == NULL.   buflen == 0 is a possibility given
the interface but I'd say this is an error and should be tested for at
the beginning of the function and the call should fail or even abort
the program.

Re: [PATCH 2/2] perf: use XSI-complaint version of strerror_r() instead of GNU-specific

[Kirill A. Shutemov]

On Mon, Jul 23, 2012 at 04:48:22PM -0400, Ulrich Drepper wrote:
> On Mon, Jul 23, 2012 at 4:31 PM, Kirill A. Shutemov
> <kirill@shutemov.name> wrote:
> > +               const char *err = strerror_r(errnum, buf, buflen);
> > +
> > +               if (err != buf && buflen > 0) {
> > +                       size_t len = strlen(err);
> > +                       char *c = mempcpy(buf, err, min(buflen - 1, len));
> > +                       *c = '\0';
> > +               }
> 
> No need to check for err == NULL.

There's no such check.

> buflen == 0 is a possibility given
> the interface but I'd say this is an error and should be tested for at
> the beginning of the function and the call should fail or even abort
> the program.

>From 11d62205ee3c534aa9b0e9a24a312438ac726ffb Mon Sep 17 00:00:00 2001
From: "Kirill A. Shutemov" <kirill@shutemov.name>
Date: Mon, 23 Jul 2012 17:41:05 +0300
Subject: [PATCH 2/2] perf: fix strerror_r() usage
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Perf uses GNU-specific version of strerror_r(). The GNU-specific
strerror_r() returns a pointer to a string containing the error message.
This may be either a pointer to a string that the function stores in
buf, or a pointer to some (immutable) static string (in which case buf
is unused).

In glibc-2.16 GNU version was marked with attribute warn_unused_result.
It triggers few warnings in perf:

util/target.c: In function ‘perf_target__strerror’:
util/target.c:114:13: error: ignoring return value of ‘strerror_r’, declared with attribute warn_unused_result [-Werror=unused-result]
ui/browsers/hists.c: In function ‘hist_browser__dump’:
ui/browsers/hists.c:981:13: error: ignoring return value of ‘strerror_r’, declared with attribute warn_unused_result [-Werror=unused-result]

They are bugs.

Let's fix strerror_r() usage.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 tools/perf/ui/browsers/hists.c |  4 ++--
 tools/perf/util/target.c       | 12 +++++++++++-
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 482f051..413bd62 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -978,8 +978,8 @@ static int hist_browser__dump(struct hist_browser *browser)
 	fp = fopen(filename, "w");
 	if (fp == NULL) {
 		char bf[64];
-		strerror_r(errno, bf, sizeof(bf));
-		ui_helpline__fpush("Couldn't write to %s: %s", filename, bf);
+		const char *err = strerror_r(errno, bf, sizeof(bf));
+		ui_helpline__fpush("Couldn't write to %s: %s", filename, err);
 		return -1;
 	}
 
diff --git a/tools/perf/util/target.c b/tools/perf/util/target.c
index 1064d5b..5c4b3b1 100644
--- a/tools/perf/util/target.c
+++ b/tools/perf/util/target.c
@@ -9,6 +9,7 @@
 #include "target.h"
 #include "debug.h"
 
+#include <assert.h>
 #include <pwd.h>
 #include <string.h>
 
@@ -110,8 +111,17 @@ int perf_target__strerror(struct perf_target *target, int errnum,
 	int idx;
 	const char *msg;
 
+	assert(buflen > 0);
+
 	if (errnum >= 0) {
-		strerror_r(errnum, buf, buflen);
+		const char *err = strerror_r(errnum, buf, buflen);
+
+		if (err != buf) {
+			size_t len = strlen(err);
+			char *c = mempcpy(buf, err, min(buflen - 1, len));
+			*c = '\0';
+		}
+
 		return 0;
 	}
 
-- 
 Kirill A. Shutemov

Re: [PATCH 2/2] perf: use XSI-complaint version of strerror_r() instead of GNU-specific

[Ulrich Drepper]

On Mon, Jul 23, 2012 at 5:06 PM, Kirill A. Shutemov
<kirill@shutemov.name> wrote:
> They are bugs.
>
> Let's fix strerror_r() usage.
>
> Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>

Acked-by: Ulrich Drepper <drepper@gmail.com>

Re: [PATCH 2/2] perf: use XSI-complaint version of strerror_r() instead of GNU-specific

[Namhyung Kim]

On Tue, 24 Jul 2012 00:06:54 +0300, Kirill A. Shutemov wrote:
> From 11d62205ee3c534aa9b0e9a24a312438ac726ffb Mon Sep 17 00:00:00 2001
> From: "Kirill A. Shutemov" <kirill@shutemov.name>
> Date: Mon, 23 Jul 2012 17:41:05 +0300
> Subject: [PATCH 2/2] perf: fix strerror_r() usage
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> Perf uses GNU-specific version of strerror_r(). The GNU-specific
> strerror_r() returns a pointer to a string containing the error message.
> This may be either a pointer to a string that the function stores in
> buf, or a pointer to some (immutable) static string (in which case buf
> is unused).
>
> In glibc-2.16 GNU version was marked with attribute warn_unused_result.
> It triggers few warnings in perf:
>
> util/target.c: In function ‘perf_target__strerror’:
> util/target.c:114:13: error: ignoring return value of ‘strerror_r’, declared with attribute warn_unused_result [-Werror=unused-result]
> ui/browsers/hists.c: In function ‘hist_browser__dump’:
> ui/browsers/hists.c:981:13: error: ignoring return value of ‘strerror_r’, declared with attribute warn_unused_result [-Werror=unused-result]
>
> They are bugs.
>
> Let's fix strerror_r() usage.
>

Thanks for fixing this. Just a minor nitpick below..


> Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
> ---
>  tools/perf/ui/browsers/hists.c |  4 ++--
>  tools/perf/util/target.c       | 12 +++++++++++-
>  2 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index 482f051..413bd62 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -978,8 +978,8 @@ static int hist_browser__dump(struct hist_browser *browser)
>  	fp = fopen(filename, "w");
>  	if (fp == NULL) {
>  		char bf[64];
> -		strerror_r(errno, bf, sizeof(bf));
> -		ui_helpline__fpush("Couldn't write to %s: %s", filename, bf);
> +		const char *err = strerror_r(errno, bf, sizeof(bf));
> +		ui_helpline__fpush("Couldn't write to %s: %s", filename, err);
>  		return -1;
>  	}
>  
> diff --git a/tools/perf/util/target.c b/tools/perf/util/target.c
> index 1064d5b..5c4b3b1 100644
> --- a/tools/perf/util/target.c
> +++ b/tools/perf/util/target.c
> @@ -9,6 +9,7 @@
>  #include "target.h"
>  #include "debug.h"
>  
> +#include <assert.h>
>  #include <pwd.h>
>  #include <string.h>
>  
> @@ -110,8 +111,17 @@ int perf_target__strerror(struct perf_target *target, int errnum,
>  	int idx;
>  	const char *msg;
>  
> +	assert(buflen > 0);
> +

It seems perf (and me too) prefers BUG_ON than assert:

  namhyung@sejong:perf$ git grep BUG_ON\( | wc -l
  55
  namhyung@sejong:perf$ git grep assert\( | wc -l
  16
  
It's not a big deal, though. I'm ok if others are happy with it.

Thanks,
Namhyung


>  	if (errnum >= 0) {
> -		strerror_r(errnum, buf, buflen);
> +		const char *err = strerror_r(errnum, buf, buflen);
> +
> +		if (err != buf) {
> +			size_t len = strlen(err);
> +			char *c = mempcpy(buf, err, min(buflen - 1, len));
> +			*c = '\0';
> +		}
> +
>  		return 0;
>  	}

[tip:perf/core] perf tools: use XSI-complaint version of strerror_r() instead of GNU-specific

[tip-bot for Kirill A. Shutemov]

Commit-ID:  4cc49d4dc82a39a542a31c1f51ead08a46fd33f1
Gitweb:     http://git.kernel.org/tip/4cc49d4dc82a39a542a31c1f51ead08a46fd33f1
Author:     Kirill A. Shutemov <kirill@shutemov.name>
AuthorDate: Tue, 24 Jul 2012 00:06:54 +0300
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 25 Jul 2012 11:46:04 -0300

perf tools: use XSI-complaint version of strerror_r() instead of GNU-specific

Perf uses GNU-specific version of strerror_r(). The GNU-specific strerror_r()
returns a pointer to a string containing the error message.  This may be either
a pointer to a string that the function stores in buf, or a pointer to some
(immutable) static string (in which case buf is unused).

In glibc-2.16 GNU version was marked with attribute warn_unused_result.  It
triggers few warnings in perf:

util/target.c: In function ‘perf_target__strerror’:
util/target.c:114:13: error: ignoring return value of ‘strerror_r’, declared with attribute warn_unused_result [-Werror=unused-result]
ui/browsers/hists.c: In function ‘hist_browser__dump’:
ui/browsers/hists.c:981:13: error: ignoring return value of ‘strerror_r’, declared with attribute warn_unused_result [-Werror=unused-result]

They are bugs.

Let's fix strerror_r() usage.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
Acked-by: Ulrich Drepper <drepper@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ulrich Drepper <drepper@gmail.com>
Link: http://lkml.kernel.org/r/20120723210654.GA25248@shutemov.name
[ committer note: s/assert/BUG_ON/g ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c |    4 ++--
 tools/perf/util/target.c       |   11 ++++++++++-
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 482f051..413bd62 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -978,8 +978,8 @@ static int hist_browser__dump(struct hist_browser *browser)
 	fp = fopen(filename, "w");
 	if (fp == NULL) {
 		char bf[64];
-		strerror_r(errno, bf, sizeof(bf));
-		ui_helpline__fpush("Couldn't write to %s: %s", filename, bf);
+		const char *err = strerror_r(errno, bf, sizeof(bf));
+		ui_helpline__fpush("Couldn't write to %s: %s", filename, err);
 		return -1;
 	}
 
diff --git a/tools/perf/util/target.c b/tools/perf/util/target.c
index 1064d5b..3f59c49 100644
--- a/tools/perf/util/target.c
+++ b/tools/perf/util/target.c
@@ -110,8 +110,17 @@ int perf_target__strerror(struct perf_target *target, int errnum,
 	int idx;
 	const char *msg;
 
+	BUG_ON(buflen > 0);
+
 	if (errnum >= 0) {
-		strerror_r(errnum, buf, buflen);
+		const char *err = strerror_r(errnum, buf, buflen);
+
+		if (err != buf) {
+			size_t len = strlen(err);
+			char *c = mempcpy(buf, err, min(buflen - 1, len));
+			*c = '\0';
+		}
+
 		return 0;
 	}
 

Re: [PATCH 1/2] perf: fix build error

[Arnaldo Carvalho de Melo]

Em Mon, Jul 23, 2012 at 06:00:44PM +0300, Kirill A. Shutemov escreveu:
> From: "Kirill A. Shutemov" <kirill@shutemov.name>
> 
> util/parse-events.c:29:5: error: redundant redeclaration of ‘parse_events_parse’ [-Werror=redundant-decls]
> In file included from util/parse-events.c:14:0:
> util/parse-events-bison.h:99:5: note: previous declaration of ‘parse_events_parse’ was here
> cc1: all warnings being treated as errors
> 
> Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>

Causes the build to fail for me:

[acme@sandy linux]$ make -j8 -C tools/perf/ O=/home/acme/git/build/perf install
make: Entering directory `/home/git/linux/tools/perf'
PERF_VERSION = 3.5.rc7.235.g5e5fcb
make: Leaving directory `/home/git/linux/tools/perf'
make: Entering directory `/home/git/linux/tools/perf'
    GEN perf-archive
make[1]: Entering directory `/home/git/linux/tools/lib/traceevent'
    CC /home/acme/git/build/perf/perf.o
    CC /home/acme/git/build/perf/util/parse-events.o
make[2]: warning: jobserver unavailable: using -j1.  Add `+' to parent make rule.
    CC /home/acme/git/build/perf/util/map.o
    CC /home/acme/git/build/perf/util/session.o
    CC /home/acme/git/build/perf/util/trace-event-parse.o
make[2]: `libtraceevent.a' is up to date.
make[1]: Leaving directory `/home/git/linux/tools/lib/traceevent'
cc1: warnings being treated as errors
util/parse-events.c: In function ‘parse_events__scanner’:
util/parse-events.c:701: error: implicit declaration of function ‘parse_events_parse’
util/parse-events.c:701: error: nested extern declaration of ‘parse_events_parse’
make: *** [/home/acme/git/build/perf/util/parse-events.o] Error 1
make: *** Waiting for unfinished jobs....
make: Leaving directory `/home/git/linux/tools/perf'
[acme@sandy linux]$ 

Re: [PATCH 1/2] perf: fix build error

[Kirill A. Shutemov]

On Mon, Jul 23, 2012 at 08:02:44PM +0200, Arnaldo Carvalho de Melo wrote:
> Em Mon, Jul 23, 2012 at 06:00:44PM +0300, Kirill A. Shutemov escreveu:
> > From: "Kirill A. Shutemov" <kirill@shutemov.name>
> > 
> > util/parse-events.c:29:5: error: redundant redeclaration of ‘parse_events_parse’ [-Werror=redundant-decls]
> > In file included from util/parse-events.c:14:0:
> > util/parse-events-bison.h:99:5: note: previous declaration of ‘parse_events_parse’ was here
> > cc1: all warnings being treated as errors
> > 
> > Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
> 
> Causes the build to fail for me:
> 
> [acme@sandy linux]$ make -j8 -C tools/perf/ O=/home/acme/git/build/perf install
> make: Entering directory `/home/git/linux/tools/perf'
> PERF_VERSION = 3.5.rc7.235.g5e5fcb
> make: Leaving directory `/home/git/linux/tools/perf'
> make: Entering directory `/home/git/linux/tools/perf'
>     GEN perf-archive
> make[1]: Entering directory `/home/git/linux/tools/lib/traceevent'
>     CC /home/acme/git/build/perf/perf.o
>     CC /home/acme/git/build/perf/util/parse-events.o
> make[2]: warning: jobserver unavailable: using -j1.  Add `+' to parent make rule.
>     CC /home/acme/git/build/perf/util/map.o
>     CC /home/acme/git/build/perf/util/session.o
>     CC /home/acme/git/build/perf/util/trace-event-parse.o
> make[2]: `libtraceevent.a' is up to date.
> make[1]: Leaving directory `/home/git/linux/tools/lib/traceevent'
> cc1: warnings being treated as errors
> util/parse-events.c: In function ‘parse_events__scanner’:
> util/parse-events.c:701: error: implicit declaration of function ‘parse_events_parse’
> util/parse-events.c:701: error: nested extern declaration of ‘parse_events_parse’
> make: *** [/home/acme/git/build/perf/util/parse-events.o] Error 1
> make: *** Waiting for unfinished jobs....
> make: Leaving directory `/home/git/linux/tools/perf'
> [acme@sandy linux]$ 

What bison version do you have?

-- 
 Kirill A. Shutemov

Re: [PATCH 1/2] perf: fix build error

[Arnaldo Carvalho de Melo]

Em Mon, Jul 23, 2012 at 09:16:08PM +0300, Kirill A. Shutemov escreveu:
> On Mon, Jul 23, 2012 at 08:02:44PM +0200, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Jul 23, 2012 at 06:00:44PM +0300, Kirill A. Shutemov escreveu:
> > > util/parse-events.c:29:5: error: redundant redeclaration of ‘parse_events_parse’ [-Werror=redundant-decls]
> > > util/parse-events-bison.h:99:5: note: previous declaration of ‘parse_events_parse’ was here
> > > cc1: all warnings being treated as errors

> > Causes the build to fail for me:

> > cc1: warnings being treated as errors
> > util/parse-events.c: In function ‘parse_events__scanner’:
> > util/parse-events.c:701: error: implicit declaration of function ‘parse_events_parse’
> > util/parse-events.c:701: error: nested extern declaration of ‘parse_events_parse’
> > make: *** [/home/acme/git/build/perf/util/parse-events.o] Error 1
> > make: *** Waiting for unfinished jobs....
> > make: Leaving directory `/home/git/linux/tools/perf'
> 
> What bison version do you have?

[acme@sandy linux]$ rpm -q bison
bison-2.4.1-5.el6.x86_64

Re: [PATCH 1/2] perf: fix build error

[Kirill A. Shutemov]

On Mon, Jul 23, 2012 at 08:18:49PM +0200, Arnaldo Carvalho de Melo wrote:
> Em Mon, Jul 23, 2012 at 09:16:08PM +0300, Kirill A. Shutemov escreveu:
> > On Mon, Jul 23, 2012 at 08:02:44PM +0200, Arnaldo Carvalho de Melo wrote:
> > > Em Mon, Jul 23, 2012 at 06:00:44PM +0300, Kirill A. Shutemov escreveu:
> > > > util/parse-events.c:29:5: error: redundant redeclaration of ‘parse_events_parse’ [-Werror=redundant-decls]
> > > > util/parse-events-bison.h:99:5: note: previous declaration of ‘parse_events_parse’ was here
> > > > cc1: all warnings being treated as errors
> 
> > > Causes the build to fail for me:
> 
> > > cc1: warnings being treated as errors
> > > util/parse-events.c: In function ‘parse_events__scanner’:
> > > util/parse-events.c:701: error: implicit declaration of function ‘parse_events_parse’
> > > util/parse-events.c:701: error: nested extern declaration of ‘parse_events_parse’
> > > make: *** [/home/acme/git/build/perf/util/parse-events.o] Error 1
> > > make: *** Waiting for unfinished jobs....
> > > make: Leaving directory `/home/git/linux/tools/perf'
> > 
> > What bison version do you have?
> 
> [acme@sandy linux]$ rpm -q bison
> bison-2.4.1-5.el6.x86_64
> 

Could you test this one?

>From 461531d2166574e4a3abc7ae27ebbea864d9f5ea Mon Sep 17 00:00:00 2001
From: "Kirill A. Shutemov" <kirill@shutemov.name>
Date: Mon, 23 Jul 2012 17:39:11 +0300
Subject: [PATCH] perf: fix build error
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Bison 2.6 started to generate parse_events_parse() declaration in
header. In this case we have redundant redeclaration:

util/parse-events.c:29:5: error: redundant redeclaration of ‘parse_events_parse’ [-Werror=redundant-decls]
In file included from util/parse-events.c:14:0:
util/parse-events-bison.h:99:5: note: previous declaration of ‘parse_events_parse’ was here
cc1: all warnings being treated as errors

Let's disable -Wredundant-decls for util/parse-events.c since it
includes header we can't control.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 tools/perf/Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 75d74e5..71c274f 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -803,6 +803,9 @@ $(OUTPUT)ui/browsers/map.o: ui/browsers/map.c $(OUTPUT)PERF-CFLAGS
 $(OUTPUT)util/rbtree.o: ../../lib/rbtree.c $(OUTPUT)PERF-CFLAGS
 	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DETC_PERFCONFIG='"$(ETC_PERFCONFIG_SQ)"' $<
 
+$(OUTPUT)util/parse-events.o: $(OUTPUT)util/parse-events.c $(OUTPUT)PERF-CFLAGS
+	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -Wno-redundant-decls $<
+
 $(OUTPUT)util/scripting-engines/trace-event-perl.o: util/scripting-engines/trace-event-perl.c $(OUTPUT)PERF-CFLAGS
 	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) $(PERL_EMBED_CCOPTS) -Wno-redundant-decls -Wno-strict-prototypes -Wno-unused-parameter -Wno-shadow $<
 
-- 
 Kirill A. Shutemov

Re: [PATCH 1/2] perf: fix build error

[Kirill A. Shutemov]

On Mon, Jul 23, 2012 at 10:51:46PM +0300, Kirill A. Shutemov wrote:
> On Mon, Jul 23, 2012 at 08:18:49PM +0200, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Jul 23, 2012 at 09:16:08PM +0300, Kirill A. Shutemov escreveu:
> > > On Mon, Jul 23, 2012 at 08:02:44PM +0200, Arnaldo Carvalho de Melo wrote:
> > > > Em Mon, Jul 23, 2012 at 06:00:44PM +0300, Kirill A. Shutemov escreveu:
> > > > > util/parse-events.c:29:5: error: redundant redeclaration of ‘parse_events_parse’ [-Werror=redundant-decls]
> > > > > util/parse-events-bison.h:99:5: note: previous declaration of ‘parse_events_parse’ was here
> > > > > cc1: all warnings being treated as errors
> > 
> > > > Causes the build to fail for me:
> > 
> > > > cc1: warnings being treated as errors
> > > > util/parse-events.c: In function ‘parse_events__scanner’:
> > > > util/parse-events.c:701: error: implicit declaration of function ‘parse_events_parse’
> > > > util/parse-events.c:701: error: nested extern declaration of ‘parse_events_parse’
> > > > make: *** [/home/acme/git/build/perf/util/parse-events.o] Error 1
> > > > make: *** Waiting for unfinished jobs....
> > > > make: Leaving directory `/home/git/linux/tools/perf'
> > > 
> > > What bison version do you have?
> > 
> > [acme@sandy linux]$ rpm -q bison
> > bison-2.4.1-5.el6.x86_64
> > 
> 
> Could you test this one?

Err... Sorry, it breaks build with O=...
Fixed version:

>From 14f476dddcb36bca93a50ef1a3341e2c60836e0a Mon Sep 17 00:00:00 2001
From: "Kirill A. Shutemov" <kirill@shutemov.name>
Date: Mon, 23 Jul 2012 17:39:11 +0300
Subject: [PATCH 1/2] perf: fix build error
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Bison 2.6 started to generate parse_events_parse() declaration in
header. In this case we have redundant redeclaration:

util/parse-events.c:29:5: error: redundant redeclaration of ‘parse_events_parse’ [-Werror=redundant-decls]
In file included from util/parse-events.c:14:0:
util/parse-events-bison.h:99:5: note: previous declaration of ‘parse_events_parse’ was here
cc1: all warnings being treated as errors

Let's disable -Wredundant-decls for util/parse-events.c since it
includes header we can't control.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 tools/perf/Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 75d74e5..1091192 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -803,6 +803,9 @@ $(OUTPUT)ui/browsers/map.o: ui/browsers/map.c $(OUTPUT)PERF-CFLAGS
 $(OUTPUT)util/rbtree.o: ../../lib/rbtree.c $(OUTPUT)PERF-CFLAGS
 	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DETC_PERFCONFIG='"$(ETC_PERFCONFIG_SQ)"' $<
 
+$(OUTPUT)util/parse-events.o: util/parse-events.c $(OUTPUT)PERF-CFLAGS
+	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -Wno-redundant-decls $<
+
 $(OUTPUT)util/scripting-engines/trace-event-perl.o: util/scripting-engines/trace-event-perl.c $(OUTPUT)PERF-CFLAGS
 	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) $(PERL_EMBED_CCOPTS) -Wno-redundant-decls -Wno-strict-prototypes -Wno-unused-parameter -Wno-shadow $<
 
-- 
 Kirill A. Shutemov

Re: [PATCH 1/2] perf: fix build error

[Namhyung Kim]

Hi, Kirill

On Tue, 24 Jul 2012 00:04:07 +0300, Kirill A. Shutemov wrote:
> From 14f476dddcb36bca93a50ef1a3341e2c60836e0a Mon Sep 17 00:00:00 2001
> From: "Kirill A. Shutemov" <kirill@shutemov.name>
> Date: Mon, 23 Jul 2012 17:39:11 +0300
> Subject: [PATCH 1/2] perf: fix build error
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> Bison 2.6 started to generate parse_events_parse() declaration in
> header. In this case we have redundant redeclaration:
>
> util/parse-events.c:29:5: error: redundant redeclaration of ‘parse_events_parse’ [-Werror=redundant-decls]
> In file included from util/parse-events.c:14:0:
> util/parse-events-bison.h:99:5: note: previous declaration of ‘parse_events_parse’ was here
> cc1: all warnings being treated as errors
>
> Let's disable -Wredundant-decls for util/parse-events.c since it
> includes header we can't control.
>

It'd be better if the subject line is more descriptive. Like:

  "perf tools: fix a build error with bison 2.6"

Other than that, looks good to me.

Thanks,
Namhyung


> Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
> ---
>  tools/perf/Makefile | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index 75d74e5..1091192 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -803,6 +803,9 @@ $(OUTPUT)ui/browsers/map.o: ui/browsers/map.c $(OUTPUT)PERF-CFLAGS
>  $(OUTPUT)util/rbtree.o: ../../lib/rbtree.c $(OUTPUT)PERF-CFLAGS
>  	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DETC_PERFCONFIG='"$(ETC_PERFCONFIG_SQ)"' $<
>  
> +$(OUTPUT)util/parse-events.o: util/parse-events.c $(OUTPUT)PERF-CFLAGS
> +	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -Wno-redundant-decls $<
> +
>  $(OUTPUT)util/scripting-engines/trace-event-perl.o: util/scripting-engines/trace-event-perl.c $(OUTPUT)PERF-CFLAGS
>  	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) $(PERL_EMBED_CCOPTS) -Wno-redundant-decls -Wno-strict-prototypes -Wno-unused-parameter -Wno-shadow $<

[tip:perf/core] perf tools: Fix build error with bison 2.6

[tip-bot for Kirill A. Shutemov]

Commit-ID:  043d1a5c14e95212dbf48251051804ede1ed1862
Gitweb:     http://git.kernel.org/tip/043d1a5c14e95212dbf48251051804ede1ed1862
Author:     Kirill A. Shutemov <kirill@shutemov.name>
AuthorDate: Tue, 24 Jul 2012 00:04:07 +0300
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 25 Jul 2012 11:49:19 -0300

perf tools: Fix build error with bison 2.6

Bison 2.6 started to generate parse_events_parse() declaration in header. In
this case we have redundant redeclaration:

util/parse-events.c:29:5: error: redundant redeclaration of ‘parse_events_parse’ [-Werror=redundant-decls]
In file included from util/parse-events.c:14:0:
util/parse-events-bison.h:99:5: note: previous declaration of ‘parse_events_parse’ was here
cc1: all warnings being treated as errors

Let's disable -Wredundant-decls for util/parse-events.c since it includes
header we can't control.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20120723210407.GA25186@shutemov.name
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Makefile |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index e8f0579..77f124f 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -804,6 +804,9 @@ $(OUTPUT)ui/browsers/map.o: ui/browsers/map.c $(OUTPUT)PERF-CFLAGS
 $(OUTPUT)util/rbtree.o: ../../lib/rbtree.c $(OUTPUT)PERF-CFLAGS
 	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DETC_PERFCONFIG='"$(ETC_PERFCONFIG_SQ)"' $<
 
+$(OUTPUT)util/parse-events.o: util/parse-events.c $(OUTPUT)PERF-CFLAGS
+	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -Wno-redundant-decls $<
+
 $(OUTPUT)util/scripting-engines/trace-event-perl.o: util/scripting-engines/trace-event-perl.c $(OUTPUT)PERF-CFLAGS
 	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) $(PERL_EMBED_CCOPTS) -Wno-redundant-decls -Wno-strict-prototypes -Wno-unused-parameter -Wno-shadow $<