From: Matthew Fernandez <matthew.fernandez@gmail.com>
Date: Sun, 25 Dec 2022 21:53:03 +0000 (-0800)
Subject: cgraph: add sorting functionality to the generic list
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=08923001def734675bb77493118c4a1dbcc7f617;p=graphviz

cgraph: add sorting functionality to the generic list

Note that this implementation needed to guard the call to `qsort` with a check
that the list is non-empty. UBSan considers passing `NULL` as the first
parameter to `qsort` undefined behavior, even when `nmemb` is 0. I do not know
why this is. I cannot find any text in `qsort` references or ISO C99 to justify
this. But it is simple enough to avoid.

Similarly, passing 0 as `size` causes out-of-bounds accesses in Glibc’s `qsort`.
It is possible there is some text in ISO C99 regarding sizes that indirectly
implies this value must be non-zero. For this implementation, we rely on this
macro never being instantiated with a 0-sized type. It is possible 0-sized types
themselves are impossible without C++’s Empty Base Optimization. I only stumbled
across this quirk of `qsort` by accidentally swapping the `size` and `nmemb`
parameters.
---

diff --git a/lib/cgraph/list.h b/lib/cgraph/list.h
index 41be77ac7..f0415c162 100644
--- a/lib/cgraph/list.h
+++ b/lib/cgraph/list.h
@@ -164,6 +164,19 @@
     }                                                                          \
   }                                                                            \
                                                                                \
+  /** sort the list using the given comparator */                              \
+  static inline LIST_UNUSED void name##_sort(                                  \
+      name##_t *list, int (*cmp)(const type *a, const type *b)) {              \
+    assert(list != NULL);                                                      \
+    assert(cmp != NULL);                                                       \
+                                                                               \
+    int (*compar)(const void *, const void *) =                                \
+        (int (*)(const void *, const void *))cmp;                              \
+    if (list->size > 0) {                                                      \
+      qsort(list->data, list->size, sizeof(type), compar);                     \
+    }                                                                          \
+  }                                                                            \
+                                                                               \
   /** deallocate unused backing storage, shrinking capacity to size */         \
   static inline LIST_UNUSED void name##_shrink_to_fit(name##_t *list) {        \
     assert(list != NULL);                                                      \
diff --git a/lib/cgraph/test_list.c b/lib/cgraph/test_list.c
index ac5cb1a11..1ffe47455 100644
--- a/lib/cgraph/test_list.c
+++ b/lib/cgraph/test_list.c
@@ -151,6 +151,108 @@ static void test_resize_to_0(void) {
   ints_free(&xs);
 }
 
+/// an int comparer
+static int cmp_int(const int *a, const int *b) {
+  if (*a < *b) {
+    return -1;
+  }
+  if (*a > *b) {
+    return 1;
+  }
+  return 0;
+}
+
+/// sort on an empty list should be a no-op
+static void test_sort_empty(void) {
+  ints_t xs = {0};
+  ints_sort(&xs, cmp_int);
+  assert(ints_size(&xs) == 0);
+  ints_free(&xs);
+}
+
+static void test_sort(void) {
+  ints_t xs = {0};
+
+  // a list of ints in an arbitrary order
+  const int ys[] = {4, 2, 10, 5, -42, 3};
+
+  // setup this list and sort it
+  for (size_t i = 0; i < sizeof(ys) / sizeof(ys[0]); ++i) {
+    ints_append(&xs, ys[i]);
+  }
+  ints_sort(&xs, cmp_int);
+
+  // we should now have a sorted version of `ys`
+  assert(ints_size(&xs) == sizeof(ys) / sizeof(ys[0]));
+  assert(ints_get(&xs, 0) == -42);
+  assert(ints_get(&xs, 1) == 2);
+  assert(ints_get(&xs, 2) == 3);
+  assert(ints_get(&xs, 3) == 4);
+  assert(ints_get(&xs, 4) == 5);
+  assert(ints_get(&xs, 5) == 10);
+
+  ints_free(&xs);
+}
+
+/// sorting an already sorted list should be a no-op
+static void test_sort_sorted(void) {
+  ints_t xs = {0};
+  const int ys[] = {-42, 2, 3, 4, 5, 10};
+
+  for (size_t i = 0; i < sizeof(ys) / sizeof(ys[0]); ++i) {
+    ints_append(&xs, ys[i]);
+  }
+  ints_sort(&xs, cmp_int);
+
+  for (size_t i = 0; i < sizeof(ys) / sizeof(ys[0]); ++i) {
+    assert(ints_get(&xs, i) == ys[i]);
+  }
+
+  ints_free(&xs);
+}
+
+typedef struct {
+  int x;
+  int y;
+} pair_t;
+
+DEFINE_LIST(pairs, pair_t)
+
+/// a pair comparer, using only the first element
+static int cmp_pair(const pair_t *a, const pair_t *b) {
+  if (a->x < b->x) {
+    return -1;
+  }
+  if (a->x > b->x) {
+    return 1;
+  }
+  return 0;
+}
+
+/// sorting a complex type should move entire values of the type together
+static void test_sort_complex(void) {
+  pairs_t xs = {0};
+
+  const pair_t ys[] = {{1, 2}, {-2, 3}, {-10, 4}, {0, 7}};
+
+  for (size_t i = 0; i < sizeof(ys) / sizeof(ys[0]); ++i) {
+    pairs_append(&xs, ys[i]);
+  }
+  pairs_sort(&xs, cmp_pair);
+
+  assert(pairs_size(&xs) == sizeof(ys) / sizeof(ys[0]));
+  assert(pairs_get(&xs, 0).x == -10);
+  assert(pairs_get(&xs, 0).y == 4);
+  assert(pairs_get(&xs, 1).x == -2);
+  assert(pairs_get(&xs, 1).y == 3);
+  assert(pairs_get(&xs, 2).x == 0);
+  assert(pairs_get(&xs, 2).y == 7);
+  assert(pairs_get(&xs, 3).x == 1);
+  assert(pairs_get(&xs, 3).y == 2);
+
+  pairs_free(&xs);
+}
+
 static void test_shrink(void) {
   ints_t xs = {0};
 
@@ -299,6 +401,10 @@ int main(void) {
   RUN(resize_empty_2);
   RUN(resize_down);
   RUN(resize_to_0);
+  RUN(sort_empty);
+  RUN(sort);
+  RUN(sort_sorted);
+  RUN(sort_complex);
   RUN(shrink);
   RUN(shrink_empty);
   RUN(free);