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@linux.dev>
---
 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
 CFLAGS += $(filter-out -Wswitch-enum -Wnested-externs,$(EXTRA_WARNINGS))
 CFLAGS += -DPACKAGE='"bpftool"' -D__EXPORTED_HEADERS__ \
 	-I$(or $(OUTPUT),.) \
-- 
2.47.1

2025-03-10 22:20 UTC+0800 ~ Jiayuan Chen <jiayuan.chen@linux.dev>
> 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@linux.dev>


Acked-by: Quentin Monnet <qmo@kernel.org>

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),.) \

March 10, 2025 at 23:49, "Quentin Monnet" <qmo@kernel.org> wrote:

> 
> 2025-03-10 22:20 UTC+0800 ~ Jiayuan Chen <jiayuan.chen@linux.dev>
> 
> > 
> > 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@linux.dev>
> > 
> 
> Acked-by: Quentin Monnet <qmo@kernel.org>
> 
> 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.
> 

Thanks for reminding me of these.

I tried adding it to Makefile.include, and indeed there were a lot of warnings,
but this part is not part of bpftool. I'll take a look later and try to enable this
options to fix these warnings as well. For now, I'll only fix the warnings related
to bpftool and these changes can also be easily synced to the bpftool on github.

> > ---
> > 
> >  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
> 
>  ...
OK，I will do this.