Messages in this thread | | | From | Linus Torvalds <> | Date | Mon, 7 Aug 2023 08:48:44 -0700 | Subject | Re: [PATCH v3 5/5] minmax: Relax check to allow comparison between int and small unsigned constants. |
| |
On Mon, 7 Aug 2023 at 03:50, David Laight <David.Laight@aculab.com> wrote: > > To my mind the value of min(variable, TWENTY) shouldn't depend > on how TWENTY is defined regardless of the type of the variable. > So 20, 20u, 20l, 20ul, (char)20, sizeof (foo), offsetof(x, y) > should all be equally valid and all generate the same result.
That sounds nice, but I don't believe it is true.
If somebody writes
a = min(b, 20u);
then that is *not* the same thing as
a = min(b, 20);
without knowing the types.
But you make them be the same thing - now they become ambiguous and depend on the type of 'b'.
Does that expression mean "give me a number 0..20" or "MININT..20"?
And the whole point of the type checking is very much to not have ambiguous comparisons where you can have those kinds of questions.
> I've found all sorts of dubious min_t() while writing these patches. > One in a filesystem was min_t(ulong, ulong_var, u64_var) and I > couldn't convince myself it was right on 32bit.
Honestly, that's a great example, but I think that's more of an argument against 'min_t()' than it is an argument for changing 'min()' and 'max()'.
I think it was a mistake to do "min_t()", and we should have done sign-based ones ("do a unsigned/signed min/max").
I agree that a "min_t()" that narrows the type is very scary, in that it might lose bits, and it's obviously also easily dependent on word size too, as in your example.
We could perhaps aim to make 'min_t()' warn about 't' being narrower than the types you compare.
(Although then you hit the traditional "C doesn't really have 'char' and 'short' types in expressions", so you'd probably have to do the type size check with widening of 't' in place, so 'min_t(char, int, int)' would be ok).
Linus
Linus
| |