Re: [PATCH v3 12/17] tools/xenstore: don't let hashtable_remove() return the removed value

[Julien Grall]

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 <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.

              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