Re: [PATCH v1] tools build: Fix a number of Wconversion warnings

[James Clark <james.clark@xxxxxxxxxx>]

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@xxxxxxxxxxxxxxx/
>
> 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@xxxxxxxxxx>

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@xxxxxxxxxx>

>   }
> 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;