[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH v2 4/4] bitmaps/bitops: Clarify tests for small constant size.


  • To: <xen-devel@xxxxxxxxxxxxx>
  • From: Tim Deegan <tim@xxxxxxx>
  • Date: Thu, 27 Feb 2014 14:27:13 +0000
  • Delivery-date: Thu, 27 Feb 2014 14:27:28 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xen.org>

No semantic changes, just makes the control flow a bit clearer.

I was looking at this bcause the (-!__builtin_constant_p(x) | x__)
formula is too clever for Coverity, but in fact it always takes me a
minute or two to understand it too. :)

Signed-off-by: Tim Deegan <tim@xxxxxxx>

---

v2: fix find_next_bit macros to evaluate 'addr' exactly once.
---
 xen/include/asm-x86/bitops.h | 62 ++++++++++++++++++++------------------------
 xen/include/xen/bitmap.h     | 30 ++++++++++++---------
 2 files changed, 46 insertions(+), 46 deletions(-)

diff --git a/xen/include/asm-x86/bitops.h b/xen/include/asm-x86/bitops.h
index ab21d92..05ed2d7 100644
--- a/xen/include/asm-x86/bitops.h
+++ b/xen/include/asm-x86/bitops.h
@@ -335,23 +335,20 @@ static inline unsigned int __scanbit(unsigned long val, 
unsigned long max)
  * @offset: The bitnumber to start searching at
  * @size: The maximum size to search
  */
-#define find_next_bit(addr, size, off) ({ \
-    unsigned int r__ = (size); \
-    unsigned int o__ = (off); \
-    switch ( -!__builtin_constant_p(size) | r__ ) \
-    { \
-    case 0: (void)(addr); break; \
-    case 1 ... BITS_PER_LONG: \
-        r__ = o__ + __scanbit(*(const unsigned long *)(addr) >> o__, r__); \
-        break; \
-    default: \
-        if ( __builtin_constant_p(off) && !o__ ) \
-            r__ = __find_first_bit(addr, r__); \
-        else \
-            r__ = __find_next_bit(addr, r__, o__); \
-        break; \
-    } \
-    r__; \
+#define find_next_bit(addr, size, off) ({                                   \
+    unsigned int r__;                                                       \
+    const unsigned long *a__ = (addr);                                      \
+    unsigned int s__ = (size);                                              \
+    unsigned int o__ = (off);                                               \
+    if ( __builtin_constant_p(size) && s__ == 0 )                           \
+        r__ = s__;                                                          \
+    else if ( __builtin_constant_p(size) && s__ <= BITS_PER_LONG )          \
+        r__ = o__ + __scanbit(*(const unsigned long *)(a__) >> o__, s__);   \
+    else if ( __builtin_constant_p(off) && !o__ )                           \
+        r__ = __find_first_bit(a__, s__);                                   \
+    else                                                                    \
+        r__ = __find_next_bit(a__, s__, o__);                               \
+    r__;                                                                    \
 })
 
 /**
@@ -370,23 +367,20 @@ static inline unsigned int __scanbit(unsigned long val, 
unsigned long max)
  * @offset: The bitnumber to start searching at
  * @size: The maximum size to search
  */
