From a845c7ec3da4d48e90a08732f0fbd76c842693a9 Mon Sep 17 00:00:00 2001
From: Dmitry Podgorny <pasis.ua@gmail.com>
Date: Sat, 24 Jun 2017 14:14:08 +0300
Subject: [PATCH] hash: avoid allocation when replace value in hash_add()

Memory allocation may fail. We don't need to allocate new entry when we
already have one.
---
 src/handler.c |  8 +++----
 src/hash.c    | 66 ++++++++++++++++++++++++++++-----------------------
 2 files changed, 40 insertions(+), 34 deletions(-)

diff --git a/src/handler.c b/src/handler.c
index dc53534..00bc8c0 100644
--- a/src/handler.c
+++ b/src/handler.c
@@ -87,8 +87,7 @@ void handler_fire_stanza(xmpp_conn_t * const conn,
                 head_old = head;
                 _handler_item_remove(&head, item);
                 if (head != head_old) {
-                    /* TODO implement hash_replace() */
-                    hash_drop(conn->id_handlers, id);
+                    /* replace old value */
                     hash_add(conn->id_handlers, id, head);
                 }
                 xmpp_free(conn->ctx, item->id);
@@ -677,10 +676,11 @@ void handler_system_delete_all(xmpp_conn_t *conn)
            freed memory. */
         key = hash_iter_next(iter);
         if (head != head_old) {
-            /* TODO implement hash_replace() to reduce number of allocations */
-            hash_drop(conn->id_handlers, key2);
+            /* hash_add() replaces value if the key exists */
             if (head != NULL)
                 hash_add(conn->id_handlers, key2, head);
+            else
+                hash_drop(conn->id_handlers, key2);
             xmpp_free(conn->ctx, key2);
         }
     }
diff --git a/src/hash.c b/src/hash.c
index 66940e5..d7b8959 100644
--- a/src/hash.c
+++ b/src/hash.c
@@ -119,6 +119,24 @@ static int _hash_key(hash_t *table, const char *key)
    return hash % (unsigned)table->length;
 }
 
+hashentry_t *_hash_entry_find(hash_t *table, const char *key)
+{
+   hashentry_t *entry;
+   int table_index = _hash_key(table, key);
+
+   /* look up the hash entry */
+   entry = table->entries[table_index];
+   while (entry != NULL) {
+	/* traverse the linked list looking for the key */
+	if (!strcmp(key, entry->key)) {
+	  /* match */
+          break;
+	}
+	entry = entry->next;
+   }
+   return entry;
+}
+
 /** add a key, value pair to a hash table.
  *  each key can appear only once; the value of any
  *  identical key will be replaced
@@ -129,23 +147,25 @@ int hash_add(hash_t *table, const char * const key, void *data)
    hashentry_t *entry = NULL;
    int table_index = _hash_key(table, key);
 
-   /* drop existing entry, if any */
-   hash_drop(table, key);
+   /* find and replace existing entry, if any */
+   entry = _hash_entry_find(table, key);
 
-   /* allocate and fill a new entry */
-   entry = xmpp_alloc(ctx, sizeof(hashentry_t));
-   if (!entry) return -1;
-   entry->key = xmpp_strdup(ctx, key);
-   if (!entry->key) {
-       xmpp_free(ctx, entry);
-       return -1;
+   if (entry == NULL) {
+      /* allocate and fill a new entry */
+      entry = xmpp_alloc(ctx, sizeof(hashentry_t));
+      if (!entry) return -1;
+      entry->key = xmpp_strdup(ctx, key);
+      if (!entry->key) {
+         xmpp_free(ctx, entry);
+         return -1;
+      }
+      /* insert ourselves in the linked list */
+      entry->next = table->entries[table_index];
+      table->entries[table_index] = entry;
+      table->num_keys++;
    }
+
    entry->value = data;
-   /* insert ourselves in the linked list */
-   /* TODO: this leaks duplicate keys */
-   entry->next = table->entries[table_index];
-   table->entries[table_index] = entry;
-   table->num_keys++;
 
    return 0;
 }
@@ -154,22 +174,9 @@ int hash_add(hash_t *table, const char * const key, void *data)
 void *hash_get(hash_t *table, const char *key)
 {
    hashentry_t *entry;
-   int table_index = _hash_key(table, key);
-   void *result = NULL;
 
-   /* look up the hash entry */
-   entry = table->entries[table_index];
-   while (entry != NULL) {
-	/* traverse the linked list looking for the key */
-	if (!strcmp(key, entry->key)) {
-	  /* match */
-	  result = entry->value;
-	  return result;
-	}
-	entry = entry->next;
-   }
-   /* no match */
-   return result;
+   entry = _hash_entry_find(table, key);
+   return entry == NULL ? NULL : entry->value;
 }
 
 /** delete a key from a hash table */
@@ -272,4 +279,3 @@ const char * hash_iter_next(hash_iterator_t *iter)
     iter->entry = entry;
     return entry->key;
 }
-