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

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

[Yao Xingtao via]

Currently, some components still open-coding the range overlap check.
Sometimes this check may be fail because some patterns are missed.

To avoid the above problems and improve the readability of the code,
it is better to use the ranges_overlap() to do this check.

Yao Xingtao (13):
  range: Make ranges_overlap() return bool
  arm/boot: make range overlap check more readable
  core/loader: make range overlap check more readable
  cxl/mailbox: make range overlap check more readable
  display/sm501: make range overlap check more readable
  aspeed_smc: make range overlap check more readable
  qtest/fuzz: make range overlap check more readable
  sparc/ldst_helper: make range overlap check more readable
  system/memory_mapping: make range overlap check more readable
  block/vhdx: make range overlap check more readable
  crypto/block-luks: make range overlap check more readable
  dump: make range overlap check more readable
  block/qcow2-cluster: make range overlap check more readable

 block/qcow2-cluster.c           | 23 +++++++++++++----------
 block/vhdx.c                    | 12 +++++++-----
 crypto/block-luks.c             |  3 ++-
 dump/dump.c                     | 12 ++++++++----
 hw/arm/boot.c                   |  5 +++--
 hw/core/loader.c                |  4 +++-
 hw/cxl/cxl-mailbox-utils.c      |  8 ++++----
 hw/display/sm501.c              | 12 ++++++------
 hw/ssi/aspeed_smc.c             |  4 ++--
 include/qemu/range.h            |  4 ++--
 system/memory_mapping.c         |  4 ++--
 target/sparc/ldst_helper.c      |  5 ++---
 tests/qtest/fuzz/generic_fuzz.c |  3 ++-
 13 files changed, 56 insertions(+), 43 deletions(-)

-- 
2.41.0

[PATCH 01/13] range: Make ranges_overlap() return bool

[Yao Xingtao via]

Just like range_overlaps_range(), use the returned bool value
to check whether 2 given ranges overlap.

Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
---
 include/qemu/range.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/qemu/range.h b/include/qemu/range.h
index 4ce694a39831..d446ad885d2d 100644
--- a/include/qemu/range.h
+++ b/include/qemu/range.h
@@ -210,8 +210,8 @@ static inline int range_covers_byte(uint64_t offset, uint64_t len,
 
 /* Check whether 2 given ranges overlap.
  * Undefined if ranges that wrap around 0. */
-static inline int ranges_overlap(uint64_t first1, uint64_t len1,
-                                 uint64_t first2, uint64_t len2)
+static inline bool ranges_overlap(uint64_t first1, uint64_t len1,
+                                  uint64_t first2, uint64_t len2)
 {
     uint64_t last1 = range_get_last(first1, len1);
     uint64_t last2 = range_get_last(first2, len2);
-- 
2.41.0

Re: [PATCH 01/13] range: Make ranges_overlap() return bool

[Philippe Mathieu-Daudé]

On 22/7/24 06:07, Yao Xingtao via wrote:
> Just like range_overlaps_range(), use the returned bool value
> to check whether 2 given ranges overlap.
> 
> Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
> ---
>   include/qemu/range.h | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 02/13] arm/boot: make range overlap check more readable

[Yao Xingtao via]

use ranges_overlap() instead of open-coding the overlap check to improve
the readability of the code.

Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
---
 hw/arm/boot.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index d480a7da02cf..a004a90e87be 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -26,6 +26,7 @@
 #include "qemu/config-file.h"
 #include "qemu/option.h"
 #include "qemu/units.h"
+#include "qemu/range.h"
 
 /* Kernel boot protocol is specified in the kernel docs
  * Documentation/arm/Booting and Documentation/arm64/booting.txt
@@ -238,8 +239,8 @@ void arm_write_secure_board_setup_dummy_smc(ARMCPU *cpu,
     assert((mvbar_addr & 0x1f) == 0 && (mvbar_addr >> 4) < 0x100);
 
     /* check that these blobs don't overlap */
-    assert((mvbar_addr + sizeof(mvbar_blob) <= info->board_setup_addr)
-          || (info->board_setup_addr + sizeof(board_setup_blob) <= mvbar_addr));
+    assert(!ranges_overlap(mvbar_addr, sizeof(mvbar_blob),
+           info->board_setup_addr, sizeof(board_setup_blob)));
 
     for (n = 0; n < ARRAY_SIZE(mvbar_blob); n++) {
         mvbar_blob[n] = tswap32(mvbar_blob[n]);
-- 
2.41.0

Re: [PATCH 02/13] arm/boot: make range overlap check more readable

[Philippe Mathieu-Daudé]

On 22/7/24 06:07, Yao Xingtao via wrote:
> use ranges_overlap() instead of open-coding the overlap check to improve
> the readability of the code.
> 
> Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
> ---
>   hw/arm/boot.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index d480a7da02cf..a004a90e87be 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -26,6 +26,7 @@
>   #include "qemu/config-file.h"
>   #include "qemu/option.h"
>   #include "qemu/units.h"
> +#include "qemu/range.h"
>   
>   /* Kernel boot protocol is specified in the kernel docs
>    * Documentation/arm/Booting and Documentation/arm64/booting.txt
> @@ -238,8 +239,8 @@ void arm_write_secure_board_setup_dummy_smc(ARMCPU *cpu,
>       assert((mvbar_addr & 0x1f) == 0 && (mvbar_addr >> 4) < 0x100);
>   
>       /* check that these blobs don't overlap */
> -    assert((mvbar_addr + sizeof(mvbar_blob) <= info->board_setup_addr)
> -          || (info->board_setup_addr + sizeof(board_setup_blob) <= mvbar_addr));
> +    assert(!ranges_overlap(mvbar_addr, sizeof(mvbar_blob),
> +           info->board_setup_addr, sizeof(board_setup_blob)));

Indentation is of, otherwise:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 03/13] core/loader: make range overlap check more readable

[Yao Xingtao via]

use ranges_overlap() instead of open-coding the overlap check to improve
the readability of the code.

Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
---
 hw/core/loader.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/core/loader.c b/hw/core/loader.c
index 31593a117171..dac0df561b16 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -63,6 +63,7 @@
 #include "qemu/cutils.h"
 #include "sysemu/runstate.h"
 #include "tcg/debuginfo.h"
+#include "qemu/range.h"
 
 #include <zlib.h>
 
@@ -1278,7 +1279,8 @@ static bool roms_overlap(Rom *last_rom, Rom *this_rom)
         return false;
     }
     return last_rom->as == this_rom->as &&
