hash: provide asserts to verify API use

- converted the Curl_hash_count() macro to a function

- Discourage accessing struct fields directly

- Document the internal API in HASH.md

Closes #14503

lib/conncache.c

@@ -123,6 +123,9 @@ static void free_bundle_hash_entry(void *freethis)
 int Curl_conncache_init(struct conncache *connc,
                         struct Curl_multi *multi, size_t size)
 {
+  Curl_hash_init(&connc->hash, size, Curl_hash_str,
+                 Curl_str_key_compare, free_bundle_hash_entry);
+
   /* allocate a new easy handle to use when closing cached connections */
   connc->closure_handle = curl_easy_init();
   if(!connc->closure_handle)
@@ -133,8 +136,6 @@ int Curl_conncache_init(struct conncache *connc,
     connc->closure_handle->set.verbose = true;
 #endif
 
-  Curl_hash_init(&connc->hash, size, Curl_hash_str,
-                 Curl_str_key_compare, free_bundle_hash_entry);
   connc->closure_handle->state.conn_cache = connc;
   connc->multi = multi;
   Curl_llist_init(&connc->shutdowns.conn_list, NULL);

lib/hash.c

@@ -33,6 +33,10 @@
 /* The last #include file should be: */
 #include "memdebug.h"
 
+/* random patterns for API verification */
+#define HASHINIT 0x7017e781
+#define ITERINIT 0x5FEDCBA9
+
 static void
 hash_element_dtor(void *user, void *element)
 {
@@ -77,6 +81,9 @@ Curl_hash_init(struct Curl_hash *h,
   h->dtor = dtor;
   h->size = 0;
   h->slots = slots;
+#ifdef DEBUGBUILD
+  h->init = HASHINIT;
+#endif
 }
 
 static struct Curl_hash_element *
@@ -107,6 +114,7 @@ void *Curl_hash_add2(struct Curl_hash *h, void *key, size_t key_len, void *p,
 
   DEBUGASSERT(h);
   DEBUGASSERT(h->slots);
+  DEBUGASSERT(h->init == HASHINIT);
   if(!h->table) {
     size_t i;
     h->table = malloc(h->slots * sizeof(struct Curl_llist));
@@ -160,6 +168,7 @@ int Curl_hash_delete(struct Curl_hash *h, void *key, size_t key_len)
 {
   DEBUGASSERT(h);
   DEBUGASSERT(h->slots);
+  DEBUGASSERT(h->init == HASHINIT);
   if(h->table) {
     struct Curl_llist_node *le;
     struct Curl_llist *l = FETCH_LIST(h, key, key_len);
@@ -184,6 +193,7 @@ void *
 Curl_hash_pick(struct Curl_hash *h, void *key, size_t key_len)
 {
   DEBUGASSERT(h);
+  DEBUGASSERT(h->init == HASHINIT);
   if(h->table) {
     struct Curl_llist_node *le;
     struct Curl_llist *l;
@@ -210,6 +220,7 @@ Curl_hash_pick(struct Curl_hash *h, void *key, size_t key_len)
 void
 Curl_hash_destroy(struct Curl_hash *h)
 {
+  DEBUGASSERT(h->init == HASHINIT);
   if(h->table) {
     size_t i;
     for(i = 0; i < h->slots; ++i) {
@@ -231,6 +242,12 @@ Curl_hash_clean(struct Curl_hash *h)
   Curl_hash_clean_with_criterium(h, NULL, NULL);
 }
 
+size_t Curl_hash_count(struct Curl_hash *h)
+{
+  DEBUGASSERT(h->init == HASHINIT);
+  return h->size;
+}
+
 /* Cleans all entries that pass the comp function criteria. */
 void
 Curl_hash_clean_with_criterium(struct Curl_hash *h, void *user,
@@ -241,6 +258,7 @@ Curl_hash_clean_with_criterium(struct Curl_hash *h, void *user,
   if(!h || !h->table)
     return;
 
+  DEBUGASSERT(h->init == HASHINIT);
   for(i = 0; i < h->slots; ++i) {
     struct Curl_llist *list = &h->table[i];
     struct Curl_llist_node *le =
@@ -285,16 +303,21 @@ size_t Curl_str_key_compare(void *k1, size_t key1_len,
 void Curl_hash_start_iterate(struct Curl_hash *hash,
                              struct Curl_hash_iterator *iter)
 {
+  DEBUGASSERT(hash->init == HASHINIT);
   iter->hash = hash;
   iter->slot_index = 0;
   iter->current_element = NULL;
+#ifdef DEBUGBUILD
+  iter->init = ITERINIT;
+#endif
 }
 
 struct Curl_hash_element *
 Curl_hash_next_element(struct Curl_hash_iterator *iter)
 {
-  struct Curl_hash *h = iter->hash;
-
+  struct Curl_hash *h;
+  DEBUGASSERT(iter->init == ITERINIT);
+  h = iter->hash;
   if(!h->table)
     return NULL; /* empty hash, nothing to return */
 

lib/hash.h

@@ -56,6 +56,9 @@ struct Curl_hash {
   Curl_hash_dtor   dtor;
   size_t slots;
   size_t size;
+#ifdef DEBUGBUILD
+  int init;
+#endif
 };
 
 typedef void (*Curl_hash_elem_dtor)(void *key, size_t key_len, void *p);
@@ -65,6 +68,9 @@ struct Curl_hash_element {
   void   *ptr;
   Curl_hash_elem_dtor dtor;
   size_t key_len;
+#ifdef DEBUGBUILD
+  int init;
+#endif
   char   key[1]; /* allocated memory following the struct */
 };
 
@@ -72,6 +78,9 @@ struct Curl_hash_iterator {
   struct Curl_hash *hash;
   size_t slot_index;
   struct Curl_llist_node *current_element;
+#ifdef DEBUGBUILD
+  int init;
+#endif
 };
 
 void Curl_hash_init(struct Curl_hash *h,
@@ -85,8 +94,9 @@ void *Curl_hash_add2(struct Curl_hash *h, void *key, size_t key_len, void *p,
                      Curl_hash_elem_dtor dtor);
 int Curl_hash_delete(struct Curl_hash *h, void *key, size_t key_len);
 void *Curl_hash_pick(struct Curl_hash *, void *key, size_t key_len);
-#define Curl_hash_count(h) ((h)->size)
+
 void Curl_hash_destroy(struct Curl_hash *h);
+size_t Curl_hash_count(struct Curl_hash *h);
 void Curl_hash_clean(struct Curl_hash *h);
 void Curl_hash_clean_with_criterium(struct Curl_hash *h, void *user,
                                     int (*comp)(void *, void *));

lib/hostip.c

@@ -257,7 +257,8 @@ void Curl_hostcache_prune(struct Curl_easy *data)
 
     /* if the cache size is still too big, use the oldest age as new
        prune limit */
-  } while(timeout && (data->dns.hostcache->size > MAX_DNS_CACHE_SIZE));
+  } while(timeout &&
+          (Curl_hash_count(data->dns.hostcache) > MAX_DNS_CACHE_SIZE));
 
   if(data->share)
     Curl_share_unlock(data, CURL_LOCK_DATA_DNS);

lib/multi.c

@@ -247,10 +247,8 @@ static size_t trhash(void *key, size_t key_length, size_t slots_num)
 
 static size_t trhash_compare(void *k1, size_t k1_len, void *k2, size_t k2_len)
 {
-  (void)k1_len;
   (void)k2_len;
-
-  return *(struct Curl_easy **)k1 == *(struct Curl_easy **)k2;
+  return !memcmp(k1, k2, k1_len);
 }
 
 static void trhash_dtor(void *nada)
@@ -2929,18 +2927,24 @@ CURLMcode Curl_multi_pollset_ev(struct Curl_multi *multi,
     }
     if(last_action && (last_action != cur_action)) {
       /* Socket was used already, but different action now */
-      if(last_action & CURL_POLL_IN)
+      if(last_action & CURL_POLL_IN) {
+        DEBUGASSERT(entry->readers);
         entry->readers--;
-      if(last_action & CURL_POLL_OUT)
+      }
+      if(last_action & CURL_POLL_OUT) {
+        DEBUGASSERT(entry->writers);
         entry->writers--;
-      if(cur_action & CURL_POLL_IN)
+      }
+      if(cur_action & CURL_POLL_IN) {
         entry->readers++;
+      }
       if(cur_action & CURL_POLL_OUT)
         entry->writers++;
     }
     else if(!last_action &&
             !Curl_hash_pick(&entry->transfers, (char *)&data, /* hash key */
                             sizeof(struct Curl_easy *))) {
+      DEBUGASSERT(entry->users < 100000); /* detect weird values */
       /* a new transfer using this socket */
       entry->users++;
       if(cur_action & CURL_POLL_IN)
@@ -3002,6 +3006,7 @@ CURLMcode Curl_multi_pollset_ev(struct Curl_multi *multi,
     if(entry) {
       unsigned char oldactions = last_ps->actions[i];
       /* this socket has been removed. Decrease user count */
+      DEBUGASSERT(entry->users);
       entry->users--;
       if(oldactions & CURL_POLL_OUT)
         entry->writers--;

lib/share.c

@@ -224,8 +224,11 @@ curl_share_cleanup(struct Curl_share *share)
     return CURLSHE_IN_USE;
   }
 
-  Curl_conncache_close_all_connections(&share->conn_cache);
-  Curl_conncache_destroy(&share->conn_cache);
+  if(share->specifier & (1 << CURL_LOCK_DATA_CONNECT)) {
+    /* avoid the hash if it was never initialized */
+    Curl_conncache_close_all_connections(&share->conn_cache);
+    Curl_conncache_destroy(&share->conn_cache);
+  }
   Curl_hash_destroy(&share->hostcache);
 
 #if !defined(CURL_DISABLE_HTTP) && !defined(CURL_DISABLE_COOKIES)