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 <jgr...@suse.com> --- 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. I can add a sentence to the commit message. Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature
