[21/59] libctf, hash: improve insertion of existing keys into dynhashes

Message ID 20200630233146.338613-22-nick.alcock@oracle.com
State New
Headers show
Series
  • Deduplicating CTF linker
Related show

Commit Message

H.J. Lu via Binutils June 30, 2020, 11:31 p.m.
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.
---
 libctf/ctf-hash.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.27.0.247.g3dff7de930

Patch

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;
 }