Re: [PATCH v3 5/5] minmax: Relax check to allow comparison between int and small unsigned constants.

[Linus Torvalds]

On Mon, 7 Aug 2023 at 03:50, David Laight <David.Laight@xxxxxxxxxx> 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