-#define find_next_zero_bit(addr, size, off) ({ \
-    unsigned int r__ = (size); \
-    unsigned int o__ = (off); \
-    switch ( -!__builtin_constant_p(size) | r__ ) \
-    { \
-    case 0: (void)(addr); break; \
-    case 1 ... BITS_PER_LONG: \
-        r__ = o__ + __scanbit(~*(const unsigned long *)(addr) >> o__, r__); \
-        break; \
-    default: \
-        if ( __builtin_constant_p(off) && !o__ ) \
-            r__ = __find_first_zero_bit(addr, r__); \
-        else \
-            r__ = __find_next_zero_bit(addr, r__, o__); \
-        break; \
-    } \
-    r__; \
+#define find_next_zero_bit(addr, size, off) ({                              \
+    unsigned int r__;                                                       \
+    const unsigned long *a__ = (addr);                                      \
+    unsigned int s__ = (size);                                              \
+    unsigned int o__ = (off);                                               \
+    if ( __builtin_constant_p(size) && s__ == 0 )                           \
+        r__ = s__;                                                          \
+    else if ( __builtin_constant_p(size) && s__ <= BITS_PER_LONG )          \
+        r__ = o__ + __scanbit(~*(const unsigned long *)(a__) >> o__, s__);  \
+    else if ( __builtin_constant_p(off) && !o__ )                           \
+        r__ = __find_first_zero_bit(a__, s__);                              \
+    else                                                                    \
+        r__ = __find_next_zero_bit(a__, s__, o__);                          \
+    r__;                                                                    \
 })
 
 /**
diff --git a/xen/include/xen/bitmap.h b/xen/include/xen/bitmap.h
index b5ec455..166e1a0 100644
--- a/xen/include/xen/bitmap.h
+++ b/xen/include/xen/bitmap.h
@@ -110,13 +110,14 @@ extern int bitmap_allocate_region(unsigned long *bitmap, 
int pos, int order);
 
 #define bitmap_bytes(nbits) (BITS_TO_LONGS(nbits) * sizeof(unsigned long))
 
-#define bitmap_switch(nbits, zero_ret, small, large)                   \
-       switch (-!__builtin_constant_p(nbits) | (nbits)) {              \
-       case 0: return zero_ret;                                        \
-       case 1 ... BITS_PER_LONG:                                       \
-               small; break;                                           \
-       default:                                                        \
-               large; break;                                           \
+#define bitmap_switch(nbits, zero, small, large)                         \
+       unsigned int n__ = (nbits);                                       \
+       if (__builtin_constant_p(nbits) && n__ == 0) {                    \
+               zero;                                                     \
+       } else if (__builtin_constant_p(nbits) && n__ <= BITS_PER_LONG) { \
+               small;                                                    \
+       } else {                                                          \
+               large;                                                    \
        }
 
 static inline void bitmap_zero(unsigned long *dst, int nbits)
@@ -191,7 +192,8 @@ static inline void bitmap_complement(unsigned long *dst, 
const unsigned long *sr
 static inline int bitmap_equal(const unsigned long *src1,
                        const unsigned long *src2, int nbits)
 {
-       bitmap_switch(nbits, -1,
+       bitmap_switch(nbits,
+               return -1,
                return !((*src1 ^ *src2) & BITMAP_LAST_WORD_MASK(nbits)),
                return __bitmap_equal(src1, src2, nbits));
 }
@@ -199,7 +201,8 @@ static inline int bitmap_equal(const unsigned long *src1,
 static inline int bitmap_intersects(const unsigned long *src1,
                        const unsigned long *src2, int nbits)
 {
-       bitmap_switch(nbits, -1,
+       bitmap_switch(nbits,
+               return -1,
                return ((*src1 & *src2) & BITMAP_LAST_WORD_MASK(nbits)) != 0,
                return __bitmap_intersects(src1, src2, nbits));
 }
@@ -207,21 +210,24 @@ static inline int bitmap_intersects(const unsigned long 
*src1,
 static inline int bitmap_subset(const unsigned long *src1,
                        const unsigned long *src2, int nbits)
 {
-       bitmap_switch(nbits, -1,
+       bitmap_switch(nbits,
+               return -1,
                return !((*src1 & ~*src2) & BITMAP_LAST_WORD_MASK(nbits)),
                return __bitmap_subset(src1, src2, nbits));
 }
 
 static inline int bitmap_empty(const unsigned long *src, int nbits)
 {
-       bitmap_switch(nbits, -1,
+       bitmap_switch(nbits,
+               return -1,
                return !(*src & BITMAP_LAST_WORD_MASK(nbits)),
                return __bitmap_empty(src, nbits));
 }
 
 static inline int bitmap_full(const unsigned long *src, int nbits)
 {
-       bitmap_switch(nbits, -1,
+       bitmap_switch(nbits,
+               return -1,
                return !(~*src & BITMAP_LAST_WORD_MASK(nbits)),
                return __bitmap_full(src, nbits));
 }
-- 
1.8.5.2


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.