Messages in this thread | | | Date | Thu, 24 Sep 2015 11:03:06 -0700 | Subject | Re: + kernelh-make-abs-work-with-64-bit-types.patch added to -mm tree | From | Linus Torvalds <> |
| |
On Wed, Sep 23, 2015 at 6:44 AM, Alexey Dobriyan <adobriyan@gmail.com> wrote: > > I've sent kabs() before which didn't go in because it didn't work for > INT_MAX et al > > [PATCH v2] Add kabs() > http://marc.info/?l=linux-kernel&m=133518745522740&w=4
Yeah, no, that's bad.
Testing against char/short is pointless. They should get upgraded to "int".
And guys, stop the idiotic "shouldn't work on unsigned values" crap. It damn well should work on unsigned values, and the semantics we want is that we treat it as a signed value.
Why? That's how "abs()" works. Really. Stop fighting it. If you were to use a real "abs()", it takes an "int" argument, and if you pass it an unsigned value, it gets converted to "int".
End of story. Stop with the broken "but but but unsigned" crap already.
So the end result is that
(a) we should look at the *size* of the argument type, not the signedness, because the signedness is immaterial
(b) we should *not* cast the thing to unsigned, because we traditionally haven't.
(c) we should definitely not use smaller than "int" as a minimum size, no crazy games with char/short. And considering our legacy, I think we should probably skip "int" and stay with "long" as the minimum size.
So I *much* prefer Michal's "abs()" definition that doesn't radically change the meaning (it keeps the old behavior *except* if you pass in a bigger size than "long", in which case it will auto-widen to "s64"). That's the maximally compatible model given the "we handle bigger types automatically" extension.
Because "maximally compatible" is a strong argument. The signedness stuff and the "let's try to use the exact same type" is just bogus.
One thing that *is* interesting is "what if 'long' and 's64' are the same size?" In particular, it means that right now Michal's patch *always* returns "long" on a 64-bit architecture, but will return "long" or "s64" on a 32-bit one. The reason that is somewhat interesting is that while the sizes and values are the same, and the resulting C type expansions are "equivalent" types, i people *print* things, you have to use different modifiers for the two cases. So you might get warnings on 32-bit architectures and not get them on 64-bit, or vice versa.
However, I don't see a good solution for that. And assuming we don't use "abs()" in an expression to printk(), I guess it doesn't much matter either.
Linus
| |