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

[Xingtao Yao (Fujitsu)]

> -----Original Message-----
> From: Peter Maydell <peter.maydell@linaro.org>
> Sent: Thursday, July 25, 2024 11:14 PM
> To: Yao, Xingtao/姚 幸涛 <yaoxt.fnst@fujitsu.com>
> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>; qemu-devel@nongnu.org
> Subject: Re: [PATCH 00/13] make range overlap check more readable
> 
> 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.
I understand your confusion, but it is difficult to match the region overlap 
check because
it has many variations, like below:
case1:
start >= old_start +old_len || start + len <= old_start

case2:
start >= old_end || end <= old_start

case3:
cur->phys_addr >= begin + length || cur->phys_addr + cur->length <= begin

case4:
new->base >= old.base +old.size || new->base + new->size <= old.base
......
and sometimes the length or end may be also an expression, I can not find a way 
to
handle all the variants.

So I decided to use the above pattern to find out the region overlap checks as 
much as possible,
then manually drop the cases that does not meet the requirements, and then 
manually modify 
the cases that meets the requirements.

 
> thanks
> -- PMM