On Thu, Jul 25, 2024 at 10:02:45AM GMT, Linus Torvalds wrote: > On Thu, 25 Jul 2024 at 02:01, David Laight <David.Laight@xxxxxxxxxx> wrote: > > > > The condition is '>= 0' so it doesn't matter if it is '1' or '0'. > > Yes, but that's because the whole conditional is so inexplicably complex. > > But the explanation is: > > > That gives a 'comparison of unsigned type against 0 is always true' warning. > > (The compiler generates that for code in the unused branches of both > > __builtin_choose_expr() and _Generic().) > > Moving the comparison to the outer level stops all such compiler warnings. > > Christ. This whole series is a nightmare of "add complexity to deal > with stupid issues". > > But the kernel test robot clearly found even more issues. > > I think we need to just go back to the old code. It was stupid and > limited and caused us to have to be more careful about types than was > strictly necessary. The problem is simply reverting reveals that seemingly a _ton_ of code has come to rely on the relaxed conditions. When I went to gather the numbers for my initial report I had to manually fix up every case which was rather painful, and that was just a defconfig + a few extra options. allmodconfig is likely to be a hellscape. I've not dug deep into the ins + outs of this, so forgive me for being vague (Arnd has a far clearer understanding) - but it seems that the majority of the complexity comes from having to absolutely ensure all this works for compile-time constant values. Arnd had a look through and determined there weren't _too_ many cases where we need this (for instance array sizes). So I wonder whether we can't just vastly simplify this stuff (and reducing the macro expansion hell) for the usual case, and implement something like cmin()/cmax() or whatever for the true-constant cases? > > But it was also about a million times simpler, and didn't cause build > time regressions. > :) > Linus
