Hi Linus, On Wed. 9 Mar 2022 at 03:13, Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > On Tue, Mar 8, 2022 at 6:12 AM Vincent Mailhol > <mailhol.vincent@xxxxxxxxxx> wrote: > > > > This patch silences a -Wtypes-limits warning in GENMASK_INPUT_CHECK() > > which is accountable for 31% of all warnings when compiling with W=2. > > Please, just make the patch be "remote -Wtypes-limits". After this patch, the number of remaining -Wtype-limits drops by 99.7% from 164714 to only 431 for an allyesconfig (some of which could be true positives). So I am inclined to keep -Wtype-limits at W=2 because it still catches some relevant issues. Aside from the issue pointed out here, it is not a hindrance. > Instead of making an already complicated check more complicated, and > making it more fragile. ACK, this patch makes it more complicated. About making it more fragile, lib/test_bits.c is here to catch issues and this patch passes those tests including the TEST_GENMASK_FAILURES. > I don't see why that int cast on h would be valid, for example. Why > just h? The compiler only complains on ((unsigned int)foo > 0) patterns, i.e. when h is unsigned and l is zero. The signness of l is not relevant here. > And should you not then check that the cast doesn't actually > change the value? The loss of precision only occurs on big values e.g. GENMASK(UINT_MAX + 1, 0). GENMASK (and GENMASK_ULL) already requires h and l to be between 0 and 31 (or 63). Out of band positive values are caught by -Wshift-count-overflow (and negative values by -Wshift-count-negative). So the use cases in which the int cast would change h value are already caught elsewhere. > But the basic issue is that the compiler warns about bad things, and > the problem isn't the code, but the compiler. ACK, the code is not broken, the compiler is guilty. I tend to agree to the rule "if not broken, don’t fix", but I consider this patch to be *the exception* because of the outstanding level of noise generated here. If my message did not convince you, then I am fine to move -Wtypes-limits from W=2 to W=3 as a compromise. But this is not my preferred solution because some -Wtypes-limits warnings are useful. Yours sincerely, Vincent Mailhol
