Hi Juergen,
On 30/05/2023 10:13, Juergen Gross wrote:
For an effective way to replace a hashtable entry add a new function
hashtable_replace().
While at it let hashtable_add() fail if an entry with the specified
key does already exist.
Please explain why you want to do it. I also think this change should be
in its own patch.
This is in preparation to replace TDB with a more simple data storage.
Signed-off-by: Juergen Gross <jgr...@suse.com>
---
tools/xenstore/hashtable.c | 52 ++++++++++++++++++++++++++++++--------
tools/xenstore/hashtable.h | 16 ++++++++++++
2 files changed, 58 insertions(+), 10 deletions(-)
diff --git a/tools/xenstore/hashtable.c b/tools/xenstore/hashtable.c
index 9daddd9782..f358bec5ae 100644
--- a/tools/xenstore/hashtable.c
+++ b/tools/xenstore/hashtable.c
@@ -141,11 +141,32 @@ 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);
Missing space after ','.
+ e = h->table[index];
+ while (NULL != e)
+ {
While you are moving the code. I think you can reduce the code size with:
for ( e = h->table[index]; e != NULL; e = e->next )
if (....)
+ /* Check hash value to short circuit heavier comparison */
+ if ((hashvalue == e->h) && (h->eqfn(k, e->k))) return e;
+ e = e->next;
+ }
+ 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 +197,28 @@ 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)
+
+ e = hashtable_search_entry(h, k);
+ return e ? e->v : NULL;
+}
+
+int hashtable_replace(struct hashtable *h, const void *k, void *v)
+{
+ struct entry *e;
+
+ e = hashtable_search_entry(h, k);
+ if (!e)
+ return ENOENT;
+
+ if (h->flags & HASHTABLE_FREE_VALUE)
{
- /* Check hash value to short circuit heavier comparison */
- if ((hashvalue == e->h) && (h->eqfn(k, e->k))) return e->v;
- e = e->next;
+ talloc_free(e->v);
+ talloc_steal(e, v);
}
- return NULL;
+
+ e->v = v;
+
Coding style: Above you don't add a newline before return but here you
do. I don't particularly care which one you want to use in xenstored.
But can you at least be consistent?
+ return 0;
}
void
diff --git a/tools/xenstore/hashtable.h b/tools/xenstore/hashtable.h
index 792f6cda7b..214aea1b3d 100644
--- a/tools/xenstore/hashtable.h
+++ b/tools/xenstore/hashtable.h
@@ -51,6 +51,22 @@ create_hashtable(const void *ctx, const char *name,
int
hashtable_add(struct hashtable *h, const void *k, void *v);
+/*****************************************************************************
+ * hashtable_replace
+
+ * @name hashtable_nsert
+ * @param h the hashtable to insert into
+ * @param k the key - hashtable claims ownership and will free on removal
+ * @param v the value - does not claim ownership
+ * @return zero for successful insertion
+ *
+ * This function does check for an entry being present before replacing it
+ * with a new value.
+ */
+
+int
+hashtable_replace(struct hashtable *h, const void *k, void *v);
+
/*****************************************************************************
* hashtable_search
--
Julien Grall
