[Qemu-devel] [PATCH 1/2] qapi: Simplify use of range.h

[Eric Blake <eblake@redhat.com>]

Calling our function g_list_insert_sorted_merged() is a misnomer,
since we are NOT writing a glib function.  Furthermore, we are
making every caller pass the same comparator function of
range_merge(): any caller that does otherwise would break
in weird ways since our internal call to ranges_can_merge() is
hard-coded to operate only on ranges, rather than paying
attention to the caller's comparator.

Better is to fix things so that callers don't have to care about
our internal comparator, and to pick a function name that makes
it obvious that we are operating specifically on a list of ranges
and not a generic list.  Plus, refactoring the code here will
make it easier to plug a memory leak in the next patch.

Note that no one used the return value of range_merge(), and that
it can only be called if its precondition was satisfied, so
simplify that while in the area.

The declaration of range_compare() had to be hoisted earlier
in the file.

CC: qemu-stable@nongnu.org
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 include/qemu/range.h         | 49 +++++++++++++++++++-------------------------
 qapi/string-input-visitor.c  | 17 ++++-----------
 qapi/string-output-visitor.c |  4 ++--
 3 files changed, 27 insertions(+), 43 deletions(-)

diff --git a/include/qemu/range.h b/include/qemu/range.h
index c903eb5..4a4801b 100644
--- a/include/qemu/range.h
+++ b/include/qemu/range.h
@@ -65,30 +65,36 @@ static inline bool ranges_can_merge(Range *range1, Range *range2)
     return !(range1->end < range2->begin || range2->end < range1->begin);
 }

-static inline int range_merge(Range *range1, Range *range2)
+static inline void range_merge(Range *range1, Range *range2)
 {
-    if (ranges_can_merge(range1, range2)) {
-        if (range1->end < range2->end) {
-            range1->end = range2->end;
-        }
-        if (range1->begin > range2->begin) {
-            range1->begin = range2->begin;
-        }
+    if (range1->end < range2->end) {
+        range1->end = range2->end;
+    }
+    if (range1->begin > range2->begin) {
+        range1->begin = range2->begin;
+    }
+}
+
+static inline gint range_compare(gconstpointer a, gconstpointer b)
+{
+    Range *ra = (Range *)a, *rb = (Range *)b;
+    if (ra->begin == rb->begin && ra->end == rb->end) {
         return 0;
+    } else if (range_get_last(ra->begin, ra->end) <
+               range_get_last(rb->begin, rb->end)) {
+        return -1;
+    } else {
+        return 1;
     }
-
-    return -1;
 }

-static inline GList *g_list_insert_sorted_merged(GList *list,
-                                                 gpointer data,
-                                                 GCompareFunc func)
+static inline GList *range_list_insert(GList *list, Range *data)
 {
     GList *l, *next = NULL;
     Range *r, *nextr;

     if (!list) {
-        list = g_list_insert_sorted(list, data, func);
+        list = g_list_insert_sorted(list, data, range_compare);
         return list;
     }

@@ -111,23 +117,10 @@ static inline GList *g_list_insert_sorted_merged(GList *list,
     }

     if (!l) {
-        list = g_list_insert_sorted(list, data, func);
+        list = g_list_insert_sorted(list, data, range_compare);
     }

     return list;
 }

-static inline gint range_compare(gconstpointer a, gconstpointer b)
-{
-    Range *ra = (Range *)a, *rb = (Range *)b;
-    if (ra->begin == rb->begin && ra->end == rb->end) {
-        return 0;
-    } else if (range_get_last(ra->begin, ra->end) <
-               range_get_last(rb->begin, rb->end)) {
-        return -1;
-    } else {
-        return 1;
-    }
-}
-
 #endif
diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index 30b5879..b546e5f 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -61,8 +61,7 @@ static int parse_str(StringInputVisitor *siv, const char *name, Error **errp)
                 cur = g_malloc0(sizeof(*cur));
                 cur->begin = start;
                 cur->end = start + 1;
-                siv->ranges = g_list_insert_sorted_merged(siv->ranges, cur,
-                                                          range_compare);
+                siv->ranges = range_list_insert(siv->ranges, cur);
                 cur = NULL;
                 str = NULL;
             } else if (*endptr == '-') {
@@ -76,10 +75,7 @@ static int parse_str(StringInputVisitor *siv, const char *name, Error **errp)
                         cur = g_malloc0(sizeof(*cur));
                         cur->begin = start;
                         cur->end = end + 1;
-                        siv->ranges =
-                            g_list_insert_sorted_merged(siv->ranges,
-                                                        cur,
-                                                        range_compare);
+                        siv->ranges = range_list_insert(siv->ranges, cur);
                         cur = NULL;
                         str = NULL;
                     } else if (*endptr == ',') {
@@ -87,10 +83,7 @@ static int parse_str(StringInputVisitor *siv, const char *name, Error **errp)
                         cur = g_malloc0(sizeof(*cur));
                         cur->begin = start;
                         cur->end = end + 1;
-                        siv->ranges =
-                            g_list_insert_sorted_merged(siv->ranges,
-                                                        cur,
-                                                        range_compare);
+                        siv->ranges = range_list_insert(siv->ranges, cur);
                         cur = NULL;
                     } else {
                         goto error;
@@ -103,9 +96,7 @@ static int parse_str(StringInputVisitor *siv, const char *name, Error **errp)
                 cur = g_malloc0(sizeof(*cur));
                 cur->begin = start;
                 cur->end = start + 1;
-                siv->ranges = g_list_insert_sorted_merged(siv->ranges,
-                                                          cur,
-                                                          range_compare);
+                siv->ranges = range_list_insert(siv->ranges, cur);
                 cur = NULL;
             } else {
                 goto error;
diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
index d013196..5ea395a 100644
--- a/qapi/string-output-visitor.c
+++ b/qapi/string-output-visitor.c
@@ -85,7 +85,7 @@ static void string_output_append(StringOutputVisitor *sov, int64_t a)
     Range *r = g_malloc0(sizeof(*r));
     r->begin = a;
     r->end = a + 1;
-    sov->ranges = g_list_insert_sorted_merged(sov->ranges, r, range_compare);
+    sov->ranges = range_list_insert(sov->ranges, r);
 }

 static void string_output_append_range(StringOutputVisitor *sov,
@@ -94,7 +94,7 @@ static void string_output_append_range(StringOutputVisitor *sov,
     Range *r = g_malloc0(sizeof(*r));
     r->begin = s;
     r->end = e + 1;
-    sov->ranges = g_list_insert_sorted_merged(sov->ranges, r, range_compare);
+    sov->ranges = range_list_insert(sov->ranges, r);
 }

 static void format_string(StringOutputVisitor *sov, Range *r, bool next,
-- 
2.5.5