diff mbox

[perf/core,branch] perf coresight: Fix ARM builds caused by misplaced __printf

Message ID 20170616145907.baba73eda242efe194e072d0@arm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kim Phillips June 16, 2017, 7:59 p.m. UTC
Trailing __printf attributes work for function declarations, but not
definitions.  This patch fixes arm32/64 builds by placing __printf
before the declarator.  Otherwise this happens:

arch/arm64/util/../../arm/util/cs-etm.c:586:1: error: attributes should be specified before the declarator in a function definition
 static int cs_device__print_file(const char *name, const char *fmt, ...) __printf(2, 3)
 ^~~~~~
arch/arm64/util/../../arm/util/cs-etm.c: In function ‘cs_etm_set_drv_config’:
arch/arm64/util/../../arm/util/cs-etm.c:610:8: error: implicit declaration of function ‘cs_device__print_file’ [-Werror=implicit-function-declaration]
  ret = cs_device__print_file(enable_sink, "%d", 1);
        ^~~~~~~~~~~~~~~~~~~~~
arch/arm64/util/../../arm/util/cs-etm.c:610:2: error: nested extern declaration of ‘cs_device__print_file’ [-Werror=nested-externs]
  ret = cs_device__print_file(enable_sink, "%d", 1);
  ^~~
At top level:
arch/arm64/util/../../arm/util/cs-etm.c:566:14: error: ‘cs_device__open_file’ defined but not used [-Werror=unused-function]
 static FILE *cs_device__open_file(const char *name)
              ^~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

Fixes: 2ee261d962ac "tools: Adopt __printf from kernel sources"
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Kim Phillips <kim.phillips@arm.com>
---
Applies to acme's perf/core branch

 tools/perf/arch/arm/util/cs-etm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Mathieu Poirier June 19, 2017, 5:51 p.m. UTC | #1
