[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <576a50c8-9ca2-4e2f-9bd8-7d9be4862920@linaro.org>
Date: Tue, 7 Jan 2025 10:33:03 +0000
From: James Clark <james.clark@...aro.org>
To: Ian Rogers <irogers@...gle.com>, Leo Yan <leo.yan@....com>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Namhyung Kim <namhyung@...nel.org>, Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>, Adrian Hunter <adrian.hunter@...el.com>,
Kan Liang <kan.liang@...ux.intel.com>, linux-perf-users@...r.kernel.org,
bpf@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] tools build: Fix a number of Wconversion warnings
On 06/01/2025 9:54 pm, Ian Rogers wrote:
> There's some expressed interest in having the compiler flag
> -Wconversion detect at build time certain kinds of potential problems:
> https://lore.kernel.org/lkml/20250103182532.GB781381@e132581.arm.com/
>
> As feature detection passes -Wconversion from CFLAGS when set, the
> feature detection compile tests need to not fail because of
> -Wconversion as the failure will be interpretted as a missing
> feature. Switch various types to avoid the -Wconversion issue, the
> exact meaning of the code is unimportant as it is typically looking
> for header file definitions.
>
> Signed-off-by: Ian Rogers <irogers@...gle.com>
What's the plan for errors in #includes that we can't modify? I noticed
the Perl feature test fails with -Wconversion but can be fixed by
disabling the warning:
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wsign-conversion"
#pragma GCC diagnostic ignored "-Wconversion"
#include <EXTERN.h>
#include <perl.h>
#pragma GCC diagnostic pop
Not sure why it needs both those things to be disabled when I only
enabled -Wconversion, but it does.
> ---
> tools/build/feature/test-backtrace.c | 2 +-
> tools/build/feature/test-bpf.c | 2 +-
> tools/build/feature/test-glibc.c | 2 +-
> tools/build/feature/test-libdebuginfod.c | 2 +-
> tools/build/feature/test-libdw.c | 2 +-
> tools/build/feature/test-libelf-gelf_getnote.c | 2 +-
> tools/build/feature/test-libelf.c | 2 +-
> tools/build/feature/test-lzma.c | 2 +-
> 8 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/tools/build/feature/test-backtrace.c b/tools/build/feature/test-backtrace.c
> index e9ddd27c69c3..7962fbad6401 100644
> --- a/tools/build/feature/test-backtrace.c
> +++ b/tools/build/feature/test-backtrace.c
> @@ -5,7 +5,7 @@
> int main(void)
> {
> void *backtrace_fns[10];
> - size_t entries;
> + int entries;
>
> entries = backtrace(backtrace_fns, 10);
> backtrace_symbols_fd(backtrace_fns, entries, 1);
> diff --git a/tools/build/feature/test-bpf.c b/tools/build/feature/test-bpf.c
> index 727d22e34a6e..e7a405f83af6 100644
> --- a/tools/build/feature/test-bpf.c
> +++ b/tools/build/feature/test-bpf.c
> @@ -44,5 +44,5 @@ int main(void)
> * Test existence of __NR_bpf and BPF_PROG_LOAD.
> * This call should fail if we run the testcase.
> */
> - return syscall(__NR_bpf, BPF_PROG_LOAD, &attr, sizeof(attr));
> + return syscall(__NR_bpf, BPF_PROG_LOAD, &attr, sizeof(attr)) == 0;
Seems a bit weird to invert some of the return values rather than doing
!= 0, but as you say, the actual values seem to be unimportant.
Reviewed-by: James Clark <james.clark@...aro.org>
> }
> diff --git a/tools/build/feature/test-glibc.c b/tools/build/feature/test-glibc.c
> index 9ab8e90e7b88..20a250419f31 100644
> --- a/tools/build/feature/test-glibc.c
> +++ b/tools/build/feature/test-glibc.c
> @@ -16,5 +16,5 @@ int main(void)
> const char *version = XSTR(__GLIBC__) "." XSTR(__GLIBC_MINOR__);
> #endif
>
> - return (long)version;
> + return version == NULL;
> }
> diff --git a/tools/build/feature/test-libdebuginfod.c b/tools/build/feature/test-libdebuginfod.c
> index da22548b8413..823f9fa9391d 100644
> --- a/tools/build/feature/test-libdebuginfod.c
> +++ b/tools/build/feature/test-libdebuginfod.c
> @@ -4,5 +4,5 @@
> int main(void)
> {
> debuginfod_client* c = debuginfod_begin();
> - return (long)c;
> + return !!c;
> }
> diff --git a/tools/build/feature/test-libdw.c b/tools/build/feature/test-libdw.c
> index 2fb59479ab77..aabd63ca76b4 100644
> --- a/tools/build/feature/test-libdw.c
> +++ b/tools/build/feature/test-libdw.c
> @@ -9,7 +9,7 @@ int test_libdw(void)
> {
> Dwarf *dbg = dwarf_begin(0, DWARF_C_READ);
>
> - return (long)dbg;
> + return dbg == NULL;
> }
>
> int test_libdw_unwind(void)
> diff --git a/tools/build/feature/test-libelf-gelf_getnote.c b/tools/build/feature/test-libelf-gelf_getnote.c
> index 075d062fe841..e06121161161 100644
> --- a/tools/build/feature/test-libelf-gelf_getnote.c
> +++ b/tools/build/feature/test-libelf-gelf_getnote.c
> @@ -4,5 +4,5 @@
>
> int main(void)
> {
> - return gelf_getnote(NULL, 0, NULL, NULL, NULL);
> + return gelf_getnote(NULL, 0, NULL, NULL, NULL) == 0;
> }
> diff --git a/tools/build/feature/test-libelf.c b/tools/build/feature/test-libelf.c
> index 905044127d56..2dbb6ea870f3 100644
> --- a/tools/build/feature/test-libelf.c
> +++ b/tools/build/feature/test-libelf.c
> @@ -5,5 +5,5 @@ int main(void)
> {
> Elf *elf = elf_begin(0, ELF_C_READ, 0);
>
> - return (long)elf;
> + return !!elf;
> }
> diff --git a/tools/build/feature/test-lzma.c b/tools/build/feature/test-lzma.c
> index 78682bb01d57..b57103774e8e 100644
> --- a/tools/build/feature/test-lzma.c
> +++ b/tools/build/feature/test-lzma.c
> @@ -4,7 +4,7 @@
> int main(void)
> {
> lzma_stream strm = LZMA_STREAM_INIT;
> - int ret;
> + lzma_ret ret;
>
> ret = lzma_stream_decoder(&strm, UINT64_MAX, LZMA_CONCATENATED);
> return ret ? -1 : 0;
Powered by blists - more mailing lists