Switch to unsigned type for fr_lst_index_t (#4190)

* Switch to unsigned type for fr_lst_index_t

This involves adding one to the true index when storing it in an
item, and subtracting one when reading it for use.

* Switch to unsigned type for fr_lst_index_t

This involves adding one to the true index when storing it in an
item, and subtracting one when reading it for use.

* Switch to unsigned type for fr_lst_index_t

This involves adding one to the true index when storing it in an
item, and subtracting one when reading it for use.

diff --git a/src/lib/util/lst.c b/src/lib/util/lst.c
index 1895af73b4c2325f96995ee0be5541323dac3f96..11a6861cbdbab7baa288e1f9aa6f9f3bf87f8bcd 100644 (file)
--- a/src/lib/util/lst.c
+++ b/src/lib/util/lst.c
@@ -75,14 +75,30 @@ static inline CC_HINT(always_inline, nonnull) void *index_addr(fr_lst_t *lst, vo
        return ((uint8_t *)data) + (lst)->offset;
 }
 
+/*
+ * Concerning item_index() and item_index_set():
+ * To let zero be the value *as stored in an item* that indicates not being in an LST,
+ * we add one to the real index when storing it and subtract one when retrieving it.
+ *
+ * This lets the LST functions use item indices in [0, lst->capacity), important for
+ * 1. the circular array, which allows an important optimization for fr_lst_pop()
+ * 2. quick reduction of indices
+ *
+ * fr_item_insert() needs to see the value actually stored, hence raw_item_index().
+ */
+static inline CC_HINT(always_inline, nonnull) fr_lst_index_t raw_item_index(fr_lst_t *lst, void *data)
+{
+       return *(fr_lst_index_t *)index_addr(lst, data);
+}
+
 static inline CC_HINT(always_inline, nonnull) fr_lst_index_t item_index(fr_lst_t *lst, void *data)
 {
-       return (*(fr_lst_index_t *)index_addr(lst, data));
+       return  raw_item_index(lst, data) - 1;
 }
 
 static inline CC_HINT(always_inline, nonnull) void item_index_set(fr_lst_t *lst, void *data, fr_lst_index_t idx)
 {
-       (*(fr_lst_index_t *)index_addr(lst, data)) = idx;
+       (*(fr_lst_index_t *)index_addr(lst, data)) = idx + 1;
 }
 
 static inline CC_HINT(always_inline, nonnull) fr_lst_index_t index_reduce(fr_lst_t *lst, fr_lst_index_t idx)
@@ -690,7 +706,7 @@ int fr_lst_extract(fr_lst_t *lst, void *data)
                return -1;
        }
 
-       if (unlikely(item_index(lst, data) < 0)) {
+       if (unlikely(raw_item_index(lst, data) == 0)) {
                fr_strerror_const("Tried to extract element not in LST");
                return -1;
        }
@@ -701,8 +717,6 @@ int fr_lst_extract(fr_lst_t *lst, void *data)
 
 int fr_lst_insert(fr_lst_t *lst, void *data)
 {
-       fr_lst_index_t  data_index;
-
        /*
         * Expand if need be. Not in the paper, but we want the capability.
         */
@@ -711,9 +725,7 @@ int fr_lst_insert(fr_lst_t *lst, void *data)
        /*
         * Don't insert something that looks like it's already in an LST.
         */
-       data_index = item_index(lst, data);
-       if (unlikely((data_index > 0) ||
-           ((data_index == 0) && (lst->num_elements > 0) && (lst->idx == 0) && (item(lst, 0) == data)))) {
+       if (unlikely(raw_item_index(lst, data) > 0)) {
                fr_strerror_const("Node is already in the LST");
                return -1;
        }

diff --git a/src/lib/util/lst.h b/src/lib/util/lst.h
index 60720fcdda28f13f5bbf66b1405f3fe04ad106d6..8925c55b19d0cfad043cda6ecb41aca81609aead 100644 (file)
--- a/src/lib/util/lst.h
+++ b/src/lib/util/lst.h
@@ -41,7 +41,7 @@ typedef struct fr_lst_s       fr_lst_t;
  * type of a structure with a member of type fr_lst_index_t. That member's name must be
  * passed as the _field argument.
  */
-typedef int fr_lst_index_t;
+typedef unsigned int fr_lst_index_t;
 
 typedef fr_lst_index_t fr_lst_iter_t;
 
@@ -77,6 +77,12 @@ typedef int8_t (*fr_lst_cmp_t)(void const *a, void const *b);
 fr_lst_t *_fr_lst_alloc(TALLOC_CTX *ctx, fr_lst_cmp_t cmp, char const *type, size_t offset) CC_HINT(nonnull(2));
 
 /** Check if an entry is inserted into an LST.
+ *
+ * @param[in] lst_id           An fr_lst_index_t value *as stored in an item*
+ *
+ * Thus one should only pass this function an index as retrieved directly from
+ * the item, *not* the value returned by item_index() (q.v.).
+ *
  * This checks a necessary condition for a fr_lst_index_t value to be
  * that of an inserted entry. A more complete check would need the entry
  * itself and a pointer to the fr_lst_t it may be inserted in.
@@ -84,7 +90,7 @@ fr_lst_t *_fr_lst_alloc(TALLOC_CTX *ctx, fr_lst_cmp_t cmp, char const *type, siz
  */
 static inline bool fr_lst_entry_inserted(fr_lst_index_t lst_id)
 {
-       return (lst_id >= 0);
+       return (lst_id > 0);
 }
 
 void   *fr_lst_peek(fr_lst_t *lst) CC_HINT(nonnull);

diff --git a/src/lib/util/lst_tests.c b/src/lib/util/lst_tests.c
index 733328a56cbf4a5d1fbf2f8e9a3bd4b8cdee8f75..8e454a9cdc32be26ed287f98007e69072adae7c7 100644 (file)
--- a/src/lib/util/lst_tests.c
+++ b/src/lib/util/lst_tests.c
@@ -116,7 +116,7 @@ static void lst_test(int skip)
 
        TEST_CASE("deletions");
        for (int entry = 0; entry < LST_TEST_SIZE; entry += skip) {
-               TEST_CHECK(array[entry].index != -1);
+               TEST_CHECK(array[entry].index != 0);
                TEST_MSG("element %i removed out of order", entry);
 
                TEST_CHECK((ret = fr_lst_extract(lst, &array[entry])) >= 0);
@@ -125,7 +125,7 @@ static void lst_test(int skip)
                TEST_CHECK(!fr_lst_contains(lst, &array[entry]));
                TEST_MSG("element %i removed but still in LST", entry);
 
-               TEST_CHECK(array[entry].index == -1);
+               TEST_CHECK(array[entry].index == 0);
                TEST_MSG("element %i removed out of order", entry);
        }
 
@@ -342,7 +342,7 @@ static void lst_cycle(void)
        removed = 0;
 
        for (i = 0; i < LST_CYCLE_SIZE; i++) {
-               if (array[i].index == -1) {
+               if (array[i].index == 0) {
                        TEST_CHECK((ret = fr_lst_insert(lst, &array[i])) >= 0);
                        TEST_MSG("insert failed, returned %i - %s", ret, fr_strerror());
                        inserted++;
@@ -407,7 +407,7 @@ static void lst_iter(void)
        for (int i = 0; i < NVALUES; i++, data = fr_lst_iter_next(lst, &iter)) {
                TEST_CHECK(data != NULL);
                TEST_CHECK(!data->visited);
-               TEST_CHECK(data->index >= 0);
+               TEST_CHECK(data->index > 0);
                data->visited = true;
        }