From: Nick Alcock <nick.alcock@oracle.com>
Date: Tue, 2 Jun 2020 20:48:12 +0000 (+0100)
Subject: libctf, hash: improve insertion of existing keys into dynhashes
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=5ceee3dba3422bc8de49768c0c2d8f2608672fe7;p=binutils-gdb.git

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 a7a31e70fc1..9e8129a610e 100644
--- 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 4696fcb2d43..1c37d7515b4 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;
 }