| Message ID | 20170616145907.baba73eda242efe194e072d0@arm.com |
|---|---|
| State | New |
| Headers | show |
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 >
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
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
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
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 --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;
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(-)