fix(hash_map): fixed leak on value update

src/utils/hash_map/hash_map.c

@@ -55,7 +55,7 @@ struct hash_map *hash_map_init(size_t size)
 bool hash_map_insert(struct hash_map *hash_map, const char *key, void *value,
                      bool *updated)
 {
-    if (hash_map == NULL || hash_map->size == 0 || key == NULL)
+    if (hash_map == NULL || hash_map->size == 0 || key == NULL || value == NULL)
         return false;
 
     size_t h = hash(key);
@@ -71,6 +71,7 @@ bool hash_map_insert(struct hash_map *hash_map, const char *key, void *value,
             if (strcmp(iter->key, key) == 0)
             {
                 // update
+                free(iter->value);
                 iter->value = value;
                 if (updated)
                     *updated = true;
@@ -96,16 +97,16 @@ bool hash_map_insert(struct hash_map *hash_map, const char *key, void *value,
     return true;
 }
 
-void hash_map_free(struct hash_map *hash_map)
+void hash_map_free(struct hash_map **hash_map)
 {
     struct pair_list *l;
     struct pair_list *prev;
 
-    if (hash_map)
+    if (hash_map != NULL && *hash_map != NULL)
     {
-        for (size_t i = 0; i < hash_map->size; i++)
+        for (size_t i = 0; i < (*hash_map)->size; i++)
         {
-            l = hash_map->data[i];
+            l = (*hash_map)->data[i];
             while (l != NULL)
             {
                 prev = l;
@@ -113,8 +114,8 @@ void hash_map_free(struct hash_map *hash_map)
                 destroy_pair_list(&prev);
             }
         }
-        free(hash_map->data);
+        free((*hash_map)->data);
-        free(hash_map);
+        free(*hash_map);
     }
 }
 

src/utils/hash_map/hash_map.h

@@ -19,10 +19,23 @@ struct hash_map
 
 struct hash_map *hash_map_init(size_t size);
 
+/**
+ * @brief Inserts a key-value pair into the hash map. Key and value are expected
+ * to be on the heap and will be managed by the hash map. It means they are
+ * consumed by this function. If the key already exists, its value is updated
+ * and the key is not consumed so it must be freed by the caller.
+ *
+ * @param hash_map The hash map.
+ * @param key The key to insert.
+ * @param value The value to insert.
+ * @param updated If not NULL, set to true if the key was already present and
+ * updated, false if the key was newly inserted.
+ * @return true on success, false on failure.
+ */
 bool hash_map_insert(struct hash_map *hash_map, const char *key, void *value,
                      bool *updated);
 
-void hash_map_free(struct hash_map *hash_map);
+void hash_map_free(struct hash_map **hash_map);
 
 void hash_map_foreach(struct hash_map *hash_map,
                       void (*fn)(const char *, const void *));

src/utils/vars/vars.c

@@ -27,14 +27,21 @@ char *get_var_or_env(const struct hash_map *vars, const char *key)
     return value;
 }
 
-bool set_var(struct hash_map *vars, const char *key, const char *value)
+bool set_var(struct hash_map *vars, const char *key, const char *value,
+             bool *updated)
 {
-    if (key == NULL || value == NULL)
+    return hash_map_insert(vars, key, (void *)value, updated);
-        return false;
-    return hash_map_insert(vars, key, (void *)value, NULL);
 }
 
 bool set_var_copy(struct hash_map *vars, const char *key, const char *value)
 {
-    return set_var(vars, strdup(key), strdup(value));
+    char *key_copy = strdup(key);
+    char *value_copy = strdup(value);
+    bool updated;
+    bool res = set_var(vars, key_copy, value_copy, &updated);
+    if (updated || !res)
+        free(key_copy);
+    if (!res)
+        free(value_copy);
+    return res;
 }

tests/unit/utils/hash_map.c

@@ -0,0 +1,217 @@
+#define _POSIX_C_SOURCE 200809L
+#include "../../../src/utils/hash_map/hash_map.h"
+
+#include <criterion/criterion.h>
+#include <criterion/new/assert.h>
+#include <criterion/redirect.h>
+#include <stdio.h>
+
+TestSuite(utils_hash_map);
+
+Test(utils_hash_map, init_free)
+{
+    struct hash_map *map = hash_map_init(10);
+    cr_expect_not_null(map);
+    cr_expect_eq(map->size, 10);
+    hash_map_free(&map);
+}
+
+Test(utils_hash_map, insert_basic)
+{
+    struct hash_map *map = hash_map_init(10);
+    cr_expect_not_null(map);
+    cr_expect_eq(map->size, 10);
+
+    bool updated = false;
+    bool res = hash_map_insert(map, strdup("key1"), strdup("value1"), &updated);
+    cr_expect_eq(res, true);
+    cr_expect_eq(updated, false);
+
+    hash_map_free(&map);
+}
+
+Test(utils_hash_map, insert_multiple)
+{
+    struct hash_map *map = hash_map_init(10);
+    cr_expect_not_null(map);
+    cr_expect_eq(map->size, 10);
+
+    bool updated = false;
+    bool res = hash_map_insert(map, strdup("key1"), strdup("value1"), &updated);
+    cr_expect_eq(res, true);
+    cr_expect_eq(updated, false);
+    res = hash_map_insert(map, strdup("key2"), strdup("value2"), &updated);
+    cr_expect_eq(res, true);
+    cr_expect_eq(updated, false);
+
+    hash_map_free(&map);
+}
+
+Test(utils_hash_map, insert_update)
+{
+    struct hash_map *map = hash_map_init(10);
+    cr_expect_not_null(map);
+    cr_expect_eq(map->size, 10);
+
+    bool updated = false;
+    bool res = hash_map_insert(map, strdup("key1"), strdup("value1"), &updated);
+    cr_expect_eq(res, true);
+    cr_expect_eq(updated, false);
+    res = hash_map_insert(map, "key1", strdup("value2"), &updated);
+    cr_expect_eq(res, true);
+    cr_expect_eq(updated, true);
+
+    hash_map_free(&map);
+}
+
+Test(utils_hash_map, insert_update_multiple)
+{
+    struct hash_map *map = hash_map_init(10);
+    cr_expect_not_null(map);
+    cr_expect_eq(map->size, 10);
+
+    bool updated = false;
+    bool res = hash_map_insert(map, strdup("key1"), strdup("value1"), &updated);
+    cr_expect_eq(res, true);
+    cr_expect_eq(updated, false);
+    res = hash_map_insert(map, strdup("key2"), strdup("value2"), &updated);
+    cr_expect_eq(res, true);
+    cr_expect_eq(updated, false);
+    res = hash_map_insert(map, "key1", strdup("value2"), &updated);
+    cr_expect_eq(res, true);
+    cr_expect_eq(updated, true);
+    res = hash_map_insert(map, "key1", strdup("value3"), &updated);
+    cr_expect_eq(res, true);
+    cr_expect_eq(updated, true);
+
+    hash_map_free(&map);
+}
+
+Test(utils_hash_map, get_basic)
+{
+    struct hash_map *map = hash_map_init(10);
+    cr_expect_not_null(map);
+    cr_expect_eq(map->size, 10);
+
+    bool updated = false;
+    bool res = hash_map_insert(map, strdup("key1"), strdup("value1"), &updated);
+    cr_expect_eq(res, true);
+    cr_expect_eq(updated, false);
+
+    char *value = (char *)hash_map_get(map, "key1");
+    cr_expect_str_eq(value, "value1");
+
+    hash_map_free(&map);
+}
+
+Test(utils_hash_map, get_after_update)
+{
+    struct hash_map *map = hash_map_init(10);
+    cr_expect_not_null(map);
+    cr_expect_eq(map->size, 10);
+
+    bool updated = false;
+    bool res = hash_map_insert(map, strdup("key1"), strdup("value1"), &updated);
+    cr_expect_eq(res, true);
+    cr_expect_eq(updated, false);
+
+    res = hash_map_insert(map, "key1", strdup("value2"), &updated);
+    cr_expect_eq(res, true);
+    cr_expect_eq(updated, true);
+
+    char *value = (char *)hash_map_get(map, "key1");
+    cr_expect_str_eq(value, "value2");
+
+    hash_map_free(&map);
+}
+
+Test(utils_hash_map, get_unknown_key)
+{
+    struct hash_map *map = hash_map_init(10);
+    cr_expect_not_null(map);
+    cr_expect_eq(map->size, 10);
+
+    char *value = (char *)hash_map_get(map, "unknown_key");
+    cr_expect_null(value);
+
+    hash_map_free(&map);
+}
+
+Test(utils_hash_map, delete_key)
+{
+    struct hash_map *map = hash_map_init(10);
+    cr_expect_not_null(map);
+    cr_expect_eq(map->size, 10);
+
+    bool updated = false;
+    bool res = hash_map_insert(map, strdup("key1"), strdup("value1"), &updated);
+    cr_expect_eq(res, true);
+    cr_expect_eq(updated, false);
+
+    res = hash_map_remove(map, "key1");
+    cr_expect_eq(res, true);
+
+    char *value = (char *)hash_map_get(map, "key1");
+    cr_expect_null(value);
+
+    hash_map_free(&map);
+}
+
+Test(utils_hash_map, delete_unknown_key)
+{
+    struct hash_map *map = hash_map_init(10);
+    cr_expect_not_null(map);
+    cr_expect_eq(map->size, 10);
+
+    bool res = hash_map_remove(map, "unknown_key");
+    cr_expect_eq(res, false);
+
+    hash_map_free(&map);
+}
+
+Test(utils_hash_map, free_nonnull_map)
+{
+    struct hash_map *map = hash_map_init(10);
+    cr_expect_not_null(map);
+    cr_expect_eq(map->size, 10);
+
+    bool res = hash_map_insert(map, strdup("key1"), strdup("value1"), NULL);
+    cr_expect_eq(res, true);
+    res = hash_map_insert(map, strdup("key2"), strdup("value2"), NULL);
+    cr_expect_eq(res, true);
+
+    hash_map_free(&map);
+}
+
+Test(utils_hash_map, free_null_map)
+{
+    hash_map_free(NULL);
+}
+
+static size_t count = 0;
+void foreach_fn(const char *key, const void *value)
+{
+    printf("Key: %s, Value: %s\n", key, (const char *)value);
+    count++;
+}
+
+Test(utils_hash_map, foreach, .init = cr_redirect_stdout)
+{
+    struct hash_map *map = hash_map_init(10);
+    cr_expect_not_null(map);
+    cr_expect_eq(map->size, 10);
+
+    bool res = hash_map_insert(map, strdup("key1"), strdup("value1"), NULL);
+    cr_expect_eq(res, true);
+    res = hash_map_insert(map, strdup("key2"), strdup("value2"), NULL);
+    cr_expect_eq(res, true);
+
+    count = 0;
+    hash_map_foreach(map, foreach_fn);
+    fflush(stdout);
+    cr_expect_eq(count, 2);
+    cr_expect_stdout_eq_str(
+        "Key: key2, Value: value2\nKey: key1, Value: value1\n");
+
+    hash_map_free(&map);
+}