Better handling of dict hash tables

Set dup and free operations to automatically manage reference counting
of dictionary keys and values.

Replace Jim_ReplaceHashEntry() with JimDictReplaceHashEntry()

Signed-off-by: Steve Bennett <steveb@workware.net.au>

1 files changed, 21 insertions, 43 deletions

diff --git a/jim.c b/jim.c
index d64e5e5..a1945d2 100644
--- a/jim.c
+++ b/jim.c
@@ -882,28 +882,25 @@ int Jim_AddHashEntry(Jim_HashTable *ht, const void *key, void *val)
     return JIM_OK;
 }
 
-/* Add an element, discarding the old if the key already exists */
-int Jim_ReplaceHashEntry(Jim_HashTable *ht, const void *key, void *val)
+/* Special, efficient version of Jim_AddHashEntry() for dictionaries */
+static void JimDictReplaceHashEntry(Jim_Interp *interp, Jim_HashTable *ht, Jim_Obj *keyObjPtr, Jim_Obj *valObjPtr)
 {
-    int existed;
     Jim_HashEntry *entry;
 
-    /* Get the index of the new element, or -1 if
-     * the element already exists. */
-    entry = JimInsertHashEntry(ht, key, 1);
+    Jim_IncrRefCount(keyObjPtr);
+    Jim_IncrRefCount(valObjPtr);
+
+    /* Find the existing element, or a new slot */
+    entry = JimInsertHashEntry(ht, keyObjPtr, 1);
     if (entry->key) {
-        /* It already exists, so replace the value */
-        Jim_FreeEntryVal(ht, entry);
-        existed = 1;
+        /* Already exists, so replace the entry */
+        Jim_DecrRefCount(interp, (Jim_Obj *)entry->key);
+        Jim_DecrRefCount(interp, (Jim_Obj *)entry->u.val);
     }
-    else {
-        /* Doesn't exist, so set the key */
-        Jim_SetHashKey(ht, entry, key);
-        existed = 0;
-    }
-    Jim_SetHashVal(ht, entry, val);
 
-    return existed;
+    /* Set the hash entry fields. */
+    entry->key = keyObjPtr;
+    entry->u.val = valObjPtr;
 }
 
 /* Search and remove an element */
@@ -1037,9 +1034,11 @@ static unsigned int JimHashTableNextPower(unsigned int size)
     }
 }
 
-/* Returns the index of a free slot that can be populated with
+/* Finds a free slot that can be populated with
  * an hash entry for the given 'key'.
- * If the key already exists, -1 is returned. */
+ * Returns NULL if not found and stores the index in *idx.
+ * Returns the hash entry if found and stores the index in *idx.
+ */
 static Jim_HashEntry *JimInsertHashEntry(Jim_HashTable *ht, const void *key, int replace)
 {
     unsigned int h;
@@ -1058,7 +1057,7 @@ static Jim_HashEntry *JimInsertHashEntry(Jim_HashTable *ht, const void *key, int
         he = he->next;
     }
 
-    /* Allocates the memory and stores key */
+    /* Not found, so allocate a new entry, hook it in and set key = NULL */
     he = Jim_Alloc(sizeof(*he));
     he->next = ht->table[h];
     ht->table[h] = he;
@@ -6957,12 +6956,7 @@ void DupDictInternalRep(Jim_Interp *interp, Jim_Obj *srcPtr, Jim_Obj *dupPtr)
     /* Copy every element from the source to the dup hash table */
     JimInitHashTableIterator(ht, &htiter);
     while ((he = Jim_NextHashEntry(&htiter)) != NULL) {
-        const Jim_Obj *keyObjPtr = he->key;
-        Jim_Obj *valObjPtr = he->u.val;
-
-        Jim_IncrRefCount((Jim_Obj *)keyObjPtr); /* ATTENTION: const cast */
-        Jim_IncrRefCount(valObjPtr);
-        Jim_AddHashEntry(dupHt, keyObjPtr, valObjPtr);
+        JimDictReplaceHashEntry(interp, dupHt, (Jim_Obj *)he->key, he->u.val);
     }
 
     dupPtr->internalRep.ptr = dupHt;
@@ -7036,18 +7030,7 @@ static int SetDictFromAny(Jim_Interp *interp, struct Jim_Obj *objPtr)
             Jim_ListIndex(interp, objPtr, i, &keyObjPtr, JIM_NONE);
             Jim_ListIndex(interp, objPtr, i + 1, &valObjPtr, JIM_NONE);
 
-            Jim_IncrRefCount(keyObjPtr);
-            Jim_IncrRefCount(valObjPtr);
-
-            if (Jim_AddHashEntry(ht, keyObjPtr, valObjPtr) != JIM_OK) {
-                Jim_HashEntry *he;
-
-                he = Jim_FindHashEntry(ht, keyObjPtr);
-                Jim_DecrRefCount(interp, keyObjPtr);
-                /* ATTENTION: const cast */
-                Jim_DecrRefCount(interp, (Jim_Obj *)he->u.val);
-                he->u.val = valObjPtr;
-            }
+            JimDictReplaceHashEntry(interp, ht, keyObjPtr, valObjPtr);
         }
 
         Jim_FreeIntRep(interp, objPtr);
@@ -7074,12 +7057,7 @@ static int DictAddElement(Jim_Interp *interp, Jim_Obj *objPtr,
     if (valueObjPtr == NULL) {  /* unset */
         return Jim_DeleteHashEntry(ht, keyObjPtr);
     }
-    Jim_IncrRefCount(keyObjPtr);
-    Jim_IncrRefCount(valueObjPtr);
-    if (Jim_ReplaceHashEntry(ht, keyObjPtr, valueObjPtr)) {
-        /* Value existed, so need to decrement key ref count */
-        Jim_DecrRefCount(interp, keyObjPtr);
-    }
+    JimDictReplaceHashEntry(interp, ht, keyObjPtr, valueObjPtr);
     return JIM_OK;
 }