[PATCH bpf-next v1 0/1] Using the right format specifiers for bpftool

Jiayuan Chen posted 1 patch 1 year ago
tools/bpf/bpftool/gen.c  | 12 ++++++------
tools/bpf/bpftool/link.c | 14 +++++++-------
tools/bpf/bpftool/main.c |  8 ++++----
tools/bpf/bpftool/map.c  | 10 +++++-----
4 files changed, 22 insertions(+), 22 deletions(-)
[PATCH bpf-next v1 0/1] Using the right format specifiers for bpftool
Posted by Jiayuan Chen 1 year ago
Fixed some incorrect formatting specifiers that were exposed when I added
the "-Wformat" flag to the compiler options.

This patch doesn't include "-Wformat" in the Makefile for now, as I've
only addressed some obvious semantic issues with the compiler warnings.
There are still other warnings that need to be tackled.

For example, there's an ifindex that's sometimes defined as a signed type
and sometimes as an unsigned type, which makes formatting a real pain
- sometimes it needs %d and sometimes %u. This might require a more
fundamental fix from the variable definition side.

If the maintainer is okay with adding "-Wformat" to the
tools/bpf/bpftool/Makefile, please let us know, and we can follow up with
further fixes.

Jiayuan Chen (1):
  bpftool: Using the right format specifiers

 tools/bpf/bpftool/gen.c  | 12 ++++++------
 tools/bpf/bpftool/link.c | 14 +++++++-------
 tools/bpf/bpftool/main.c |  8 ++++----
 tools/bpf/bpftool/map.c  | 10 +++++-----
 4 files changed, 22 insertions(+), 22 deletions(-)

-- 
2.43.5
Re: [PATCH bpf-next v1 0/1] Using the right format specifiers for bpftool
Posted by Quentin Monnet 1 year ago
On 07/02/2025 12:37, Jiayuan Chen wrote:
> Fixed some incorrect formatting specifiers that were exposed when I added
> the "-Wformat" flag to the compiler options.
> 
> This patch doesn't include "-Wformat" in the Makefile for now, as I've
> only addressed some obvious semantic issues with the compiler warnings.
> There are still other warnings that need to be tackled.
> 
> For example, there's an ifindex that's sometimes defined as a signed type
> and sometimes as an unsigned type, which makes formatting a real pain
> - sometimes it needs %d and sometimes %u. This might require a more
> fundamental fix from the variable definition side.
> 
> If the maintainer is okay with adding "-Wformat" to the
> tools/bpf/bpftool/Makefile, please let us know, and we can follow up with
> further fixes.

No objection from the maintainer, thanks for looking into this. Did you
catch these issues with just "-Wformat"? I'm asking because I need to
use an additional flag, "-Wformat-signedness", to have my compiler
display the %d/%u reports.

Thanks,
Quentin
Re: [PATCH bpf-next v1 0/1] Using the right format specifiers for bpftool
Posted by Jiayuan Chen 1 year ago
On Fri, Feb 07, 2025 at 01:22:19PM +0000, Quentin Monnet wrote:
> On 07/02/2025 12:37, Jiayuan Chen wrote:
> > Fixed some incorrect formatting specifiers that were exposed when I added
> > the "-Wformat" flag to the compiler options.
> > 
> > This patch doesn't include "-Wformat" in the Makefile for now, as I've
> > only addressed some obvious semantic issues with the compiler warnings.
> > There are still other warnings that need to be tackled.
> > 
> > For example, there's an ifindex that's sometimes defined as a signed type
> > and sometimes as an unsigned type, which makes formatting a real pain
> > - sometimes it needs %d and sometimes %u. This might require a more
> > fundamental fix from the variable definition side.
> > 
> > If the maintainer is okay with adding "-Wformat" to the
> > tools/bpf/bpftool/Makefile, please let us know, and we can follow up with
> > further fixes.
> 
> No objection from the maintainer, thanks for looking into this. Did you
> catch these issues with just "-Wformat"? I'm asking because I need to
> use an additional flag, "-Wformat-signedness", to have my compiler
> display the %d/%u reports.
> 
> Thanks,
> Quentin
Yes, I previously added '-Wformat -Wformat-signedness', but I just tried
again and it turns out that only '-Wformat-signedness' takes effect.
Re: [PATCH bpf-next v1 0/1] Using the right format specifiers for bpftool
Posted by Quentin Monnet 1 year ago
2025-02-07 22:27 UTC+0800 ~ Jiayuan Chen <mrpre@163.com>
> On Fri, Feb 07, 2025 at 01:22:19PM +0000, Quentin Monnet wrote:
>> On 07/02/2025 12:37, Jiayuan Chen wrote:
>>> Fixed some incorrect formatting specifiers that were exposed when I added
>>> the "-Wformat" flag to the compiler options.
>>>
>>> This patch doesn't include "-Wformat" in the Makefile for now, as I've
>>> only addressed some obvious semantic issues with the compiler warnings.
>>> There are still other warnings that need to be tackled.
>>>
>>> For example, there's an ifindex that's sometimes defined as a signed type
>>> and sometimes as an unsigned type, which makes formatting a real pain
>>> - sometimes it needs %d and sometimes %u. This might require a more
>>> fundamental fix from the variable definition side.
>>>
>>> If the maintainer is okay with adding "-Wformat" to the
>>> tools/bpf/bpftool/Makefile, please let us know, and we can follow up with
>>> further fixes.
>>
>> No objection from the maintainer, thanks for looking into this. Did you
>> catch these issues with just "-Wformat"? I'm asking because I need to
>> use an additional flag, "-Wformat-signedness", to have my compiler
>> display the %d/%u reports.
>>
>> Thanks,
>> Quentin
> Yes, I previously added '-Wformat -Wformat-signedness', but I just tried
> again and it turns out that only '-Wformat-signedness' takes effect.


I rememeber now that -Wformat is already included in bpftool's Makefile
via tools/scripts/Makefile.include (variable $(EXTRA_WARNINGS)), so it
wouldn't make a difference anyway whether you add it again in bpftool's
Makefile or not.