Add a aws_hash_table_put helper (awslabs#134)

* Add a aws_hash_table_put helper

This makes the common use case of inserting a key/value pair with overwrite
easier (removes the need to declare an element local).

* Handle null destructors in hash_table_put

* Fix clang-format error

Author: bdonlan

include/aws/common/hash_table.h

@@ -217,6 +217,20 @@ int aws_hash_table_create(
     struct aws_hash_element **p_elem,
     int *was_created);
 
+/**
+ * Inserts a new element at key, with the given value. If another element
+ * exists at that key, the old element will be overwritten; both old key and
+ * value objects will be destroyed.
+ *
+ * If was_created is non-NULL, *was_created is set to 0 if an existing
+ * element was found, or 1 is a new element was created.
+ *
+ * Returns AWS_OP_SUCCESS if an item was found or created.
+ * Raises AWS_ERROR_OOM if hash table expansion was required and memory
+ */
+AWS_COMMON_API
+int aws_hash_table_put(struct aws_hash_table *map, const void *key, void *value, int *was_created);
+
 /**
  * Removes element at key. Always returns AWS_OP_SUCCESS.
  *

source/hash_table.c

@@ -505,6 +505,41 @@ int aws_hash_table_create(
     return AWS_OP_SUCCESS;
 }
 
+AWS_COMMON_API
+int aws_hash_table_put(struct aws_hash_table *map, const void *key, void *value, int *was_created) {
+    struct aws_hash_element *p_elem;
+    int was_created_fallback;
+
+    if (!was_created) {
+        was_created = &was_created_fallback;
+    }
+
+    if (aws_hash_table_create(map, key, &p_elem, was_created)) {
+        return AWS_OP_ERR;
+    }
+
+    /*
+     * aws_hash_table_create might resize the table, which results in map->p_impl changing.
+     * It is therefore important to wait to read p_impl until after we return.
+     */
+    struct hash_table_state *state = map->p_impl;
+
+    if (!*was_created) {
+        if (p_elem->key != key && state->destroy_key_fn) {
+            state->destroy_key_fn((void *)p_elem->key);
+        }
+
+        if (state->destroy_value_fn) {
+            state->destroy_value_fn((void *)p_elem->value);
+        }
+    }
+
+    p_elem->key = key;
+    p_elem->value = value;
+
+    return AWS_OP_SUCCESS;
+}
+
 /* Clears an entry. Does _not_ invoke destructor callbacks.
  * Returns the last slot touched (note that if we wrap, we'll report an index
  * lower than the original entry's index)

tests/CMakeLists.txt

@@ -119,8 +119,10 @@ add_test_case(scheduler_pops_task_late_test)
 add_test_case(scheduler_task_timestamp_test)
 add_test_case(scheduler_reentrant_safe)
 
-add_test_case(test_hash_table_put_get)
-add_test_case(test_hash_table_string_put_get)
+add_test_case(test_hash_table_create_find)
+add_test_case(test_hash_table_string_create_find)
+add_test_case(test_hash_table_put)
+add_test_case(test_hash_table_put_null_dtor)
 add_test_case(test_hash_table_string_clean_up)
 add_test_case(test_hash_table_hash_collision)
 add_test_case(test_hash_table_hash_overwrite)

tests/hash_table_test.c

@@ -29,8 +29,29 @@ static const char *TEST_VAL_STR_2 = "value 2";
 #define ASSERT_HASH_TABLE_ENTRY_COUNT(map, count)                                                                      \
     ASSERT_UINT_EQUALS(count, aws_hash_table_get_entry_count(map), "Hash map should have %d entries", count)
 
-AWS_TEST_CASE(test_hash_table_put_get, s_test_hash_table_put_get_fn)
-static int s_test_hash_table_put_get_fn(struct aws_allocator *allocator, void *ctx) {
+#define ASSERT_NO_KEY(hash_table, key)                                                                                 \
+    do {                                                                                                               \
+        AWS_STATIC_STRING_FROM_LITERAL(assert_key, key);                                                               \
+        struct aws_hash_element *pElem_assert;                                                                         \
+        ASSERT_SUCCESS(aws_hash_table_find((hash_table), (void *)assert_key, &pElem_assert));                          \
+        ASSERT_NULL(pElem_assert, "Expected key to not be present: " key);                                             \
+    } while (0)
+
+#define ASSERT_KEY_VALUE(hash_table, key, expected)                                                                    \
+    do {                                                                                                               \
+        AWS_STATIC_STRING_FROM_LITERAL(assert_key, key);                                                               \
+        AWS_STATIC_STRING_FROM_LITERAL(assert_value, expected);                                                        \
+        struct aws_hash_element *pElem_assert;                                                                         \
+        ASSERT_SUCCESS(aws_hash_table_find((hash_table), (void *)assert_key, &pElem_assert));                          \
+        ASSERT_NOT_NULL(pElem_assert, "Expected key to be present: " key);                                             \
+        ASSERT_TRUE(                                                                                                   \
+            aws_string_eq(assert_value, (const struct aws_string *)pElem_assert->value),                               \
+            "Expected key \"" key "\" to have value \"" expected "\"; actually had value \"%s\"",                      \
+            aws_string_bytes((const struct aws_string *)pElem_assert->value));                                         \
+    } while (0)
+
+AWS_TEST_CASE(test_hash_table_create_find, s_test_hash_table_create_find_fn)
+static int s_test_hash_table_create_find_fn(struct aws_allocator *allocator, void *ctx) {
 
     (void)ctx;
 
@@ -81,8 +102,8 @@ static int s_test_hash_table_put_get_fn(struct aws_allocator *allocator, void *c
     return 0;
 }
 
-AWS_TEST_CASE(test_hash_table_string_put_get, s_test_hash_table_string_put_get_fn)
-static int s_test_hash_table_string_put_get_fn(struct aws_allocator *allocator, void *ctx) {
+AWS_TEST_CASE(test_hash_table_string_create_find, s_test_hash_table_string_create_find_fn)
+static int s_test_hash_table_string_create_find_fn(struct aws_allocator *allocator, void *ctx) {
     (void)ctx;
 
     struct aws_hash_table hash_table;
@@ -189,6 +210,86 @@ static int s_test_hash_table_string_put_get_fn(struct aws_allocator *allocator,
     return 0;
 }
 
+static const void *last_key, *last_value;
+
+static void destroy_key_record(void *key) {
+    last_key = key;
+}
+
+static void destroy_value_record(void *value) {
+    last_value = value;
+}
+
+AWS_TEST_CASE(test_hash_table_put, s_test_hash_table_put_fn)
+static int s_test_hash_table_put_fn(struct aws_allocator *allocator, void *ctx) {
+    (void)ctx;
+
+    struct aws_hash_table hash_table;
+    struct aws_hash_element *pElem;
+    int was_created;
+
+    int ret = aws_hash_table_init(
+        &hash_table, allocator, 10, aws_hash_string, aws_string_eq, destroy_key_record, destroy_value_record);
+    ASSERT_SUCCESS(ret, "Hash Map init should have succeeded.");
+
+    AWS_STATIC_STRING_FROM_LITERAL(sentinel, "");
+    AWS_STATIC_STRING_FROM_LITERAL(key_a_1, "a");
+    AWS_STATIC_STRING_FROM_LITERAL(value_b_1, "b");
+
+    ASSERT_NO_KEY(&hash_table, "a");
+    last_key = last_value = sentinel;
+    aws_hash_table_put(&hash_table, key_a_1, (void *)value_b_1, &was_created);
+    ASSERT_INT_EQUALS(was_created, 1);
+    ASSERT_KEY_VALUE(&hash_table, "a", "b");
+    /* dtors were not called, even with nulls */
+    ASSERT_PTR_EQUALS(last_key, sentinel);
+    ASSERT_PTR_EQUALS(last_value, sentinel);
+
+    AWS_STATIC_STRING_FROM_LITERAL(key_a_2, "a");
+    AWS_STATIC_STRING_FROM_LITERAL(value_c_1, "c");
+
+    last_key = last_value = NULL;
+    aws_hash_table_put(&hash_table, key_a_2, (void *)value_c_1, &was_created);
+    ASSERT_INT_EQUALS(was_created, 0);
+    ASSERT_KEY_VALUE(&hash_table, "a", "c");
+
+    ASSERT_SUCCESS(aws_hash_table_find(&hash_table, (void *)key_a_1, &pElem));
+    ASSERT_PTR_EQUALS(key_a_2, pElem->key);
+    /* verify dtor was called on the old key ptr */
+    ASSERT_PTR_EQUALS(last_key, key_a_1);
+    ASSERT_PTR_EQUALS(last_value, value_b_1);
+
+    last_key = last_value = NULL;
+    aws_hash_table_put(&hash_table, key_a_2, (void *)value_b_1, NULL);
+    ASSERT_KEY_VALUE(&hash_table, "a", "b");
+
+    /* Since the key ptr did not change, it was not destroyed */
+    ASSERT_PTR_EQUALS(last_key, NULL);
+    /* The value was destroyed however */
+    ASSERT_PTR_EQUALS(last_value, value_c_1);
+
+    aws_hash_table_clean_up(&hash_table);
+
+    return 0;
+}
+AWS_TEST_CASE(test_hash_table_put_null_dtor, s_test_hash_table_put_null_dtor_fn)
+static int s_test_hash_table_put_null_dtor_fn(struct aws_allocator *allocator, void *ctx) {
+    (void)ctx;
+
+    struct aws_hash_table hash_table;
+
+    int ret = aws_hash_table_init(&hash_table, allocator, 10, aws_hash_string, aws_string_eq, NULL, NULL);
+    ASSERT_SUCCESS(ret, "Hash Map init should have succeeded.");
+
+    AWS_STATIC_STRING_FROM_LITERAL(foo, "foo");
+    ASSERT_SUCCESS(aws_hash_table_put(&hash_table, foo, (void *)foo, NULL));
+    ASSERT_SUCCESS(aws_hash_table_put(&hash_table, foo, (void *)foo, NULL));
+
+    aws_hash_table_clean_up(&hash_table);
+
+    return 0;
+}
+
 AWS_TEST_CASE(test_hash_table_string_clean_up, s_test_hash_table_string_clean_up_fn)
 static int s_test_hash_table_string_clean_up_fn(struct aws_allocator *allocator, void *ctx) {