On Tue, 25 Nov 2025 01:09:42 +0000
Matthew Wilcox <[email protected]> wrote:
> On Mon, Nov 24, 2025 at 03:30:19PM -0800, Linus Torvalds wrote:
> > That all a very standard thing in assembly programming, which this is
> > all about. 'entry' is a signed offset from its own address.
>
> I used to be an assembly programmer ... 28 years ago. I've mostly put
> that world out of my mind (and being able to write a 20,000 instruction
> ARM32 program entirely in assembly is just not that useful an
> accomplishment to put on my CV). Anyway, this isn't the point ...
>
> > > The warning is ... not the best phrased, but in terms of divining the
> > > programmer's intent, I genuinely don't know if this code is supposed
> > > to zero-extend or sign-extend the s32 to unsigned long.
> >
> > What?
> >
> > A signed value gets sign-extended when cast to a larger type. That's
> > how all of this always works. Casting a signed value to 'unsigned
> > long' will set the high bits in the result.
> >
> > That's pretty much the *definition* of a signed value. It gets
> > sign-extended when used, and then obviously it becomes a large
> > unsigned value, but this is how two's complement addition
> > fundamentally works.
>
> Yes, agreed.
>
> > So honestly, what's the problem with this code?
> >
> > The warning makes no sense, and is garbage. Are we not allowed to add
> > signed integers to unsigned 64-bit values now, because that addition
> > involves that cast of a signed 32-bit entry to an unsigned 64-bit one?
> >
> > There is NO WAY that warning is valid, it's; not *ever* something we
> > should enable, and the fact that you people are discussing it as such
> > is just crazy.
> >
> > That code would not be improved at all by adding another cast (to
> > first cast that s32 to 'long', in order to then add it to 'unsigned
> > long').
> >
> > Imagine how many other places you add integers to 'unsigned long'.
> > EVERY SINGLE ONE of those places involves sign-extending the integer
> > and then doing arithmetic in unsigned.
>
> I have bad news. Rust requires it.
>
> fn add(base: u64, off: i32) -> u64 {
> base + off
> }
>
> error[E0308]: mismatched types
> --> add.rs:2:12
> |
> 2 | base + off
> | ^^^ expected `u64`, found `i32`
>
> error[E0277]: cannot add `i32` to `u64`
> --> add.rs:2:10
> |
> 2 | base + off
> | ^ no implementation for `u64 + i32`
> |
> = help: the trait `Add<i32>` is not implemented for `u64`
> = help: the following other types implement trait `Add<Rhs>`:
> <u64 as Add>
> <u64 as Add<&u64>>
> <&'a u64 as Add<u64>>
> <&u64 as Add<&u64>>
>
> so the Rust language people have clearly decided that this is too
> complicated for your average programmer to figure out, and you need
> explicit casts to make it work.
>
Jeepers...
As I've found looking at min_t() you can't trust kernel programmers
(never mind 'average' ones) to use the correct cast.
It wouldn't surprise be if the casts cause more bugs that the automatic
conversions that C does.
It wouldn't be as bad if there were separate 'casts' for widening and narrowing.
You also need the compiler to be doing 'value tracking' rather than just
looking at the types.
If I do:
int len = read(.....);
if (len < 0)
return -1;
if (len > sizeof (...))
...
then -Wsign-compare complains, but a statically_true(len >= 0) is fine.
David
