Commit 7f8f9ef1 introduced the ability to store a list of integers as a sorted list of ranges, but when merging ranges, it leaks one or more ranges. It was also using range_get_last() incorrectly within range_compare() (a range is a start/end pair, but range_get_last() is for start/len pairs), and will also mishandle a range ending in UINT64_MAX (remember, we document that no range covers 2**64 bytes, but that ranges that end on UINT64_MAX have end < begin).
The whole merge algorithm was rather complex, especially since we are hard-coding things to a list of ranges; so just rewrite the thing to open-code the traversal and comparisons, making the range_compare() helper function give us a nicer answer, avoiding the need to pass any callbacks to g_list_*(). And reusing range_extend() ensures we cover the corner cases correctly. Drop the now-unused range_merge() and ranges_can_merge(). Doing this lets test-string-{input,output}-visitor pass under valgrind without leaks. CC: qemu-sta...@nongnu.org Signed-off-by: Eric Blake <ebl...@redhat.com> --- include/qemu/range.h | 78 +++++++++++++++++++++------------------------------- 1 file changed, 31 insertions(+), 47 deletions(-) diff --git a/include/qemu/range.h b/include/qemu/range.h index 4a4801b..9955cca 100644 --- a/include/qemu/range.h +++ b/include/qemu/range.h @@ -59,67 +59,51 @@ static inline int ranges_overlap(uint64_t first1, uint64_t len1, return !(last2 < first1 || last1 < first2); } -/* 0,1 can merge with 1,2 but don't overlap */ -static inline bool ranges_can_merge(Range *range1, Range *range2) +/* Return -1 if @a < b, 1 if greater, and 0 if they overlap. */ +static inline int range_compare(Range *a, Range *b) { - return !(range1->end < range2->begin || range2->end < range1->begin); -} - -static inline void range_merge(Range *range1, Range *range2) -{ - 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)) { + if (a->end && a->end < b->begin) { return -1; - } else { + } + if (b->end && a->begin > b->end) { return 1; } + return 0; } +/* Insert @data into @list of ranges; caller no longer owns @data */ static inline GList *range_list_insert(GList *list, Range *data) { - GList *l, *next = NULL; - Range *r, *nextr; + GList *l = list; - if (!list) { - list = g_list_insert_sorted(list, data, range_compare); - return list; + /* Range lists require no empty ranges */ + assert(data->begin || data->end); + + /* Skip all list elements strictly less than data */ + while (l && range_compare(l->data, data) < 0) { + l = l->next; + } + + /* If no list, or rest of list exceeds data, insert data and done */ + if (!l || range_compare(l->data, data) > 0) { + return g_list_insert_before(list, l, data); } - nextr = data; - l = list; - while (l && l != next && nextr) { - r = l->data; - if (ranges_can_merge(r, nextr)) { - range_merge(r, nextr); - l = g_list_remove_link(l, next); - next = g_list_next(l); - if (next) { - nextr = next->data; - } else { - nextr = NULL; - } - } else { - l = g_list_next(l); + /* Merge data into current list element */ + range_extend(l->data, data); + g_free(data); + + /* Merge any subsequent list elements that now also overlap */ + while (l->next && range_compare(l->data, l->next->data) == 0) { + range_extend(l->data, l->next->data); + g_free(l->next->data); + /* We know we aren't deleting the list head, so shave time + * by passing l instead of list */ + if (g_list_delete_link(l, l->next) != l) { + abort(); } } - if (!l) { - list = g_list_insert_sorted(list, data, range_compare); - } - return list; } -- 2.5.5