Re: [PATCH bpf-next v1 1/2] bpftool: Add -Wformat-signedness flag to detect format errors

[Quentin Monnet]

2025-03-10 22:20 UTC+0800 ~ Jiayuan Chen <jiayuan.chen@xxxxxxxxx>
> This commit adds the -Wformat-signedness compiler flag to detect and
> prevent printf format errors, where signed or unsigned types are
> mismatched with format specifiers. This helps to catch potential issues at
> compile-time, ensuring that our code is more robust and reliable. With
> this flag, the compiler will now warn about incorrect format strings, such
> as using %d with unsigned types or %u with signed types.
> 
> Signed-off-by: Jiayuan Chen <jiayuan.chen@xxxxxxxxx>


Acked-by: Quentin Monnet <qmo@xxxxxxxxxx>

Thanks for that. Have you looked into enabling the flag along with the
other EXTRA_WARNINGS in tools/scripts/Makefile.include? It would be
ideal to have it there, but I suppose it raises too many warnings across
tools/? (I didn't try myself.) No objection to taking it in bpftool only.


> ---
>  tools/bpf/bpftool/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> index dd9f3ec84201..d9f3eb51a48f 100644
> --- a/tools/bpf/bpftool/Makefile
> +++ b/tools/bpf/bpftool/Makefile
> @@ -71,7 +71,7 @@ prefix ?= /usr/local
>  bash_compdir ?= /usr/share/bash-completion/completions
>  
>  CFLAGS += -O2
> -CFLAGS += -W -Wall -Wextra -Wno-unused-parameter -Wno-missing-field-initializers
> +CFLAGS += -W -Wall -Wextra -Wformat-signedness -Wno-unused-parameter -Wno-missing-field-initializers


Nit: This line is becoming long enough that I'd consider moving each
flag to its own line, for better reading:

	CFLAGS += -W
	CFLAGS += -Wall
	CFLAGS += -Wextra
	CFLAGS += -Wformat-signedness
	...

>  CFLAGS += $(filter-out -Wswitch-enum -Wnested-externs,$(EXTRA_WARNINGS))
>  CFLAGS += -DPACKAGE='"bpftool"' -D__EXPORTED_HEADERS__ \
>  	-I$(or $(OUTPUT),.) \