Re: [PATCH v3 3/3] selftests/bpf: Prepare to add -Wsign-compare for bpf tests

[Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>]

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 <mehdi.benhadjkhelifa@gmail.com>
> 
> 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))) {
>