lib: typesafe hash table breadcrumbs

Looking at the coverity report, it complains that tabshift could be
zero, resulting in a uint32_t shifted by 33 (which is undefined.)

As I was confused by the "+ 1", in addition to the SA assume(), leave
some breadcumbs for next time this comes up.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

diff --git a/lib/typesafe.c b/lib/typesafe.c
index 0da35d0f8c185ef5c94f0ed3826505996a817b93..c0774479852da261af67d70572da58b55f2c77fa 100644 (file)
--- a/lib/typesafe.c
+++ b/lib/typesafe.c
@@ -85,6 +85,15 @@ void typesafe_hash_grow(struct thash_head *head)
        uint32_t newsize = head->count, i, j;
        uint8_t newshift, delta;
 
+       /* note hash_grow is called after head->count++, so newsize is
+        * guaranteed to be >= 1.  So the minimum argument to builtin_ctz
+        * below is 2, which returns 1, and that makes newshift >= 2.
+        *
+        * Calling hash_grow with a zero head->count would result in a
+        * malformed hash table that has tabshift == 1.
+        */
+       assert(head->count > 0);
+
        hash_consistency_check(head);
 
        newsize |= newsize >> 1;

diff --git a/lib/typesafe.h b/lib/typesafe.h
index 3292b6ec8b89d6175a2a7dce1403e9a1d8685adf..66612be167cfe2265b1f07d8e12eff2bbebde061 100644 (file)
--- a/lib/typesafe.h
+++ b/lib/typesafe.h
@@ -783,6 +783,12 @@ struct thash_head {
        struct thash_item **entries;
        uint32_t count;
 
+       /* tabshift can be 0 if the hash table is empty and entries is NULL.
+        * otherwise it will always be 2 or larger because it contains
+        * the shift value *plus 1*.  This is a trick to make HASH_SIZE return
+        * the correct value (with the >> 1) for tabshift == 0, without needing
+        * a conditional branch.
+        */
        uint8_t tabshift;
        uint8_t minshift, maxshift;
 };
@@ -791,8 +797,11 @@ struct thash_head {
        ((1U << (tabshift)) >> 1)
 #define HASH_SIZE(head) \
        _HASH_SIZE((head).tabshift)
-#define _HASH_KEY(tabshift, val) \
-       ((val) >> (33 - (tabshift)))
+#define _HASH_KEY(tabshift, val)                                               \
+       ({                                                                     \
+               assume((tabshift) >= 2 && (tabshift) <= 33);                   \
+               (val) >> (33 - (tabshift));                                    \
+       })
 #define HASH_KEY(head, val) \
        _HASH_KEY((head).tabshift, val)
 #define HASH_GROW_THRESHOLD(head) \