Re: [PATCH 00/13] make range overlap check more readable

[Peter Maydell]

On Mon, 22 Jul 2024 at 08:00, Xingtao Yao (Fujitsu) via
<qemu-devel@nongnu.org> wrote:
>
>
>
> > -----Original Message-----
> > From: Philippe Mathieu-Daudé <philmd@linaro.org>
> > Sent: Monday, July 22, 2024 2:43 PM
> > To: Yao, Xingtao/姚 幸涛 <yaoxt.fnst@fujitsu.com>; qemu-devel@nongnu.org
> > Subject: Re: [PATCH 00/13] make range overlap check more readable
> >
> > Hi Yao,
> >
> > On 22/7/24 06:07, Yao Xingtao via wrote:
> > > Currently, some components still open-coding the range overlap check.
> > > Sometimes this check may be fail because some patterns are missed.
> >
> > How did you catch all these use cases?
> I used the Coccinelle to match these use cases, the pattern is below
> range_overlap.cocci:
>
> // use ranges_overlap() instead of open-coding the overlap check
> @@
> expression E1, E2, E3, E4;
> @@
> (
> - E2 <= E3 || E1 >= E4
> + !ranges_overlap(E1, E2, E3, E4)
> |

Maybe I'm misunderstanding the coccinelle patch here, but
I don't see how it produces the results in the patchset.
ranges_overlap() takes arguments (start1, len1, start2, len2),
but an expression like "E2 <= E3 || E1 >= E4" is working
with start,end pairs to indicate the ranges. And looking
at e.g. patch 9:

- if (cur->phys_addr >= begin + length ||
- cur->phys_addr + cur->length <= begin) {
+ if (!ranges_overlap(cur->phys_addr, cur->length, begin, length)) {

the kind of if() check you get for start, length pairs
has an addition in it, which I don't see in any of these
coccinelle script fragments.

thanks
-- PMM