-        last_rom->addr + last_rom->romsize > this_rom->addr;
+        ranges_overlap(last_rom->addr, last_rom->romsize,
+                       this_rom->addr, this_rom->romsize);
 }
 
 static const char *rom_as_name(Rom *rom)
-- 
2.41.0

[PATCH 04/13] cxl/mailbox: make range overlap check more readable

[Yao Xingtao via]

use ranges_overlap() instead of open-coding the overlap check to improve
the readability of the code.

Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
---
 hw/cxl/cxl-mailbox-utils.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index 74eeb6fde739..507690c0dd16 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -1086,8 +1086,8 @@ static CXLRetCode cmd_media_get_poison_list(const struct cxl_cmd *cmd,
 
     QLIST_FOREACH(ent, poison_list, node) {
         /* Check for no overlap */
-        if (ent->start >= query_start + query_length ||
-            ent->start + ent->length <= query_start) {
+        if (!ranges_overlap(ent->start, ent->length,
+                            query_start, query_length)) {
             continue;
         }
         record_count++;
@@ -1100,8 +1100,8 @@ static CXLRetCode cmd_media_get_poison_list(const struct cxl_cmd *cmd,
         uint64_t start, stop;
 
         /* Check for no overlap */
-        if (ent->start >= query_start + query_length ||
-            ent->start + ent->length <= query_start) {
+        if (!ranges_overlap(ent->start, ent->length,
+                            query_start, query_length)) {
             continue;
         }
 
-- 
2.41.0

Re: [PATCH 04/13] cxl/mailbox: make range overlap check more readable

[Philippe Mathieu-Daudé]

On 22/7/24 06:07, Yao Xingtao via wrote:
> use ranges_overlap() instead of open-coding the overlap check to improve
> the readability of the code.
> 
> Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
> ---
>   hw/cxl/cxl-mailbox-utils.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 05/13] display/sm501: make range overlap check more readable

[Yao Xingtao via]

use ranges_overlap() instead of open-coding the overlap check to improve
the readability of the code.

Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
---
 hw/display/sm501.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index 26dc8170d89b..c25b2574e447 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -823,15 +823,15 @@ static void sm501_2d_operation(SM501State *s)
             }
             /* If reverse blit do simple check for overlaps */
             if (rtl && src_base == dst_base && src_pitch == dst_pitch) {
-                overlap = (src_x < dst_x + width && src_x + width > dst_x &&
-                           src_y < dst_y + height && src_y + height > dst_y);
+                overlap = (ranges_overlap(src_x, width, dst_x, width) &&
+                           ranges_overlap(src_y, height, dst_y, height));
             } else if (rtl) {
-                unsigned int sb, se, db, de;
+                unsigned int sb, sl, db, dl;
                 sb = src_base + (src_x + src_y * src_pitch) * bypp;
-                se = sb + (width + (height - 1) * src_pitch) * bypp;
+                sl = (width + (height - 1) * src_pitch) * bypp;
                 db = dst_base + (dst_x + dst_y * dst_pitch) * bypp;
-                de = db + (width + (height - 1) * dst_pitch) * bypp;
-                overlap = (db < se && sb < de);
+                dl = (width + (height - 1) * dst_pitch) * bypp;
+                overlap = ranges_overlap(sb, sl, db, dl);
             }
 #ifdef CONFIG_PIXMAN
             if (overlap && (s->use_pixman & BIT(2))) {
-- 
2.41.0

[PATCH 06/13] aspeed_smc: make range overlap check more readable

[Yao Xingtao via]

use ranges_overlap() instead of open-coding the overlap check to improve
the readability of the code.

Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
---
 hw/ssi/aspeed_smc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index 49205ab76d38..cdca359e1d88 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -31,6 +31,7 @@
 #include "qemu/error-report.h"
 #include "qapi/error.h"
 #include "qemu/units.h"
+#include "qemu/range.h"
 #include "trace.h"
 
 #include "hw/irq.h"
@@ -246,8 +247,7 @@ static bool aspeed_smc_flash_overlap(const AspeedSMCState *s,
 
         asc->reg_to_segment(s, s->regs[R_SEG_ADDR0 + i], &seg);
 
-        if (new->addr + new->size > seg.addr &&
-            new->addr < seg.addr + seg.size) {
+        if (ranges_overlap(new->addr, new->size, seg.addr, seg.size)) {
             aspeed_smc_error("new segment CS%d [ 0x%"
                              HWADDR_PRIx" - 0x%"HWADDR_PRIx" ] overlaps with "
                              "CS%d [ 0x%"HWADDR_PRIx" - 0x%"HWADDR_PRIx" ]",
-- 
2.41.0

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

[Philippe Mathieu-Daudé]

On 22/7/24 06:07, Yao Xingtao via wrote:
> use ranges_overlap() instead of open-coding the overlap check to improve
> the readability of the code.
> 
> Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
> ---
>   hw/ssi/aspeed_smc.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 07/13] qtest/fuzz: make range overlap check more readable

[Yao Xingtao via]

use ranges_overlap() instead of open-coding the overlap check to improve
the readability of the code.

Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
---
 tests/qtest/fuzz/generic_fuzz.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
index ec842e03c5e6..d107a496da63 100644
--- a/tests/qtest/fuzz/generic_fuzz.c
+++ b/tests/qtest/fuzz/generic_fuzz.c
@@ -11,6 +11,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/range.h"
 
 #include <wordexp.h>
 
@@ -211,7 +212,7 @@ void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr)
          i < dma_regions->len && (avoid_double_fetches || qtest_log_enabled);
          ++i) {
         region = g_array_index(dma_regions, address_range, i);
-        if (addr < region.addr + region.size && addr + len > region.addr) {
+        if (ranges_overlap(addr, len, region.addr, region.size)) {
             double_fetch = true;
             if (addr < region.addr
                 && avoid_double_fetches) {
-- 
2.41.0

Re: [PATCH 07/13] qtest/fuzz: make range overlap check more readable

[Alexander Bulekov]

On 240722 0007, Yao Xingtao wrote:
> use ranges_overlap() instead of open-coding the overlap check to improve
> the readability of the code.
> 
> Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>

Reviewed-by: Alexander Bulekov <alxndr@bu.edu>

Thank you

> ---
>  tests/qtest/fuzz/generic_fuzz.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
> index ec842e03c5e6..d107a496da63 100644
> --- a/tests/qtest/fuzz/generic_fuzz.c
> +++ b/tests/qtest/fuzz/generic_fuzz.c
> @@ -11,6 +11,7 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#include "qemu/range.h"
>  
>  #include <wordexp.h>
>  
> @@ -211,7 +212,7 @@ void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr)
>           i < dma_regions->len && (avoid_double_fetches || qtest_log_enabled);
>           ++i) {
>          region = g_array_index(dma_regions, address_range, i);
> -        if (addr < region.addr + region.size && addr + len > region.addr) {
> +        if (ranges_overlap(addr, len, region.addr, region.size)) {
>              double_fetch = true;
>              if (addr < region.addr
>                  && avoid_double_fetches) {
> -- 
> 2.41.0
> 

Re: [PATCH 07/13] qtest/fuzz: make range overlap check more readable

[Philippe Mathieu-Daudé]

On 22/7/24 06:07, Yao Xingtao via wrote:
> use ranges_overlap() instead of open-coding the overlap check to improve
> the readability of the code.
> 
> Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
> ---
>   tests/qtest/fuzz/generic_fuzz.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 08/13] sparc/ldst_helper: make range overlap check more readable

[Yao Xingtao via]

use ranges_overlap() instead of open-coding the overlap check to improve
the readability of the code.

Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
---
 target/sparc/ldst_helper.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/target/sparc/ldst_helper.c b/target/sparc/ldst_helper.c
index 2d48e98bf468..d92c9f15934e 100644
--- a/target/sparc/ldst_helper.c
+++ b/target/sparc/ldst_helper.c
@@ -19,6 +19,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/log.h"
+#include "qemu/range.h"
 #include "cpu.h"
 #include "tcg/tcg.h"
 #include "exec/helper-proto.h"
@@ -240,9 +241,7 @@ static void replace_tlb_1bit_lru(SparcTLBEntry *tlb,
             if (new_ctx == ctx) {
                 uint64_t vaddr = tlb[i].tag & ~0x1fffULL;
                 uint64_t size = 8192ULL << 3 * TTE_PGSIZE(tlb[i].tte);
-                if (new_vaddr == vaddr
-                    || (new_vaddr < vaddr + size
-                        && vaddr < new_vaddr + new_size)) {
+                if (ranges_overlap(new_vaddr, new_size, vaddr, size)) {
                     DPRINTF_MMU("auto demap entry [%d] %lx->%lx\n", i, vaddr,
                                 new_vaddr);
                     replace_tlb_entry(&tlb[i], tlb_tag, tlb_tte, env1);
-- 
2.41.0

Re: [PATCH 08/13] sparc/ldst_helper: make range overlap check more readable

[Philippe Mathieu-Daudé]

On 22/7/24 06:07, Yao Xingtao via wrote:
> use ranges_overlap() instead of open-coding the overlap check to improve
> the readability of the code.
> 
> Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
> ---
>   target/sparc/ldst_helper.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/target/sparc/ldst_helper.c b/target/sparc/ldst_helper.c
> index 2d48e98bf468..d92c9f15934e 100644
> --- a/target/sparc/ldst_helper.c
> +++ b/target/sparc/ldst_helper.c
> @@ -19,6 +19,7 @@
>   
>   #include "qemu/osdep.h"
>   #include "qemu/log.h"
> +#include "qemu/range.h"
>   #include "cpu.h"
>   #include "tcg/tcg.h"
>   #include "exec/helper-proto.h"
> @@ -240,9 +241,7 @@ static void replace_tlb_1bit_lru(SparcTLBEntry *tlb,
>               if (new_ctx == ctx) {
>                   uint64_t vaddr = tlb[i].tag & ~0x1fffULL;
>                   uint64_t size = 8192ULL << 3 * TTE_PGSIZE(tlb[i].tte);
> -                if (new_vaddr == vaddr

Please mention in the patch description why it is safe to remove this
equality check. With that:

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> -                    || (new_vaddr < vaddr + size
> -                        && vaddr < new_vaddr + new_size)) {
> +                if (ranges_overlap(new_vaddr, new_size, vaddr, size)) {
>                       DPRINTF_MMU("auto demap entry [%d] %lx->%lx\n", i, vaddr,
>                                   new_vaddr);
>                       replace_tlb_entry(&tlb[i], tlb_tag, tlb_tte, env1);

[PATCH 09/13] system/memory_mapping: make range overlap check more readable

[Yao Xingtao via]

use ranges_overlap() instead of open-coding the overlap check to improve
the readability of the code.

Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
---
 system/memory_mapping.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/system/memory_mapping.c b/system/memory_mapping.c
index 6f884c5b90c9..ca2390eb8044 100644
--- a/system/memory_mapping.c
+++ b/system/memory_mapping.c
@@ -12,6 +12,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/range.h"
 #include "qapi/error.h"
 
 #include "sysemu/memory_mapping.h"
@@ -353,8 +354,7 @@ void memory_mapping_filter(MemoryMappingList *list, int64_t begin,
     MemoryMapping *cur, *next;
 
     QTAILQ_FOREACH_SAFE(cur, &list->head, next, next) {
-        if (cur->phys_addr >= begin + length ||
-            cur->phys_addr + cur->length <= begin) {
+        if (!ranges_overlap(cur->phys_addr, cur->length, begin, length)) {
             QTAILQ_REMOVE(&list->head, cur, next);
             g_free(cur);
             list->num--;
-- 
2.41.0

Re: [PATCH 09/13] system/memory_mapping: make range overlap check more readable

[Philippe Mathieu-Daudé]

On 22/7/24 06:07, Yao Xingtao wrote:
> use ranges_overlap() instead of open-coding the overlap check to improve
> the readability of the code.
> 
> Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
> ---
>   system/memory_mapping.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/system/memory_mapping.c b/system/memory_mapping.c
> index 6f884c5b90c9..ca2390eb8044 100644
> --- a/system/memory_mapping.c
> +++ b/system/memory_mapping.c
> @@ -12,6 +12,7 @@
>    */
>   
>   #include "qemu/osdep.h"
> +#include "qemu/range.h"
>   #include "qapi/error.h"
>   
>   #include "sysemu/memory_mapping.h"
> @@ -353,8 +354,7 @@ void memory_mapping_filter(MemoryMappingList *list, int64_t begin,
>       MemoryMapping *cur, *next;
>   
>       QTAILQ_FOREACH_SAFE(cur, &list->head, next, next) {
> -        if (cur->phys_addr >= begin + length ||
> -            cur->phys_addr + cur->length <= begin) {
> +        if (!ranges_overlap(cur->phys_addr, cur->length, begin, length)) {

Maybe this one is easier to read the other way around:

      if (!ranges_overlap(begin, length, cur->phys_addr, cur->length)) {

Anyhow,

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 09/13] system/memory_mapping: make range overlap check more readable

[David Hildenbrand]

On 22.07.24 06:07, Yao Xingtao wrote:
> use ranges_overlap() instead of open-coding the overlap check to improve
> the readability of the code.
> 
> Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
> ---
>   system/memory_mapping.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/system/memory_mapping.c b/system/memory_mapping.c
> index 6f884c5b90c9..ca2390eb8044 100644
> --- a/system/memory_mapping.c
> +++ b/system/memory_mapping.c
> @@ -12,6 +12,7 @@
>    */
>   
>   #include "qemu/osdep.h"
> +#include "qemu/range.h"
>   #include "qapi/error.h"
>   
>   #include "sysemu/memory_mapping.h"
> @@ -353,8 +354,7 @@ void memory_mapping_filter(MemoryMappingList *list, int64_t begin,
>       MemoryMapping *cur, *next;
>   
>       QTAILQ_FOREACH_SAFE(cur, &list->head, next, next) {
> -        if (cur->phys_addr >= begin + length ||
> -            cur->phys_addr + cur->length <= begin) {
> +        if (!ranges_overlap(cur->phys_addr, cur->length, begin, length)) {
>               QTAILQ_REMOVE(&list->head, cur, next);
>               g_free(cur);
>               list->num--;

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

[PATCH 10/13] block/vhdx: make range overlap check more readable

[Yao Xingtao via]

use range_overlaps_range() instead of open-coding the overlap check to improve
the readability of the code.

Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
---
 block/vhdx.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/block/vhdx.c b/block/vhdx.c
index 5aa1a1350626..c31661b946b6 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -32,6 +32,7 @@
 #include "qapi/qmp/qdict.h"
 #include "qapi/qobject-input-visitor.h"
 #include "qapi/qapi-visit-block-core.h"
+#include "qemu/range.h"
 
 /* Options for VHDX creation */
 
@@ -231,15 +232,16 @@ void vhdx_guid_generate(MSGUID *guid)
 static int vhdx_region_check(BDRVVHDXState *s, uint64_t start, uint64_t length)
 {
     int ret = 0;
-    uint64_t end;
     VHDXRegionEntry *r;
+    Range range1, range2;
 
-    end = start + length;
+    range_init_nofail(&range1, start, length);
     QLIST_FOREACH(r, &s->regions, entries) {
-        if (!((start >= r->end) || (end <= r->start))) {
+        range_init_nofail(&range2, r->start, r->end - r->start);
+        if (range_overlaps_range(&range1, &range2)) {
             error_report("VHDX region %" PRIu64 "-%" PRIu64 " overlaps with "
-                         "region %" PRIu64 "-%." PRIu64, start, end, r->start,
-                         r->end);
+                         "region %" PRIu64 "-%." PRIu64, start, start + length,
+                         r->start, r->end);
             ret = -EINVAL;
             goto exit;
         }
-- 
2.41.0

[PATCH 11/13] crypto/block-luks: make range overlap check more readable

[Yao Xingtao via]

use ranges_overlap() instead of open-coding the overlap check to improve
the readability of the code.

Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
---
 crypto/block-luks.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index 5b777c15d3cd..45347adeeb71 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -33,6 +33,7 @@
 #include "qemu/uuid.h"
 
 #include "qemu/bitmap.h"
+#include "qemu/range.h"
 
 /*
  * Reference for the LUKS format implemented here is
@@ -572,7 +573,7 @@ qcrypto_block_luks_check_header(const QCryptoBlockLUKS *luks,
                                                        header_sectors,
                                                        slot2->stripes);
 
-            if (start1 + len1 > start2 && start2 + len2 > start1) {
+            if (ranges_overlap(start1, len1, start2, len2)) {
                 error_setg(errp,
                            "Keyslots %zu and %zu are overlapping in the header",
                            i, j);
-- 
2.41.0

Re: [PATCH 11/13] crypto/block-luks: make range overlap check more readable

[Philippe Mathieu-Daudé]

On 22/7/24 06:07, Yao Xingtao via wrote:
> use ranges_overlap() instead of open-coding the overlap check to improve
> the readability of the code.
> 
> Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
> ---
>   crypto/block-luks.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 11/13] crypto/block-luks: make range overlap check more readable

[Daniel P. Berrangé]

On Mon, Jul 22, 2024 at 12:07:40AM -0400, Yao Xingtao wrote:
> use ranges_overlap() instead of open-coding the overlap check to improve
> the readability of the code.
> 
> Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
> ---
>  crypto/block-luks.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH 12/13] dump: make range overlap check more readable

[Yao Xingtao via]

use ranges_overlap() instead of open-coding the overlap check to improve
the readability of the code.

Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
---
 dump/dump.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/dump/dump.c b/dump/dump.c
index 84064d890d2c..45e84428aea5 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -30,6 +30,7 @@
 #include "migration/blocker.h"
 #include "hw/core/cpu.h"
 #include "win_dump.h"
+#include "qemu/range.h"
 
 #include <zlib.h>
 #ifdef CONFIG_LZO
@@ -574,8 +575,10 @@ static void get_offset_range(hwaddr phys_addr,
 
     QTAILQ_FOREACH(block, &s->guest_phys_blocks.head, next) {
         if (dump_has_filter(s)) {
-            if (block->target_start >= s->filter_area_begin + s->filter_area_length ||
-                block->target_end <= s->filter_area_begin) {
+            if (!ranges_overlap(block->target_start,
+                                block->target_end - block->target_start,
+                                s->filter_area_begin,
+                                s->filter_area_length)) {
                 /* This block is out of the range */
                 continue;
             }
@@ -734,8 +737,9 @@ int64_t dump_filtered_memblock_start(GuestPhysBlock *block,
 {
     if (filter_area_length) {
         /* return -1 if the block is not within filter area */
-        if (block->target_start >= filter_area_start + filter_area_length ||
-            block->target_end <= filter_area_start) {
+        if (!ranges_overlap(block->target_start,
+                            block->target_end - block->target_start,
+                            filter_area_start, filter_area_length)) {
             return -1;
         }
 
-- 
2.41.0

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

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 2025 bytes --]

On Mon, Jul 22, 2024 at 8:10 AM Yao Xingtao <yaoxt.fnst@fujitsu.com> wrote:

> use ranges_overlap() instead of open-coding the overlap check to improve
> the readability of the code.
>
> Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  dump/dump.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/dump/dump.c b/dump/dump.c
> index 84064d890d2c..45e84428aea5 100644
> --- a/dump/dump.c
> +++ b/dump/dump.c
> @@ -30,6 +30,7 @@
>  #include "migration/blocker.h"
>  #include "hw/core/cpu.h"
>  #include "win_dump.h"
> +#include "qemu/range.h"
>
>  #include <zlib.h>
>  #ifdef CONFIG_LZO
> @@ -574,8 +575,10 @@ static void get_offset_range(hwaddr phys_addr,
>
>      QTAILQ_FOREACH(block, &s->guest_phys_blocks.head, next) {
>          if (dump_has_filter(s)) {
> -            if (block->target_start >= s->filter_area_begin +
> s->filter_area_length ||
> -                block->target_end <= s->filter_area_begin) {
> +            if (!ranges_overlap(block->target_start,
> +                                block->target_end - block->target_start,
> +                                s->filter_area_begin,
> +                                s->filter_area_length)) {
>                  /* This block is out of the range */
>                  continue;
>              }
> @@ -734,8 +737,9 @@ int64_t dump_filtered_memblock_start(GuestPhysBlock
> *block,
>  {
>      if (filter_area_length) {
>          /* return -1 if the block is not within filter area */
> -        if (block->target_start >= filter_area_start + filter_area_length
> ||
> -            block->target_end <= filter_area_start) {
> +        if (!ranges_overlap(block->target_start,
> +                            block->target_end - block->target_start,
> +                            filter_area_start, filter_area_length)) {
>              return -1;
>          }
>
> --
> 2.41.0
>
>

[-- Attachment #2: Type: text/html, Size: 2991 bytes --]

[PATCH 13/13] block/qcow2-cluster: make range overlap check more readable

[Yao Xingtao via]

use range_overlaps_range() instead of open-coding the overlap check to
improve the readability of the code.

Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
---
 block/qcow2-cluster.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index ce8c0076b3b5..88d65c4b99e6 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -30,6 +30,7 @@
 #include "qcow2.h"
 #include "qemu/bswap.h"
 #include "qemu/memalign.h"
+#include "qemu/range.h"
 #include "trace.h"
 
 int coroutine_fn qcow2_shrink_l1_table(BlockDriverState *bs,
@@ -1408,23 +1409,25 @@ static int coroutine_fn handle_dependencies(BlockDriverState *bs,
     BDRVQcow2State *s = bs->opaque;
     QCowL2Meta *old_alloc;
     uint64_t bytes = *cur_bytes;
+    Range range1, range2, range3;
 
+    range_init_nofail(&range1, guest_offset, bytes);
     QLIST_FOREACH(old_alloc, &s->cluster_allocs, next_in_flight) {
 
-        uint64_t start = guest_offset;
-        uint64_t end = start + bytes;
-        uint64_t old_start = start_of_cluster(s, l2meta_cow_start(old_alloc));
-        uint64_t old_end = ROUND_UP(l2meta_cow_end(old_alloc), s->cluster_size);
+        uint64_t cow_start = l2meta_cow_start(old_alloc);
+        uint64_t cow_end = l2meta_cow_end(old_alloc);
+        uint64_t start = start_of_cluster(s, cow_start);
+        uint64_t end = ROUND_UP(cow_end, s->cluster_size);
 
-        if (end <= old_start || start >= old_end) {
+        range_init_nofail(&range2, start, end - start);
+        if (!range_overlaps_range(&range1, &range2)) {
             /* No intersection */
             continue;
         }
 
+        range_init_nofail(&range3, cow_start, cow_end - cow_start);
         if (old_alloc->keep_old_clusters &&
-            (end <= l2meta_cow_start(old_alloc) ||
-             start >= l2meta_cow_end(old_alloc)))
-        {
+            !range_overlaps_range(&range1, &range3)) {
             /*
              * Clusters intersect but COW areas don't. And cluster itself is
              * already allocated. So, there is no actual conflict.
@@ -1434,9 +1437,9 @@ static int coroutine_fn handle_dependencies(BlockDriverState *bs,
 
         /* Conflict */
 
-        if (start < old_start) {
+        if (guest_offset < start) {
             /* Stop at the start of a running allocation */
-            bytes = old_start - start;
+            bytes = start - guest_offset;
         } else {
             bytes = 0;
         }
-- 
2.41.0

RE: [PATCH 13/13] block/qcow2-cluster: make range overlap check more readable

[Xingtao Yao (Fujitsu) via]

ping.

> -----Original Message-----
> From: Yao Xingtao <yaoxt.fnst@fujitsu.com>
> Sent: Monday, July 22, 2024 12:08 PM
> To: qemu-devel@nongnu.org; Kevin Wolf <kwolf@redhat.com>; Hanna Reitz
> <hreitz@redhat.com>
> Cc: Yao, Xingtao/姚 幸涛 <yaoxt.fnst@fujitsu.com>; qemu-block@nongnu.org
> Subject: [PATCH 13/13] block/qcow2-cluster: make range overlap check more
> readable
> 
> use range_overlaps_range() instead of open-coding the overlap check to
> improve the readability of the code.
> 
> Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
> ---
>  block/qcow2-cluster.c | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index ce8c0076b3b5..88d65c4b99e6 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -30,6 +30,7 @@
>  #include "qcow2.h"
>  #include "qemu/bswap.h"
>  #include "qemu/memalign.h"
> +#include "qemu/range.h"
>  #include "trace.h"
> 
>  int coroutine_fn qcow2_shrink_l1_table(BlockDriverState *bs,
> @@ -1408,23 +1409,25 @@ static int coroutine_fn
> handle_dependencies(BlockDriverState *bs,
>      BDRVQcow2State *s = bs->opaque;
>      QCowL2Meta *old_alloc;
>      uint64_t bytes = *cur_bytes;
> +    Range range1, range2, range3;
> 
> +    range_init_nofail(&range1, guest_offset, bytes);
>      QLIST_FOREACH(old_alloc, &s->cluster_allocs, next_in_flight) {
> 
> -        uint64_t start = guest_offset;
> -        uint64_t end = start + bytes;
> -        uint64_t old_start = start_of_cluster(s, l2meta_cow_start(old_alloc));
> -        uint64_t old_end = ROUND_UP(l2meta_cow_end(old_alloc),
> s->cluster_size);
> +        uint64_t cow_start = l2meta_cow_start(old_alloc);
> +        uint64_t cow_end = l2meta_cow_end(old_alloc);
> +        uint64_t start = start_of_cluster(s, cow_start);
> +        uint64_t end = ROUND_UP(cow_end, s->cluster_size);
> 
> -        if (end <= old_start || start >= old_end) {
> +        range_init_nofail(&range2, start, end - start);
> +        if (!range_overlaps_range(&range1, &range2)) {
>              /* No intersection */
>              continue;
>          }
> 
> +        range_init_nofail(&range3, cow_start, cow_end - cow_start);
>          if (old_alloc->keep_old_clusters &&
> -            (end <= l2meta_cow_start(old_alloc) ||
> -             start >= l2meta_cow_end(old_alloc)))
> -        {
> +            !range_overlaps_range(&range1, &range3)) {
>              /*
>               * Clusters intersect but COW areas don't. And cluster itself is
>               * already allocated. So, there is no actual conflict.
> @@ -1434,9 +1437,9 @@ static int coroutine_fn
> handle_dependencies(BlockDriverState *bs,
> 
>          /* Conflict */
> 
> -        if (start < old_start) {
> +        if (guest_offset < start) {
>              /* Stop at the start of a running allocation */
> -            bytes = old_start - start;
> +            bytes = start - guest_offset;
>          } else {
>              bytes = 0;
>          }
> --
> 2.41.0

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

[Philippe Mathieu-Daudé]

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?

> To avoid the above problems and improve the readability of the code,
> it is better to use the ranges_overlap() to do this check.
> 
> Yao Xingtao (13):
>    range: Make ranges_overlap() return bool
>    arm/boot: make range overlap check more readable
>    core/loader: make range overlap check more readable
>    cxl/mailbox: make range overlap check more readable
>    display/sm501: make range overlap check more readable
>    aspeed_smc: make range overlap check more readable
>    qtest/fuzz: make range overlap check more readable
>    sparc/ldst_helper: make range overlap check more readable
>    system/memory_mapping: make range overlap check more readable
>    block/vhdx: make range overlap check more readable
>    crypto/block-luks: make range overlap check more readable
>    dump: make range overlap check more readable
>    block/qcow2-cluster: make range overlap check more readable

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

[Xingtao Yao (Fujitsu) via]

> -----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)
|

- (E2 <= E3) || (E1 >= E4)
+ !ranges_overlap(E1, E2, E3, E4)
|

- E1 < E4 && E2 > E3
+ ranges_overlap(E1, E2, E3, E4)
|

- (E1 < E4) && (E2 > E3)
+ ranges_overlap(E1, E2, E3, E4)
|

- (E1 >= E3 && E1 < E4) || (E2 > E3 && E2 <= E4)
+ ranges_overlap(E1, E2, E3, E4)

|
- ((E1 >= E3) && (E1 < E4)) || ((E2 > E3) && (E2 <= E4))
+ ranges_overlap(E1, E2, E3, E4)
)

then execute the command:
# spatch --macro-file scripts/cocci-macro-file.h --sp-file range_overlap.cocci --keep-comments --in-place --use-gitgrep --dir .

but some of the matched cases are not valid and need to be 
manually judged.

there may be cases that have not been matched yet.

> 
> > To avoid the above problems and improve the readability of the code,
> > it is better to use the ranges_overlap() to do this check.
> >
> > Yao Xingtao (13):
> >    range: Make ranges_overlap() return bool
> >    arm/boot: make range overlap check more readable
> >    core/loader: make range overlap check more readable
> >    cxl/mailbox: make range overlap check more readable
> >    display/sm501: make range overlap check more readable
> >    aspeed_smc: make range overlap check more readable
> >    qtest/fuzz: make range overlap check more readable
> >    sparc/ldst_helper: make range overlap check more readable
> >    system/memory_mapping: make range overlap check more readable
> >    block/vhdx: make range overlap check more readable
> >    crypto/block-luks: make range overlap check more readable
> >    dump: make range overlap check more readable
> >    block/qcow2-cluster: make range overlap check more readable

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

[Philippe Mathieu-Daudé]

On 22/7/24 08:59, Xingtao Yao (Fujitsu) 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)
> |
> 
> - (E2 <= E3) || (E1 >= E4)
> + !ranges_overlap(E1, E2, E3, E4)
> |
> 
> - E1 < E4 && E2 > E3
> + ranges_overlap(E1, E2, E3, E4)
> |
> 
> - (E1 < E4) && (E2 > E3)
> + ranges_overlap(E1, E2, E3, E4)
> |
> 
> - (E1 >= E3 && E1 < E4) || (E2 > E3 && E2 <= E4)
> + ranges_overlap(E1, E2, E3, E4)
> 
> |
> - ((E1 >= E3) && (E1 < E4)) || ((E2 > E3) && (E2 <= E4))
> + ranges_overlap(E1, E2, E3, E4)
> )

Please add to scripts/coccinelle/range.cocci.

> 
> then execute the command:
> # spatch --macro-file scripts/cocci-macro-file.h --sp-file range_overlap.cocci --keep-comments --in-place --use-gitgrep --dir .
> 
> but some of the matched cases are not valid and need to be
> manually judged.
> 
> there may be cases that have not been matched yet.

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

[Xingtao Yao (Fujitsu) via]

> -----Original Message-----
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Sent: Monday, July 22, 2024 3:37 PM
> To: Yao, Xingtao/姚 幸涛 <yaoxt.fnst@fujitsu.com>; qemu-devel@nongnu.org
> Subject: Re: [PATCH 00/13] make range overlap check more readable
> 
> On 22/7/24 08:59, Xingtao Yao (Fujitsu) 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)
> > |
> >
> > - (E2 <= E3) || (E1 >= E4)
> > + !ranges_overlap(E1, E2, E3, E4)
> > |
> >
> > - E1 < E4 && E2 > E3
> > + ranges_overlap(E1, E2, E3, E4)
> > |
> >
> > - (E1 < E4) && (E2 > E3)
> > + ranges_overlap(E1, E2, E3, E4)
> > |
> >
> > - (E1 >= E3 && E1 < E4) || (E2 > E3 && E2 <= E4)
> > + ranges_overlap(E1, E2, E3, E4)
> >
> > |
> > - ((E1 >= E3) && (E1 < E4)) || ((E2 > E3) && (E2 <= E4))
> > + ranges_overlap(E1, E2, E3, E4)
> > )
> 
> Please add to scripts/coccinelle/range.cocci.
OK, I will add this file in next revision.

> 
> >
> > then execute the command:
> > # spatch --macro-file scripts/cocci-macro-file.h --sp-file range_overlap.cocci
> --keep-comments --in-place --use-gitgrep --dir .
> >
> > but some of the matched cases are not valid and need to be
> > manually judged.
> >
> > there may be cases that have not been matched yet.

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

[Philippe Mathieu-Daudé]

Hi Yao,

On 22/7/24 09:40, Xingtao Yao (Fujitsu) wrote:

>> Please add to scripts/coccinelle/range.cocci.
> OK, I will add this file in next revision.

Please Cc me on v3 (I'm dropping v2 from my queue).

Thanks,

Phil.

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

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

[Xingtao Yao (Fujitsu) via]

> -----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

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

[Peter Maydell]

On Fri, 26 Jul 2024 at 01:16, Xingtao Yao (Fujitsu)
<yaoxt.fnst@fujitsu.com> wrote:
>
>
>
> > -----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.

Ah, I see -- you just used coccinelle as a more flexible grep,
effectively. That's fine -- it just means that when we review
the series we need to review each patch for e.g. off-by-one
errors rather than being able to do that only on the
coccinelle patch.

thanks
-- PMM