libctf, hash: improve insertion of existing keys into dynhashes

Right now, if you insert a key/value pair into a dynhash, the old slot's
key is freed and the new one always assigned.  This seemed sane to me
when I wrote it, but I got it wrong time and time again.  It's much
less confusing to free the key passed in: if a key-freeing function
was passed, you are asserting that the dynhash owns the key in any
case, so if you pass in a key it is always buggy to assume it sticks
around.  Freeing the old key means that you can't even safely look up a
key from out of a dynhash and hold on to it, because some other matching
key might force it to be freed at any time.

In the new model, you can always get a key out of a dynhash with
ctf_dynhash_lookup_kv and hang on to it until the kv-pair is actually
deleted from the dynhash.  In the old model the pointer to the key might
be freed at any time if a matching key was inserted.

libctf/
	* ctf-hash.c (ctf_hashtab_insert): Free the key passed in if
	there is a key-freeing function and the key already exists.

diff --git a/libctf/ChangeLog b/libctf/ChangeLog
index a7a31e70fc1b2c58bf697a9e0e06842fe3c1049c..9e8129a610ee71e6bafba2ccc2ad64340125be18 100644 (file)
--- a/libctf/ChangeLog
+++ b/libctf/ChangeLog
@@ -1,3 +1,8 @@
+2020-07-22  Nick Alcock  <nick.alcock@oracle.com>
+
+       * ctf-hash.c (ctf_hashtab_insert): Free the key passed in if
+       there is a key-freeing function and the key already exists.
+
 2020-07-22  Nick Alcock  <nick.alcock@oracle.com>
 
        * ctf-inlines.h: New file.

diff --git a/libctf/ctf-hash.c b/libctf/ctf-hash.c
index 4696fcb2d435f6f3633fc001aa5b67af44d8e6c1..1c37d7515b43839b9d15238da8427f807d48e49f 100644 (file)
--- a/libctf/ctf-hash.c
+++ b/libctf/ctf-hash.c
@@ -171,15 +171,15 @@ ctf_hashtab_insert (struct htab *htab, void *key, void *value,
       *slot = malloc (sizeof (ctf_helem_t));
       if (!*slot)
        return NULL;
+      (*slot)->key = key;
     }
   else
     {
       if (key_free)
-         key_free ((*slot)->key);
+         key_free (key);
       if (value_free)
          value_free ((*slot)->value);
     }
-  (*slot)->key = key;
   (*slot)->value = value;
   return *slot;
 }