Browse Source

HASH_DEL should be able to delete a const-qualified node

HASH_DEL doesn't need to modify the target node; in fact it should not,
because that target node is usually going straight back to `free` and
any modifications to it would be wasted effort. So, let's make that an
actual tested guarantee, and let's make it const-correct.

Inspired by the discussion in #253.
Arthur O'Dwyer 2 years ago
parent
commit
ca98384ce7
5 changed files with 60 additions and 2 deletions
  1. 1 1
      src/uthash.h
  2. 1 1
      tests/Makefile
  3. 1 0
      tests/README
  4. 0 0
      tests/test97.ans
  5. 57 0
      tests/test97.c

+ 1 - 1
src/uthash.h

@@ -452,7 +452,7 @@ do {
 
 
 #define HASH_DELETE_HH(hh,head,delptrhh)                                         \
 #define HASH_DELETE_HH(hh,head,delptrhh)                                         \
 do {                                                                             \
 do {                                                                             \
-  struct UT_hash_handle *_hd_hh_del = (delptrhh);                                \
+  const struct UT_hash_handle *_hd_hh_del = (delptrhh);                          \
   if ((_hd_hh_del->prev == NULL) && (_hd_hh_del->next == NULL)) {                \
   if ((_hd_hh_del->prev == NULL) && (_hd_hh_del->next == NULL)) {                \
     HASH_BLOOM_FREE((head)->hh.tbl);                                             \
     HASH_BLOOM_FREE((head)->hh.tbl);                                             \
     uthash_free((head)->hh.tbl->buckets,                                         \
     uthash_free((head)->hh.tbl->buckets,                                         \

+ 1 - 1
tests/Makefile

@@ -12,7 +12,7 @@ PROGS = test1 test2 test3 test4 test5 test6 test7 test8 test9   \
         test66 test67 test68 test69 test70 test71 test72 test73 \
         test66 test67 test68 test69 test70 test71 test72 test73 \
         test74 test75 test76 test77 test78 test79 test80 test81 \
         test74 test75 test76 test77 test78 test79 test80 test81 \
         test82 test83 test84 test85 test86 test87 test88 test89 \
         test82 test83 test84 test85 test86 test87 test88 test89 \
-        test90 test91 test92 test93 test94 test95 test96
+        test90 test91 test92 test93 test94 test95 test96 test97
 CFLAGS += -I$(HASHDIR)
 CFLAGS += -I$(HASHDIR)
 #CFLAGS += -DHASH_BLOOM=16
 #CFLAGS += -DHASH_BLOOM=16
 #CFLAGS += -O2
 #CFLAGS += -O2

+ 1 - 0
tests/README

@@ -98,6 +98,7 @@ test93: alt_fatal
 test94: utlist with fields named other than 'next' and 'prev'
 test94: utlist with fields named other than 'next' and 'prev'
 test95: utstack
 test95: utstack
 test96: HASH_FUNCTION + HASH_KEYCMP
 test96: HASH_FUNCTION + HASH_KEYCMP
+test97: deleting a const-qualified node from a hash
 
 
 Other Make targets
 Other Make targets
 ================================================================================
 ================================================================================

+ 0 - 0
tests/test97.ans


+ 57 - 0
tests/test97.c

@@ -0,0 +1,57 @@
+#include <assert.h>
+#include <stdlib.h>
+#include <string.h>
+#include "uthash.h"
+
+struct item {
+    int payload;
+    UT_hash_handle hh;
+};
+
+void delete_without_modifying(struct item *head, const struct item *p)
+{
+    struct item old;
+    memcpy(&old, p, sizeof(struct item)); // also copy the padding bits
+    assert(memcmp(&old, p, sizeof(struct item)) == 0);
+    assert(p->hh.tbl == head->hh.tbl); // class invariant
+    HASH_DEL(head, p);
+    assert(memcmp(&old, p, sizeof(struct item)) == 0);  // unmodified by HASH_DEL
+}
+
+int main()
+{
+    struct item *items = NULL;
+    struct item *found = NULL;
+    int fortytwo = 42;
+    int i;
+
+    for (i=0; i < 100; i++) {
+        struct item *p = (struct item *)malloc(sizeof *p);
+        p->payload = i;
+        HASH_ADD_INT(items, payload, p);
+    }
+    assert(HASH_COUNT(items) == 100);
+
+    // Delete item "42" from the hash, wherever it is.
+    HASH_FIND_INT(items, &fortytwo, found);
+    assert(found != NULL);
+    assert(found->payload == 42);
+    delete_without_modifying(items, found);
+
+    assert(HASH_COUNT(items) == 99);
+    HASH_FIND_INT(items, &fortytwo, found);
+    assert(found == NULL);
+
+    // Delete the very first item in the hash.
+    assert(items != NULL);
+    i = items->payload;
+    delete_without_modifying(items, items);
+
+    assert(HASH_COUNT(items) == 98);
+    HASH_FIND_INT(items, &i, found);
+    assert(found == NULL);
+
+    // leak the items, we don't care
+
+    return 0;
+}