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.

2 files changed, 7 insertions, 2 deletions

diff --git a/libctf/ChangeLog b/libctf/ChangeLog
index a7a31e7..9e8129a 100644
--- a/libctf/ChangeLog
+++ b/libctf/ChangeLog
@@ -1,5 +1,10 @@
 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.
 	* ctf-impl.h: Include it.
 	(ctf_hash_iter_find_f): New typedef.
diff --git a/libctf/ctf-hash.c b/libctf/ctf-hash.c
index 4696fcb..1c37d75 100644
--- 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;
 }