[Qemu-devel] [PULL 15/36] range: Replace internal representation of Range

From: "Michael S. Tsirkin" <mst@redhat.com>
To: qemu-devel@nongnu.org
Cc: Peter Maydell <peter.maydell@linaro.org>,
	Markus Armbruster <armbru@redhat.com>,
	Eric Blake <eblake@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>
Subject: [Qemu-devel] [PULL 15/36] range: Replace internal representation of Range
Date: Mon, 4 Jul 2016 19:47:03 +0300	[thread overview]
Message-ID: <20160704194703-mutt-send-email-mst@redhat.com> (raw)
In-Reply-To: <1467650742-17580-1-git-send-email-mst@redhat.com>

From: Markus Armbruster <armbru@redhat.com>

Range represents a range as follows.  Member @start is the inclusive
lower bound, member @end is the exclusive upper bound.  Zero @end is
special: if @start is also zero, the range is empty, else @end is to
be interpreted as 2^64.  No other empty ranges may occur.

The range [0,2^64-1] cannot be represented.  If you try to create it
with range_set_bounds1(), you get the empty range instead.  If you try
to create it with range_set_bounds() or range_extend(), assertions
fail.  Before range_set_bounds() existed, the open-coded creation
usually got you the empty range instead.  Open deathtrap.

Moreover, the code dealing with the janus-faced @end is too clever by
half.

Dumb this down to a more pedestrian representation: members @lob and
@upb are inclusive lower and upper bounds.  The empty range is encoded
as @lob = 1, @upb = 0.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/qemu/range.h | 56 +++++++++++++++++++++++++---------------------------
 util/range.c         | 16 +++++++--------
 2 files changed, 34 insertions(+), 38 deletions(-)

diff --git a/include/qemu/range.h b/include/qemu/range.h
index c8c46a9..f28f0c1 100644
--- a/include/qemu/range.h
+++ b/include/qemu/range.h
@@ -26,37 +26,38 @@
 /*
  * Operations on 64 bit address ranges.
  * Notes:
- *   - ranges must not wrap around 0, but can include the last byte ~0x0LL.
- *   - this can not represent a full 0 to ~0x0LL range.
+ * - Ranges must not wrap around 0, but can include UINT64_MAX.
  */
 
-/* A structure representing a range of addresses. */
 struct Range {
-    uint64_t begin; /* First byte of the range, or 0 if empty. */
-    uint64_t end;   /* 1 + the last byte. 0 if range empty or ends at ~0x0LL. */
+    /*
+     * Do not access members directly, use the functions!
+     * A non-empty range has @lob <= @upb.
+     * An empty range has @lob == @upb + 1.
+     */
+    uint64_t lob;        /* inclusive lower bound */
+    uint64_t upb;        /* inclusive upper bound */
 };
 
 static inline void range_invariant(Range *range)
 {
-    assert((!range->begin && !range->end) /* empty */
-           || range->begin <= range->end - 1); /* non-empty */
+    assert(range->lob <= range->upb || range->lob == range->upb + 1);
 }
 
 /* Compound literal encoding the empty range */
-#define range_empty ((Range){ .begin = 0, .end = 0 })
+#define range_empty ((Range){ .lob = 1, .upb = 0 })
 
 /* Is @range empty? */
 static inline bool range_is_empty(Range *range)
 {
     range_invariant(range);
-    return !range->begin && !range->end;
+    return range->lob > range->upb;
 }
 
 /* Does @range contain @val? */
 static inline bool range_contains(Range *range, uint64_t val)
 {
-    return !range_is_empty(range)
-        && val >= range->begin && val <= range->end - 1;
+    return val >= range->lob && val <= range->upb;
 }
 
 /* Initialize @range to the empty range */
@@ -71,14 +72,11 @@ static inline void range_make_empty(Range *range)
  * Both bounds are inclusive.
  * The interval must not be empty, i.e. @lob must be less than or
  * equal @upb.
- * The interval must not be [0,UINT64_MAX], because Range can't
- * represent that.
  */
 static inline void range_set_bounds(Range *range, uint64_t lob, uint64_t upb)
 {
-    assert(lob <= upb);
-    range->begin = lob;
-    range->end = upb + 1;       /* may wrap to zero, that's okay */
+    range->lob = lob;
+    range->upb = upb;
     assert(!range_is_empty(range));
 }
 
@@ -91,8 +89,12 @@ static inline void range_set_bounds(Range *range, uint64_t lob, uint64_t upb)
 static inline void range_set_bounds1(Range *range,
                                      uint64_t lob, uint64_t upb_plus1)
 {
-    range->begin = lob;
-    range->end = upb_plus1;
+    if (!lob && !upb_plus1) {
+        *range = range_empty;
+    } else {
+        range->lob = lob;
+        range->upb = upb_plus1 - 1;
+    }
     range_invariant(range);
 }
 
@@ -100,20 +102,18 @@ static inline void range_set_bounds1(Range *range,
 static inline uint64_t range_lob(Range *range)
 {
     assert(!range_is_empty(range));
-    return range->begin;
+    return range->lob;
 }
 
 /* Return @range's upper bound.  @range must not be empty. */
 static inline uint64_t range_upb(Range *range)
 {
     assert(!range_is_empty(range));
-    return range->end - 1;
+    return range->upb;
 }
 
 /*
  * Extend @range to the smallest interval that includes @extend_by, too.
- * This must not extend @range to cover the interval [0,UINT64_MAX],
- * because Range can't represent that.
  */
 static inline void range_extend(Range *range, Range *extend_by)
 {
@@ -124,15 +124,13 @@ static inline void range_extend(Range *range, Range *extend_by)
         *range = *extend_by;
         return;
     }
-    if (range->begin > extend_by->begin) {
-        range->begin = extend_by->begin;
+    if (range->lob > extend_by->lob) {
+        range->lob = extend_by->lob;
     }
-    /* Compare last byte in case region ends at ~0x0LL */
-    if (range->end - 1 < extend_by->end - 1) {
-        range->end = extend_by->end;
+    if (range->upb < extend_by->upb) {
+        range->upb = extend_by->upb;
     }
-    /* Must not extend to { .begin = 0, .end = 0 }: */
-    assert(!range_is_empty(range));
+    range_invariant(range);
 }
 
 /* Get last byte of a range from offset + length.
diff --git a/util/range.c b/util/range.c
index e5f2e71..416df7c 100644
--- a/util/range.c
+++ b/util/range.c
@@ -22,20 +22,18 @@
 #include "qemu/range.h"
 
 /*
- * Operations on 64 bit address ranges.
- * Notes:
- *   - ranges must not wrap around 0, but can include the last byte ~0x0LL.
- *   - this can not represent a full 0 to ~0x0LL range.
+ * Return -1 if @a < @b, 1 @a > @b, and 0 if they touch or overlap.
+ * Both @a and @b must not be empty.
  */
-
-/* Return -1 if @a < @b, 1 if greater, and 0 if they touch or overlap. */
 static inline int range_compare(Range *a, Range *b)
 {
-    /* Zero a->end is 2**64, and therefore not less than any b->begin */
-    if (a->end && a->end < b->begin) {
+    assert(!range_is_empty(a) && !range_is_empty(b));
+
+    /* Careful, avoid wraparound */
+    if (b->lob && b->lob - 1 > a->upb) {
         return -1;
     }
-    if (b->end && a->begin > b->end) {
+    if (a->lob && a->lob - 1 > b->upb) {
         return 1;
     }
     return 0;
-- 
MST
