hashtable: pass HT into hash function

When defining a custom hash function for a hashtable key, you typically start with:

  HT_START_KEY_DEFN(key)
  HT_DEF_KEY_FIELD(k, unsigned char *)
  HT_END_KEY_DEFN(KEY)

In this setup, the hash function signature requires keybuf and len as
parameters rather than the hashtable key itself. As a result,
accessing members of the hashtable structure becomes awkward, since
you must do something like:

  #define FROM_KEYBUF_TO_HT_KEY(keybuf, type) (type)((keybuf) - sizeof(HT_KEY))

  static uint64_t ht_hash(uint8_t *keybuf, size_t keylen)
  {
      KEY *k = FROM_KEYBUF_TO_HT_KEY(keybuf, KEY *);
      ...
  }

This kind of pointer arithmetic is both unnecessary and error-prone.
A cleaner approach is to pass the HT pointer directly into the hash
function. From there, you can safely cast it to the required type
without the pointer gymnastics.

Signed-off-by: Nikola Pajkovsky <nikolap@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Neil Horman <nhorman@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/28677)

diff --git a/crypto/hashtable/hashtable.c b/crypto/hashtable/hashtable.c
index e09e9149e3de633ad32114c9c9c9e614db67d82e..99971ef71fd16525da9d1e385a8c5e2cacfe7e76 100644 (file)
--- a/crypto/hashtable/hashtable.c
+++ b/crypto/hashtable/hashtable.c
@@ -176,6 +176,11 @@ static void internal_free_nop(HT_VALUE *v)
     return;
 }
 
+static uint64_t internal_ht_hash_fn(HT_KEY *key)
+{
+    return ossl_fnv1a_hash(key->keybuf, key->keysize);
+}
+
 HT *ossl_ht_new(const HT_CONFIG *conf)
 {
     HT *new = OPENSSL_zalloc(sizeof(*new));
@@ -222,7 +227,7 @@ HT *ossl_ht_new(const HT_CONFIG *conf)
         goto err;
 
     if (new->config.ht_hash_fn == NULL)
-        new->config.ht_hash_fn = ossl_fnv1a_hash;
+        new->config.ht_hash_fn = internal_ht_hash_fn;
 
     return new;
 
@@ -648,7 +653,7 @@ int ossl_ht_insert(HT *h, HT_KEY *key, HT_VALUE *data, HT_VALUE **olddata)
      * we have to take our lock here to prevent other changes
      * to the bucket list
      */
-    hash = h->config.ht_hash_fn(key->keybuf, key->keysize);
+    hash = h->config.ht_hash_fn(key);
 
     for (i = 0;
          (rc = ossl_ht_insert_locked(h, hash, newval, olddata)) == -1
@@ -677,7 +682,7 @@ HT_VALUE *ossl_ht_get(HT *h, HT_KEY *key)
     uint64_t ehash;
     int lockless_reads = h->config.lockless_reads;
 
-    hash = h->config.ht_hash_fn(key->keybuf, key->keysize);
+    hash = h->config.ht_hash_fn(key);
 
     md = ossl_rcu_deref(&h->md);
     neigh_idx = neigh_idx_start = hash & md->neighborhood_mask;
@@ -726,7 +731,7 @@ int ossl_ht_delete(HT *h, HT_KEY *key)
     if (h->config.lockless_reads)
         return 0;
 
-    hash = h->config.ht_hash_fn(key->keybuf, key->keysize);
+    hash = h->config.ht_hash_fn(key);
 
     neigh_idx = hash & h->md->neighborhood_mask;
     PREFETCH_NEIGHBORHOOD(h->md->neighborhoods[neigh_idx]);

diff --git a/include/internal/hashtable.h b/include/internal/hashtable.h
index fccb1c1915f4f392edc71f4344f49188e56af3e8..86d3518b77ec89ca46600eae6eecac01d64f35fe 100644 (file)
--- a/include/internal/hashtable.h
+++ b/include/internal/hashtable.h
@@ -50,7 +50,7 @@ typedef struct ht_value_list_st {
 typedef struct ht_config_st {
     OSSL_LIB_CTX *ctx;
     void (*ht_free_fn)(HT_VALUE *obj);
-    uint64_t (*ht_hash_fn)(uint8_t *key, size_t keylen);
+    uint64_t (*ht_hash_fn)(HT_KEY *key);
     size_t init_neighborhoods;
     uint32_t collision_check;
     uint32_t lockless_reads;

diff --git a/test/lhash_test.c b/test/lhash_test.c
index ea7f4942991c9abbac732b50b87dfc60884c7f45..c229df902c9139984bcf8c7bddf9f2805c29790c 100644 (file)
--- a/test/lhash_test.c
+++ b/test/lhash_test.c
@@ -207,9 +207,9 @@ static int int_foreach(HT_VALUE *v, void *arg)
     return 1;
 }
 
-static uint64_t hashtable_hash(uint8_t *key, size_t keylen)
+static uint64_t hashtable_hash(HT_KEY *key)
 {
-    return (uint64_t)(*(uint32_t *)key);
+    return (uint64_t)(*(uint32_t *)key->keybuf);
 }
 
 static int test_int_hashtable(void)