On Wed, 24 Sep 2025 17:23:49 +0100
Mehdi Ben Hadj Khelifa [off-list ref] wrote:

-Change all the source files and the corresponding headers 
to having matching sign comparisons.

'Fixing' -Wsign-compare by adding loads of casts doesn't seem right.
The only real way is to change all the types to unsigned ones.
But it is of questionable benefit and make the code harder to read.

Consider the following:
	int x = read(fd, buf, len);
	if (x < 0)
		return -1;
	if (x > sizeof (struct fubar))
		return -1;
That will generate a 'sign-compare' error, but min(x, sizeof (struct fubar))
doesn't generate an error because the compiler knows 'x' isn't negative.

A well known compiler also rejects:
	unsigned char a;
	unsigned int b;
	if (b > a)
		return;
because 'a' is promoted to 'signed int' before it does the check.

So until the compilers start looking at the known domain of the value
(not just the type) I enabling -Wsign-compare' is pretty pointless.

As a matter of interest did you actually find any bugs?

	David

Signed-off-by: Mehdi Ben Hadj Khelifa <redacted>
---
As suggested by the TODO, -Wsign-compare was added to the C compilation
flags for the selftests/bpf/Makefile and all corresponding files in
selftests and a single file under tools/lib/bpf/usdt.bpf.h have been
carefully changed to account for correct sign comparisons either by
explicit casting or changing the variable type.Only local variables
and variables which are in limited scope have been changed in cases
where it doesn't break the code.Other struct variables or global ones 
have left untouched to avoid other conflicts and opted to explicit 
casting in this case.This change will help avoid implicit type 
conversions and have predictable behavior.

I have already compiled all bpf tests with no errors as well as the
kernel and have ran all the selftests with no obvious side effects.
I would like to know if it's more convinient to have all changes as
a single patch like here or if it needs to be divided in some way 
and sent as a patch series.

Best Regards,
Mehdi Ben Hadj Khelifa

...