On 16 June 2017 at 13:59, Kim Phillips <kim.phillips@arm.com> wrote:
> Trailing __printf attributes work for function declarations, but not
> definitions.  This patch fixes arm32/64 builds by placing __printf
> before the declarator.  Otherwise this happens:
>
> arch/arm64/util/../../arm/util/cs-etm.c:586:1: error: attributes should be specified before the declarator in a function definition
>  static int cs_device__print_file(const char *name, const char *fmt, ...) __printf(2, 3)
>  ^~~~~~
> arch/arm64/util/../../arm/util/cs-etm.c: In function ‘cs_etm_set_drv_config’:
> arch/arm64/util/../../arm/util/cs-etm.c:610:8: error: implicit declaration of function ‘cs_device__print_file’ [-Werror=implicit-function-declaration]
>   ret = cs_device__print_file(enable_sink, "%d", 1);
>         ^~~~~~~~~~~~~~~~~~~~~
> arch/arm64/util/../../arm/util/cs-etm.c:610:2: error: nested extern declaration of ‘cs_device__print_file’ [-Werror=nested-externs]
>   ret = cs_device__print_file(enable_sink, "%d", 1);
>   ^~~
> At top level:
> arch/arm64/util/../../arm/util/cs-etm.c:566:14: error: ‘cs_device__open_file’ defined but not used [-Werror=unused-function]
>  static FILE *cs_device__open_file(const char *name)
>               ^~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
>
> Fixes: 2ee261d962ac "tools: Adopt __printf from kernel sources"
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Kim Phillips <kim.phillips@arm.com>
> ---
> Applies to acme's perf/core branch
>
>  tools/perf/arch/arm/util/cs-etm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
> index 90a6f42ba904..7ce3d1a25133 100644
> --- a/tools/perf/arch/arm/util/cs-etm.c
> +++ b/tools/perf/arch/arm/util/cs-etm.c
> @@ -583,7 +583,7 @@ static FILE *cs_device__open_file(const char *name)
>
>  }
>
> -static int cs_device__print_file(const char *name, const char *fmt, ...) __printf(2, 3)
> +static int __printf(2, 3) cs_device__print_file(const char *name, const char *fmt, ...)
>  {
>         va_list args;
>         FILE *file;

I just tested Kim's solution on my side.

Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>

> --
> 2.11.0
>
Arnaldo Carvalho de Melo June 19, 2017, 6:42 p.m. UTC | #2
Em Mon, Jun 19, 2017 at 11:51:20AM -0600, Mathieu Poirier escreveu:
> On 16 June 2017 at 13:59, Kim Phillips <kim.phillips@arm.com> wrote:
> > -static int cs_device__print_file(const char *name, const char *fmt, ...) __printf(2, 3)
> > +static int __printf(2, 3) cs_device__print_file(const char *name, const char *fmt, ...)
> >  {
> >         va_list args;
> >         FILE *file;
> 
> I just tested Kim's solution on my side.
> 
> Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>

Thanks for checking, since I haven't pushed this to Ingo I just squashed
Kim's fix into the buggy cset.

Now I'm trying to build it with lots of cross build containers to see if
there are any other problems before push this up to Ingo.

- Arnaldo
Kim Phillips June 19, 2017, 7:26 p.m. UTC | #3
On Mon, 19 Jun 2017 15:42:09 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Mon, Jun 19, 2017 at 11:51:20AM -0600, Mathieu Poirier escreveu:
> > On 16 June 2017 at 13:59, Kim Phillips <kim.phillips@arm.com> wrote:
> > > -static int cs_device__print_file(const char *name, const char *fmt, ...) __printf(2, 3)
> > > +static int __printf(2, 3) cs_device__print_file(const char *name, const char *fmt, ...)
> > >  {
> > >         va_list args;
> > >         FILE *file;
> > 
> > I just tested Kim's solution on my side.
> > 
> > Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> 
> Thanks for checking, since I haven't pushed this to Ingo I just squashed
> Kim's fix into the buggy cset.
> 
> Now I'm trying to build it with lots of cross build containers to see if
> there are any other problems before push this up to Ingo.

Thanks, I'm still trying to learn the submission process...

Slightly off-topic, but when you push kernel.org/.../acme/linux.git, do
you push --tags?

If I do a fresh clone, checkout perf/core or urgent, build perf,
PERF-VERSION-GEN first uses git to find the version number, then the
Makefile.  But in acme/linux.git, the latest version tag looks to match
the master branch version: 3.2: so I get a 3.2-reporting version on
what should be 4.12-rc4 (perf/core's current Makefile):

$ tools/perf/perf --version
perf version 3.2.gd15e591

Is this an acme/linux.git tree maintenance issue, or should
PERF-VERSION-GEN be modified to compare versions gotten from git vs.
the Makefile, and just use the higher one?

Thanks,

Kim
Arnaldo Carvalho de Melo June 19, 2017, 7:52 p.m. UTC | #4
Em Mon, Jun 19, 2017 at 02:26:33PM -0500, Kim Phillips escreveu:
> On Mon, 19 Jun 2017 15:42:09 -0300
> Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> 
> > Em Mon, Jun 19, 2017 at 11:51:20AM -0600, Mathieu Poirier escreveu:
> > > On 16 June 2017 at 13:59, Kim Phillips <kim.phillips@arm.com> wrote:
> > > > -static int cs_device__print_file(const char *name, const char *fmt, ...) __printf(2, 3)
> > > > +static int __printf(2, 3) cs_device__print_file(const char *name, const char *fmt, ...)
> > > >  {
> > > >         va_list args;
> > > >         FILE *file;
> > > 
> > > I just tested Kim's solution on my side.
> > > 
> > > Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> > 
> > Thanks for checking, since I haven't pushed this to Ingo I just squashed
> > Kim's fix into the buggy cset.
> > 
> > Now I'm trying to build it with lots of cross build containers to see if
> > there are any other problems before push this up to Ingo.
> 
> Thanks, I'm still trying to learn the submission process...
> 
> Slightly off-topic, but when you push kernel.org/.../acme/linux.git, do
> you push --tags?

I usually just do:

git push acme.korg perf/urgent

or

git push acme.korg perf/urgent

[acme@jouet linux]$ git remote -v | grep korg
acme.korg	gitolite@gitolite.kernel.org:/pub/scm/linux/kernel/git/acme/linux.git (fetch)
acme.korg	gitolite@gitolite.kernel.org:/pub/scm/linux/kernel/git/acme/linux.git (push)
[acme@jouet linux]$ 

Or some other branch, acme/master isn't updated. I guess I should do
like tip/master, and always have acme/master with acme/perf/core merged
with acme/perf/urgent, but for now, just consider acme/perf/urgent and
acme/perf/core.
 
> If I do a fresh clone, checkout perf/core or urgent, build perf,
> PERF-VERSION-GEN first uses git to find the version number, then the
> Makefile.  But in acme/linux.git, the latest version tag looks to match
> the master branch version: 3.2: so I get a 3.2-reporting version on
> what should be 4.12-rc4 (perf/core's current Makefile):
> 
> $ tools/perf/perf --version
> perf version 3.2.gd15e591
> 
> Is this an acme/linux.git tree maintenance issue, or should
> PERF-VERSION-GEN be modified to compare versions gotten from git vs.
> the Makefile, and just use the higher one?
> 
> Thanks,
> 
> Kim
Kim Phillips June 19, 2017, 8:29 p.m. UTC | #5
On Mon, 19 Jun 2017 16:52:22 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Mon, Jun 19, 2017 at 02:26:33PM -0500, Kim Phillips escreveu:
> > On Mon, 19 Jun 2017 15:42:09 -0300
> > Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > 
> > > Em Mon, Jun 19, 2017 at 11:51:20AM -0600, Mathieu Poirier escreveu:
> > > > On 16 June 2017 at 13:59, Kim Phillips <kim.phillips@arm.com> wrote:
> > > > > -static int cs_device__print_file(const char *name, const char *fmt, ...) __printf(2, 3)
> > > > > +static int __printf(2, 3) cs_device__print_file(const char *name, const char *fmt, ...)
> > > > >  {
> > > > >         va_list args;
> > > > >         FILE *file;
> > > > 
> > > > I just tested Kim's solution on my side.
> > > > 
> > > > Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> > > 
> > > Thanks for checking, since I haven't pushed this to Ingo I just squashed
> > > Kim's fix into the buggy cset.
> > > 
> > > Now I'm trying to build it with lots of cross build containers to see if
> > > there are any other problems before push this up to Ingo.
> > 
> > Thanks, I'm still trying to learn the submission process...
> > 
> > Slightly off-topic, but when you push kernel.org/.../acme/linux.git, do
> > you push --tags?
> 
> I usually just do:
> 
> git push acme.korg perf/urgent
> 
> or
> 
> git push acme.korg perf/urgent
> 
> [acme@jouet linux]$ git remote -v | grep korg
> acme.korg	gitolite@gitolite.kernel.org:/pub/scm/linux/kernel/git/acme/linux.git (fetch)
> acme.korg	gitolite@gitolite.kernel.org:/pub/scm/linux/kernel/git/acme/linux.git (push)
> [acme@jouet linux]$ 
> 
> Or some other branch, acme/master isn't updated. I guess I should do
> like tip/master, and always have acme/master with acme/perf/core merged
> with acme/perf/urgent, but for now, just consider acme/perf/urgent and
> acme/perf/core.

I'm not particularly concerned about the master branch being
up-to-date, I'm more concerned about the repo's tags being up to date,
so perf --version will work correctly.

If you don't mind publishing all your local tags, and you don't want to
'push --tags' every time, adding this line to acme.korg's remote config
section will do it automatically:

push = +refs/tags/*

Thanks,

Kim

>  
> > If I do a fresh clone, checkout perf/core or urgent, build perf,
> > PERF-VERSION-GEN first uses git to find the version number, then the
> > Makefile.  But in acme/linux.git, the latest version tag looks to match
> > the master branch version: 3.2: so I get a 3.2-reporting version on
> > what should be 4.12-rc4 (perf/core's current Makefile):
> > 
> > $ tools/perf/perf --version
> > perf version 3.2.gd15e591
> > 
> > Is this an acme/linux.git tree maintenance issue, or should
> > PERF-VERSION-GEN be modified to compare versions gotten from git vs.
> > the Makefile, and just use the higher one?
> > 
> > Thanks,
> > 
> > Kim
diff mbox

Patch

diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
index 90a6f42ba904..7ce3d1a25133 100644
--- a/tools/perf/arch/arm/util/cs-etm.c
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -583,7 +583,7 @@  static FILE *cs_device__open_file(const char *name)
 
 }
 
-static int cs_device__print_file(const char *name, const char *fmt, ...) __printf(2, 3)
+static int __printf(2, 3) cs_device__print_file(const char *name, const char *fmt, ...)
 {
 	va_list args;
 	FILE *file;