Re: linux-4.20-rc1/arch/x86/mm/pageattr.c:366: logical error ?

[Thomas Gleixner]

David,

On Mon, 5 Nov 2018, David Binderman wrote:

> 
> linux-4.20-rc1/arch/x86/mm/pageattr.c:366]: (style) Same expression on both sides of '||' because 'r1_start<=r2_end&&r1_end>=r2_start' and 'r2_start<=r1_end&&r2_end>=r1_start' represent the same value.
>

The compiler is actually right.

The moron who wrote that code obviously thought that checking the same
thing twice makes sure that the result is more correct. :)

> Source code is
> 
> static bool overlaps(unsigned long r1_start, unsigned long r1_end,
>                      unsigned long r2_start, unsigned long r2_end)
> {
>         return (r1_start <= r2_end && r1_end >= r2_start) ||
>                 (r2_start <= r1_end && r2_end >= r1_start);
> }
> 
> Maybe better code
> 
> static bool overlaps(unsigned long r1_start, unsigned long r1_end,
>                      unsigned long r2_start, unsigned long r2_end)
> {
>         /* r1_start inside r2_start .. r2_end
>             or r1_end inside r2_start .. r2_end
>           */
>         return (r1_start >= r2_start && r1_start <= r2_end) ||
>                 (r1_end >= r2_start && r1_end <= r2_end);
> }

That maybe better code fails with these ranges:

r1 0x0100 - 0x01ff
r2 0x0110 - 0x011f

It claims that there is no overlap between the two ranges. I leave it to
you to figure out why it fails.

But the function which the compiler complains about is actually correct for
all variants of input pairs.

It's just having the same condition twice. The second part after the || is
just the reverse expressed, but identical variant of the first.

So the obvious solution is to remove that redundant part.

Thanks,

	tglx