xen/bitops: Fix break usage in for_each_set_bit() loop

for_each_set_bit()'s use of a double for loop had an accidental bug with a
break in the inner loop leading to an infinite outer loop.

Adjust for_each_set_bit() to avoid this behaviour, and add extend
test_for_each_set_bit() with a test case for this.

Fixes: ed26376f20bf ("xen/bitops: Introduce for_each_set_bit()")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Frediano Ziglio <frediano.ziglio@cloud.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

diff --git a/xen/common/bitops.c b/xen/common/bitops.c
index 91ae961440af1adf652daf1e9dd3f96ca089f21a..bd17ddef57008cdef10fa105220da1bf6c65f99b 100644 (file)
--- a/xen/common/bitops.c
+++ b/xen/common/bitops.c
@@ -86,7 +86,7 @@ static void __init test_fls(void)
 
 static void __init test_for_each_set_bit(void)
 {
-    unsigned int  ui,  ui_res = 0;
+    unsigned int  ui,  ui_res = 0, tmp;
     unsigned long ul,  ul_res = 0;
     uint64_t      ull, ull_res = 0;
 
@@ -110,6 +110,21 @@ static void __init test_for_each_set_bit(void)
 
     if ( ull != ull_res )
         panic("for_each_set_bit(uint64) expected %#"PRIx64", got %#"PRIx64"\n", ull, ull_res);
+
+    /* Check that we can break from the middle of the loop. */
+    ui = HIDE(0x80001008U);
+    tmp = 0;
+    ui_res = 0;
+    for_each_set_bit ( i, ui )
+    {
+        if ( tmp++ > 1 )
+            break;
+
+        ui_res |= 1U << i;
+    }
+
+    if ( ui_res != 0x1008 )
+        panic("for_each_set_bit(break) expected 0x1008, got %#x\n", ui_res);
 }
 
 static void __init test_multiple_bits_set(void)

diff --git a/xen/include/xen/bitops.h b/xen/include/xen/bitops.h
index 79615fb89d040d2f25bca13c393eb41ae507f46c..448b2d3e09377e237399f7a2540efe27db3ca9ed 100644 (file)
--- a/xen/include/xen/bitops.h
+++ b/xen/include/xen/bitops.h
@@ -299,7 +299,7 @@ static always_inline attr_const unsigned int fls64(uint64_t x)
  * A copy of @val is taken internally.
  */
 #define for_each_set_bit(iter, val)                     \
-    for ( typeof(val) __v = (val); __v; )               \
+    for ( typeof(val) __v = (val); __v; __v = 0 )       \
         for ( unsigned int (iter);                      \
               __v && ((iter) = ffs_g(__v) - 1, true);   \
               __v &= __v - 1 )