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

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

[Jiayuan Chen]

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

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

[Jiayuan Chen]

Fixed some formatting specifiers errors, such as using %d for int and %u
for unsigned int, as well as other byte-length types.

Signed-off-by: Jiayuan Chen <mrpre@163.com>
---
 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(-)

diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index 5a4d3240689e..67a60114368f 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -670,7 +670,7 @@ static void codegen_destroy(struct bpf_object *obj, const char *obj_name)
 			continue;
 		if (bpf_map__is_internal(map) &&
 		    (bpf_map__map_flags(map) & BPF_F_MMAPABLE))
-			printf("\tskel_free_map_data(skel->%1$s, skel->maps.%1$s.initial_value, %2$zd);\n",
+			printf("\tskel_free_map_data(skel->%1$s, skel->maps.%1$s.initial_value, %2$zu);\n",
 			       ident, bpf_map_mmap_sz(map));
 		codegen("\
 			\n\
@@ -984,7 +984,7 @@ static int walk_st_ops_shadow_vars(struct btf *btf, const char *ident,
 
 		offset = m->offset / 8;
 		if (next_offset < offset)
-			printf("\t\t\tchar __padding_%d[%d];\n", i, offset - next_offset);
+			printf("\t\t\tchar __padding_%d[%u];\n", i, offset - next_offset);
 
 		switch (btf_kind(member_type)) {
 		case BTF_KIND_INT:
@@ -1052,7 +1052,7 @@ static int walk_st_ops_shadow_vars(struct btf *btf, const char *ident,
 	/* Cannot fail since it must be a struct type */
 	size = btf__resolve_size(btf, map_type_id);
 	if (next_offset < (__u32)size)
-		printf("\t\t\tchar __padding_end[%d];\n", size - next_offset);
+		printf("\t\t\tchar __padding_end[%u];\n", size - next_offset);
 
 out:
 	btf_dump__free(d);
@@ -2095,7 +2095,7 @@ btfgen_mark_type(struct btfgen_info *info, unsigned int type_id, bool follow_poi
 		break;
 	/* tells if some other type needs to be handled */
 	default:
-		p_err("unsupported kind: %s (%d)", btf_kind_str(btf_type), type_id);
+		p_err("unsupported kind: %s (%u)", btf_kind_str(btf_type), type_id);
 		return -EINVAL;
 	}
 
@@ -2147,7 +2147,7 @@ static int btfgen_record_field_relo(struct btfgen_info *info, struct bpf_core_sp
 			btf_type = btf__type_by_id(btf, type_id);
 			break;
 		default:
-			p_err("unsupported kind: %s (%d)",
+			p_err("unsupported kind: %s (%u)",
 			      btf_kind_str(btf_type), btf_type->type);
 			return -EINVAL;
 		}
@@ -2246,7 +2246,7 @@ static int btfgen_mark_type_match(struct btfgen_info *info, __u32 type_id, bool
 	}
 	/* tells if some other type needs to be handled */
 	default:
-		p_err("unsupported kind: %s (%d)", btf_kind_str(btf_type), type_id);
+		p_err("unsupported kind: %s (%u)", btf_kind_str(btf_type), type_id);
 		return -EINVAL;
 	}
 
diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
index 5cd503b763d7..52fd2c9fac56 100644
--- a/tools/bpf/bpftool/link.c
+++ b/tools/bpf/bpftool/link.c
@@ -107,7 +107,7 @@ static int link_parse_fd(int *argc, char ***argv)
 
 		fd = bpf_link_get_fd_by_id(id);
 		if (fd < 0)
-			p_err("failed to get link with ID %d: %s", id, strerror(errno));
+			p_err("failed to get link with ID %u: %s", id, strerror(errno));
 		return fd;
 	} else if (is_prefix(**argv, "pinned")) {
 		char *path;
@@ -404,7 +404,7 @@ static char *perf_config_hw_cache_str(__u64 config)
 	if (hw_cache)
 		snprintf(str, PERF_HW_CACHE_LEN, "%s-", hw_cache);
 	else
-		snprintf(str, PERF_HW_CACHE_LEN, "%lld-", config & 0xff);
+		snprintf(str, PERF_HW_CACHE_LEN, "%llu-", config & 0xff);
 
 	op = perf_event_name(evsel__hw_cache_op, (config >> 8) & 0xff);
 	if (op)
@@ -412,7 +412,7 @@ static char *perf_config_hw_cache_str(__u64 config)
 			 "%s-", op);
 	else
 		snprintf(str + strlen(str), PERF_HW_CACHE_LEN - strlen(str),
-			 "%lld-", (config >> 8) & 0xff);
+			 "%llu-", (config >> 8) & 0xff);
 
 	result = perf_event_name(evsel__hw_cache_result, config >> 16);
 	if (result)
@@ -420,7 +420,7 @@ static char *perf_config_hw_cache_str(__u64 config)
 			 "%s", result);
 	else
 		snprintf(str + strlen(str), PERF_HW_CACHE_LEN - strlen(str),
-			 "%lld", config >> 16);
+			 "%llu", config >> 16);
 	return str;
 }
 
@@ -623,7 +623,7 @@ static void show_link_ifindex_plain(__u32 ifindex)
 	else
 		snprintf(devname, sizeof(devname), "(detached)");
 	if (ret)
-		snprintf(devname, sizeof(devname), "%s(%d)",
+		snprintf(devname, sizeof(devname), "%s(%u)",
 			 tmpname, ifindex);
 	printf("ifindex %s  ", devname);
 }
@@ -699,7 +699,7 @@ void netfilter_dump_plain(const struct bpf_link_info *info)
 	if (pfname)
 		printf("\n\t%s", pfname);
 	else
-		printf("\n\tpf: %d", pf);
+		printf("\n\tpf: %u", pf);
 
 	if (hookname)
 		printf(" %s", hookname);
@@ -773,7 +773,7 @@ static void show_uprobe_multi_plain(struct bpf_link_info *info)
 	printf("func_cnt %u  ", info->uprobe_multi.count);
 
 	if (info->uprobe_multi.pid)
-		printf("pid %d  ", info->uprobe_multi.pid);
+		printf("pid %u  ", info->uprobe_multi.pid);
 
 	printf("\n\t%-16s   %-16s   %-16s", "offset", "ref_ctr_offset", "cookies");
 	for (i = 0; i < info->uprobe_multi.count; i++) {
diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index 08d0ac543c67..cd5963cb6058 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -152,7 +152,7 @@ static int do_version(int argc, char **argv)
 			     BPFTOOL_MINOR_VERSION, BPFTOOL_PATCH_VERSION);
 #endif
 		jsonw_name(json_wtr, "libbpf_version");
-		jsonw_printf(json_wtr, "\"%d.%d\"",
+		jsonw_printf(json_wtr, "\"%u.%u\"",
 			     libbpf_major_version(), libbpf_minor_version());
 
 		jsonw_name(json_wtr, "features");
@@ -370,7 +370,7 @@ static int do_batch(int argc, char **argv)
 		while ((cp = strstr(buf, "\\\n")) != NULL) {
 			if (!fgets(contline, sizeof(contline), fp) ||
 			    strlen(contline) == 0) {
-				p_err("missing continuation line on command %d",
+				p_err("missing continuation line on command %u",
 				      lines);
 				err = -1;
 				goto err_close;
@@ -381,7 +381,7 @@ static int do_batch(int argc, char **argv)
 				*cp = '\0';
 
 			if (strlen(buf) + strlen(contline) + 1 > sizeof(buf)) {
-				p_err("command %d is too long", lines);
+				p_err("command %u is too long", lines);
 				err = -1;
 				goto err_close;
 			}
@@ -423,7 +423,7 @@ static int do_batch(int argc, char **argv)
 		err = -1;
 	} else {
 		if (!json_output)
-			printf("processed %d commands\n", lines);
+			printf("processed %u commands\n", lines);
 	}
 err_close:
 	if (fp != stdin)
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index b89bd792c1d5..ed4a9bd82931 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -285,7 +285,7 @@ static void print_entry_plain(struct bpf_map_info *info, unsigned char *key,
 		}
 		if (info->value_size) {
 			for (i = 0; i < n; i++) {
-				printf("value (CPU %02d):%c",
+				printf("value (CPU %02u):%c",
 				       i, info->value_size > 16 ? '\n' : ' ');
 				fprint_hex(stdout, value + i * step,
 					   info->value_size, " ");
@@ -316,7 +316,7 @@ static char **parse_bytes(char **argv, const char *name, unsigned char *val,
 	}
 
 	if (i != n) {
-		p_err("%s expected %d bytes got %d", name, n, i);
+		p_err("%s expected %u bytes got %u", name, n, i);
 		return NULL;
 	}
 
@@ -462,7 +462,7 @@ static void show_map_header_json(struct bpf_map_info *info, json_writer_t *wtr)
 		jsonw_string_field(wtr, "name", info->name);
 
 	jsonw_name(wtr, "flags");
-	jsonw_printf(wtr, "%d", info->map_flags);
+	jsonw_printf(wtr, "%u", info->map_flags);
 }
 
 static int show_map_close_json(int fd, struct bpf_map_info *info)
@@ -588,7 +588,7 @@ static int show_map_close_plain(int fd, struct bpf_map_info *info)
 			if (prog_type_str)
 				printf("owner_prog_type %s  ", prog_type_str);
 			else
-				printf("owner_prog_type %d  ", prog_type);
+				printf("owner_prog_type %u  ", prog_type);
 		}
 		if (owner_jited)
 			printf("owner%s jited",
@@ -615,7 +615,7 @@ static int show_map_close_plain(int fd, struct bpf_map_info *info)
 		printf("\n\t");
 
 	if (info->btf_id)
-		printf("btf_id %d", info->btf_id);
+		printf("btf_id %u", info->btf_id);
 
 	if (frozen)
 		printf("%sfrozen", info->btf_id ? "  " : "");
-- 
2.43.5

Re: [PATCH bpf-next v1 1/1] bpftool: Using the right format specifiers

[Quentin Monnet]

On 07/02/2025 12:37, Jiayuan Chen wrote:
> Fixed some formatting specifiers errors, such as using %d for int and %u
> for unsigned int, as well as other byte-length types.
> 
> Signed-off-by: Jiayuan Chen <mrpre@163.com>

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

Thank you.

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

[Quentin Monnet]

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

[Jiayuan Chen]

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

[Quentin Monnet]

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.

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

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Fri,  7 Feb 2025 20:37:05 +0800 you 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.
> 
> [...]

Here is the summary with links:
  - [bpf-next,v1,1/1] bpftool: Using the right format specifiers
    https://git.kernel.org/bpf/bpf-next/c/17c3dc50294b

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html