On 18/01/2023 06:17, Juergen Gross wrote:
On 17.01.23 23:03, Julien Grall wrote:
Hi Juergen,
On 17/01/2023 09:11, Juergen Gross wrote:
Letting hashtable_remove() return the value of the removed element is
not used anywhere in Xenstore, and it conflicts with a hashtable
created specifying the HASHTABLE_FREE_VALUE flag.
So just drop returning the value.
Any reason this can't be void? If there are, then I would consider to
return a bool as the return can only be 2 values.
I think you are right. Switching to void should be fine.
Signed-off-by: Juergen Gross <[email protected]>
---
V3:
- new patch
---
tools/xenstore/hashtable.c | 10 +++++-----
tools/xenstore/hashtable.h | 4 ++--
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/tools/xenstore/hashtable.c b/tools/xenstore/hashtable.c
index 299549c51e..6738719e47 100644
--- a/tools/xenstore/hashtable.c
+++ b/tools/xenstore/hashtable.c
@@ -214,7 +214,7 @@ hashtable_search(struct hashtable *h, void *k)
}
/*****************************************************************************/
-void * /* returns value associated with key */
+int
hashtable_remove(struct hashtable *h, void *k)
{
/* TODO: consider compacting the table when the load factor
drops enough,
@@ -222,7 +222,6 @@ hashtable_remove(struct hashtable *h, void *k)
struct entry *e;
struct entry **pE;
- void *v;
unsigned int hashvalue, index;
hashvalue = hash(h,k);
@@ -236,16 +235,17 @@ hashtable_remove(struct hashtable *h, void *k)
{
*pE = e->next;
h->entrycount--;
- v = e->v;
if (h->flags & HASHTABLE_FREE_KEY)
free(e->k);
+ if (h->flags & HASHTABLE_FREE_VALUE)
+ free(e->v);
I don't quite understand how this change is related to this patch.
With not returning the value pointer any longer there would be no way
for the caller to free it, so it must be freed by hashtable_remove()
if the related flag was set.
That makes sense now. Thanks for the explanation.
I can add a sentence to the commit message.
Yes please.
The rest of this patch looks good to me.
Cheers,
--
Julien Grall
