Hi Mehdi, You are trying to do much with the patch series. I don't think it will help much as reviewers will give comments and you will revise the patches. This loop will continue forever.
I totally agree with Daniel. Please write a proper commit message. While writing a commit message or creating a patch.Please try to give the answers of the following questions. 1) What is the issue which your patch resolves? 2) Are you trying to do more than one thing at a time? Please don't. 3) if a patch modifies more than one file then either these changes inter link with each other or they have similar kind of work. ~~Vivek On Thu, Sep 25, 2025 at 9:04 PM Mehdi Ben Hadj Khelifa <[email protected]> wrote: > > On 9/25/25 4:04 PM, Daniel Borkmann wrote: > > On 9/25/25 12:35 PM, Mehdi Ben Hadj Khelifa wrote: > >> -Change only variable types for correct sign comparisons > >> > >> Signed-off-by: Mehdi Ben Hadj Khelifa <[email protected]> > > > > Pls write some better commit messages and not just copy/paste the same > > $subj/ > > message every time; proper sentences w/o the weird " -" indent. > > Understood, though the changes are very similar / are the same with the > same goal that's why it made sense to me to do that and I will remove > the - in future commits.> Also say > > why > > this is needed in the commit message, and add a reference to the commit > > which > > initially added this as a TODO, i.e. 495d2d8133fd ("selftests/bpf: > > Attempt to > > build BPF programs with -Wsign-compare"). > I will do that in the upcoming version. > > > If you group these, then maybe > > also > > include the parts of the compiler-emitted warnings during build which are > > relevant to the code changes you do here. > > Okay. I will do that. Should i send a v4 with the recommended changes > but not including the rest of the files meaning the ones that I haven't > uploaded in this patch series which contain type casting or should i > just make changes for these files in this series? > Also will it be better if dropped these versions and made a new patch > with v1? > > Thank you for your review and time Daniel. > Regards, > Mehdi > >> --- > >> tools/testing/selftests/bpf/progs/test_xdp_dynptr.c | 2 +- > >> tools/testing/selftests/bpf/progs/test_xdp_loop.c | 2 +- > >> tools/testing/selftests/bpf/progs/test_xdp_noinline.c | 4 ++-- > >> tools/testing/selftests/bpf/progs/uprobe_multi.c | 4 ++-- > >> .../selftests/bpf/progs/uprobe_multi_session_recursive.c | 5 +++-- > >> .../selftests/bpf/progs/verifier_iterating_callbacks.c | 2 +- > >> 6 files changed, 10 insertions(+), 9 deletions(-) > >> > >> diff --git a/tools/testing/selftests/bpf/progs/test_xdp_dynptr.c b/ > >> tools/testing/selftests/bpf/progs/test_xdp_dynptr.c > >> index 67a77944ef29..12ad0ec91021 100644 > >> --- a/tools/testing/selftests/bpf/progs/test_xdp_dynptr.c > >> +++ b/tools/testing/selftests/bpf/progs/test_xdp_dynptr.c > >> @@ -89,7 +89,7 @@ static __always_inline int handle_ipv4(struct xdp_md > >> *xdp, struct bpf_dynptr *xd > >> struct vip vip = {}; > >> int dport; > >> __u32 csum = 0; > >> - int i; > >> + size_t i; > >> __builtin_memset(eth_buffer, 0, sizeof(eth_buffer)); > >> __builtin_memset(iph_buffer_tcp, 0, sizeof(iph_buffer_tcp)); > >> diff --git a/tools/testing/selftests/bpf/progs/test_xdp_loop.c b/ > >> tools/testing/selftests/bpf/progs/test_xdp_loop.c > >> index 93267a68825b..e9b7bbff5c23 100644 > >> --- a/tools/testing/selftests/bpf/progs/test_xdp_loop.c > >> +++ b/tools/testing/selftests/bpf/progs/test_xdp_loop.c > >> @@ -85,7 +85,7 @@ static __always_inline int handle_ipv4(struct xdp_md > >> *xdp) > >> struct vip vip = {}; > >> int dport; > >> __u32 csum = 0; > >> - int i; > >> + size_t i; > >> if (iph + 1 > data_end) > >> return XDP_DROP; > >> diff --git a/tools/testing/selftests/bpf/progs/test_xdp_noinline.c b/ > >> tools/testing/selftests/bpf/progs/test_xdp_noinline.c > >> index fad94e41cef9..85ef3c0a3e20 100644 > >> --- a/tools/testing/selftests/bpf/progs/test_xdp_noinline.c > >> +++ b/tools/testing/selftests/bpf/progs/test_xdp_noinline.c > >> @@ -372,7 +372,7 @@ bool encap_v4(struct xdp_md *xdp, struct ctl_value > >> *cval, > >> next_iph_u16 = (__u16 *) iph; > >> __pragma_loop_unroll_full > >> - for (int i = 0; i < sizeof(struct iphdr) >> 1; i++) > >> + for (size_t i = 0; i < sizeof(struct iphdr) >> 1; i++) > >> csum += *next_iph_u16++; > >> iph->check = ~((csum & 0xffff) + (csum >> 16)); > >> if (bpf_xdp_adjust_head(xdp, (int)sizeof(struct iphdr))) > >> @@ -423,7 +423,7 @@ int send_icmp_reply(void *data, void *data_end) > >> iph->check = 0; > >> next_iph_u16 = (__u16 *) iph; > >> __pragma_loop_unroll_full > >> - for (int i = 0; i < sizeof(struct iphdr) >> 1; i++) > >> + for (size_t i = 0; i < sizeof(struct iphdr) >> 1; i++) > >> csum += *next_iph_u16++; > >> iph->check = ~((csum & 0xffff) + (csum >> 16)); > >> return swap_mac_and_send(data, data_end); > >> diff --git a/tools/testing/selftests/bpf/progs/uprobe_multi.c b/tools/ > >> testing/selftests/bpf/progs/uprobe_multi.c > >> index 44190efcdba2..f99957773c3a 100644 > >> --- a/tools/testing/selftests/bpf/progs/uprobe_multi.c > >> +++ b/tools/testing/selftests/bpf/progs/uprobe_multi.c > >> @@ -20,13 +20,13 @@ __u64 uretprobe_multi_func_3_result = 0; > >> __u64 uprobe_multi_sleep_result = 0; > >> -int pid = 0; > >> +__u32 pid = 0; > >> int child_pid = 0; > >> int child_tid = 0; > >> int child_pid_usdt = 0; > >> int child_tid_usdt = 0; > >> -int expect_pid = 0; > >> +__u32 expect_pid = 0; > >> bool bad_pid_seen = false; > >> bool bad_pid_seen_usdt = false; > >> diff --git a/tools/testing/selftests/bpf/progs/ > >> uprobe_multi_session_recursive.c b/tools/testing/selftests/bpf/progs/ > >> uprobe_multi_session_recursive.c > >> index 8fbcd69fae22..017f1859ebe8 100644 > >> --- a/tools/testing/selftests/bpf/progs/uprobe_multi_session_recursive.c > >> +++ b/tools/testing/selftests/bpf/progs/uprobe_multi_session_recursive.c > >> @@ -3,6 +3,7 @@ > >> #include <bpf/bpf_helpers.h> > >> #include <bpf/bpf_tracing.h> > >> #include <stdbool.h> > >> +#include <stddef.h> > >> #include "bpf_kfuncs.h" > >> #include "bpf_misc.h" > >> @@ -10,8 +11,8 @@ char _license[] SEC("license") = "GPL"; > >> int pid = 0; > >> -int idx_entry = 0; > >> -int idx_return = 0; > >> +size_t idx_entry = 0; > >> +size_t idx_return = 0; > >> __u64 test_uprobe_cookie_entry[6]; > >> __u64 test_uprobe_cookie_return[3]; > >> diff --git a/tools/testing/selftests/bpf/progs/ > >> verifier_iterating_callbacks.c b/tools/testing/selftests/bpf/progs/ > >> verifier_iterating_callbacks.c > >> index 75dd922e4e9f..72f9f8c23c93 100644 > >> --- a/tools/testing/selftests/bpf/progs/verifier_iterating_callbacks.c > >> +++ b/tools/testing/selftests/bpf/progs/verifier_iterating_callbacks.c > >> @@ -593,7 +593,7 @@ int loop_inside_iter_volatile_limit(const void *ctx) > >> { > >> struct bpf_iter_num it; > >> int *v, sum = 0; > >> - __u64 i = 0; > >> + __s32 i = 0; > >> bpf_iter_num_new(&it, 0, ARR2_SZ); > >> while ((v = bpf_iter_num_next(&it))) { > > > >
