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.
>
> Signed-off-by: Juergen Gross <jgross@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.
> free(e);
> - return v;
> + return 1;
> }
> pE = &(e->next);
> e = e->next;
> }
> - return NULL;
> + return 0;
> }
>
> /*****************************************************************************/
> diff --git a/tools/xenstore/hashtable.h b/tools/xenstore/hashtable.h
> index 6d65431f96..d6feb1b038 100644
> --- a/tools/xenstore/hashtable.h
> +++ b/tools/xenstore/hashtable.h
> @@ -68,10 +68,10 @@ hashtable_search(struct hashtable *h, void *k);
> * @name hashtable_remove
> * @param h the hashtable to remove the item from
> * @param k the key to search for - does not claim ownership
> - * @return the value associated with the key, or NULL if none found
> + * @return 0 if element not found
> */
>
> -void * /* returns value */
> +int
> hashtable_remove(struct hashtable *h, void *k);
>
> /*****************************************************************************
Cheers,
--
Julien Grall