[v4,02/19] tools/xenstore: let hashtable_add() fail in case of existing entry

Commit Message

Jürgen Groß Aug. 14, 2023, 7:46 a.m. UTC

Silently adding another entry with the same key to a hashtable is a
perfect receipt for later failure with hard to diagnose symptoms.

Let hashtable_add() fail in case another entry with the same key is
already existing.

Signed-off-by: Juergen Gross <[e-mail]>
Reviewed-by: Julien Grall <[e-mail]>
---
V2:
- split off from next patch (Julien Grall)
- fix coding style (Julien Grall)
- use for () loop (Julien Grall)
---
 tools/xenstore/hashtable.c | 40 ++++++++++++++++++++++++++------------
 tools/xenstore/hashtable.h |  6 ------
 2 files changed, 28 insertions(+), 18 deletions(-)

Patch

diff --git a/tools/xenstore/hashtable.c b/tools/xenstore/hashtable.c
index 9daddd9782..88e14c4c57 100644
--- a/tools/xenstore/hashtable.c
+++ b/tools/xenstore/hashtable.c
@@ -141,11 +141,34 @@  static int hashtable_expand(struct hashtable *h)
     return 0;
 }
 
+static struct entry *hashtable_search_entry(const struct hashtable *h,
+                                            const void *k)
+{
+    struct entry *e;
+    unsigned int hashvalue, index;
+
+    hashvalue = hash(h, k);
+    index = indexFor(h->tablelength, hashvalue);
+    e = h->table[index];
+
+    for (e = h->table[index]; e; e = e->next)
+    {
+        /* Check hash value to short circuit heavier comparison */
+        if ((hashvalue == e->h) && (h->eqfn(k, e->k)))
+            return e;
+    }
+
+    return NULL;
+}
+
 int hashtable_add(struct hashtable *h, const void *k, void *v)
 {
-    /* This method allows duplicate keys - but they shouldn't be used */
     unsigned int index;
     struct entry *e;
+
+    if (hashtable_search_entry(h, k))
+        return EEXIST;
+
     if (++(h->entrycount) > h->loadlimit)
     {
         /* Ignore the return value. If expand fails, we should
@@ -176,17 +199,10 @@  int hashtable_add(struct hashtable *h, const void *k, void *v)
 void *hashtable_search(const struct hashtable *h, const void *k)
 {
     struct entry *e;
-    unsigned int hashvalue, index;
-    hashvalue = hash(h,k);
-    index = indexFor(h->tablelength,hashvalue);
-    e = h->table[index];
-    while (NULL != e)
-    {
-        /* Check hash value to short circuit heavier comparison */
-        if ((hashvalue == e->h) && (h->eqfn(k, e->k))) return e->v;
-        e = e->next;
-    }
-    return NULL;
+
+    e = hashtable_search_entry(h, k);
+
+    return e ? e->v : NULL;
 }
 
 void
diff --git a/tools/xenstore/hashtable.h b/tools/xenstore/hashtable.h
index 792f6cda7b..e208d439a2 100644
--- a/tools/xenstore/hashtable.h
+++ b/tools/xenstore/hashtable.h
@@ -40,12 +40,6 @@  create_hashtable(const void *ctx, const char *name,
  *
  * This function will cause the table to expand if the insertion would take
  * the ratio of entries to table size over the maximum load factor.
- *
- * This function does not check for repeated insertions with a duplicate key.
- * The value returned when using a duplicate key is undefined -- when
- * the hashtable changes size, the order of retrieval of duplicate key
- * entries is reversed.
- * If in doubt, remove before insert.
  */
 
 int