Fix yaml memory leak for duplicate dict keys. Refactor yaml test layout.

Signed-off-by: Rule Timothy (VM/EMT3) <[email protected]>

dse/clib/util/yaml.c

@@ -594,7 +594,12 @@ static YamlNode* _create_node(char* name, YamlNode* parent)
     if (parent) {
         if (parent->node_type == YAML_MAPPING_NODE) {
             assert(node->name);
-            if (node->name) hashmap_set(&parent->mapping, node->name, node);
+            if (node->name) {
+                /* Prevent duplicate dict keys, last wins. */
+                void* o = hashmap_remove(&parent->mapping, node->name);
+                if (o) _destroy_node(o);
+                hashmap_set(&parent->mapping, node->name, node);
+            }
         } else if (parent->node_type == YAML_SEQUENCE_NODE) {
             hashlist_append(&parent->sequence, node);
         }

tests/util/CMakeLists.txt

@@ -35,10 +35,10 @@ target_link_libraries(test_util
 #        -Wl,--wrap=strdup
 )
 set(YAML_EXAMPLE_RESOURCE_FILES
-    bool.yaml
-    uint.yaml
-    test.yaml
-    empty_doc.yaml
+    data/bool.yaml
+    data/uint.yaml
+    data/test.yaml
+    data/empty_doc.yaml
 )
 install(TARGETS test_util)
 install(

tests/util/__test__.c

@@ -9,20 +9,19 @@
 uint8_t __log_level__ = LOG_ERROR; /* LOG_ERROR LOG_INFO LOG_DEBUG LOG_TRACE */
 
 
-void test_buffer_append__general(void** state);
-void test_buffer_append__extended(void** state);
-void test_yaml_load_single_doc(void** state);
-void test_yaml_load_file(void** state);
-void test_yaml_find_doc_doclist(void** state);
-void test_yaml_find_node_doclist(void** state);
-void test_yaml_find_node_seq_doclist(void** state);
-void test_yaml_find_node_seq(void** state);
-void test_yaml_get_uint(void** state);
-void test_yaml_get_int(void** state);
-void test_yaml_get_double(void** state);
-void test_yaml_get_bool(void** state);
-void test_yaml_get_string(void** state);
-void test_yaml_get_parser(void** state);
+extern int run_binary_tests(void);
+extern int run_yaml_tests(void);
+
+
+int main()
+{
+    __log_level__ = LOG_QUIET;//LOG_DEBUG;//LOG_QUIET;
+
+    int rc = 0;
+    rc |= run_binary_tests();
+    rc |= run_yaml_tests();
+    return rc;
+}
 
 
 /* This wrap is not compatable with LibYAML for some reason. You get
@@ -36,26 +35,3 @@ char* __wrap_strdup(const char* s)
     void*  dup = malloc(len);
     return (char*)memcpy(dup, s, len);
 }
-
-
-int main()
-{
-    const struct CMUnitTest tests[] = {
-        cmocka_unit_test(test_buffer_append__general),
-        cmocka_unit_test(test_buffer_append__extended),
-        cmocka_unit_test(test_yaml_load_single_doc),
-        cmocka_unit_test(test_yaml_load_file),
-        cmocka_unit_test(test_yaml_find_doc_doclist),
-        cmocka_unit_test(test_yaml_find_node_doclist),
-        cmocka_unit_test(test_yaml_find_node_seq_doclist),
-        cmocka_unit_test(test_yaml_find_node_seq),
-        cmocka_unit_test(test_yaml_get_uint),
-        cmocka_unit_test(test_yaml_get_int),
-        cmocka_unit_test(test_yaml_get_double),
-        cmocka_unit_test(test_yaml_get_bool),
-        cmocka_unit_test(test_yaml_get_string),
-        cmocka_unit_test(test_yaml_get_parser),
-    };
-
-    return cmocka_run_group_tests(tests, NULL, NULL);
-}

tests/util/data/dict_dup.yaml

@@ -0,0 +1,6 @@
+annotations:
+  struct_member_name: temperature
+  struct_member_offset: 2
+  struct_member_primitive_type: int16_t
+  init_value: foo
+  init_value: bar

tests/util/test_binary.c

@@ -78,3 +78,14 @@ void test_buffer_append__extended(void** state)
     /* Cleanup */
     free(buffer);
 }
+
+
+int run_binary_tests(void)
+{
+    const struct CMUnitTest tests[] = {
+        cmocka_unit_test(test_buffer_append__general),
+        cmocka_unit_test(test_buffer_append__extended),
+    };
+
+    return cmocka_run_group_tests_name("BINARY", tests, NULL, NULL);
+}

tests/util/test_yaml.c

@@ -7,11 +7,12 @@
 #include <dse/clib/util/yaml.h>
 #include <dse/logger.h>
 
-#define FILENAME      "util/test.yaml"
-#define FILE          "util/values.yaml"
-#define EMPTY_FILE    "util/empty_doc.yaml"
-#define UINT_FILE     "util/uint.yaml"
-#define BOOL_FILE     "util/bool.yaml"
+#define FILENAME      "util/data/test.yaml"
+#define FILE          "util/data/values.yaml"
+#define EMPTY_FILE    "util/data/empty_doc.yaml"
+#define UINT_FILE     "util/data/uint.yaml"
+#define BOOL_FILE     "util/data/bool.yaml"
+#define DICT_DUP_FILE "util/data/dict_dup.yaml"
 
 #define ARRAY_SIZE(x) (sizeof(x) / sizeof(x[0]))
 #define UNUSED(x)     ((void)x)
@@ -359,3 +360,46 @@ void test_yaml_find_node_seq(void** state)
 
     dse_yaml_destroy_node(doc);
 }
+
+
+void test_yaml_duplicated_dict_entry(void** state)
+{
+    /*
+    Checking that this does not memory leak.
+          annotations:
+            struct_member_name: temperature
+            struct_member_offset: 2
+            struct_member_primitive_type: int16_t
+            init_value: foo
+            init_value: bar    ****** duplicate ******
+    */
+    UNUSED(state);
+
+    YamlNode* doc = dse_yaml_load_single_doc(DICT_DUP_FILE);
+    assert_non_null(doc);
+    const char* s = dse_yaml_get_scalar(doc, "annotations/init_value");
+    assert_string_equal("bar", s);
+    dse_yaml_destroy_node(doc);
+}
+
+
+int run_yaml_tests(void)
+{
+    const struct CMUnitTest tests[] = {
+        cmocka_unit_test(test_yaml_load_single_doc),
+        cmocka_unit_test(test_yaml_load_file),
+        cmocka_unit_test(test_yaml_find_doc_doclist),
+        cmocka_unit_test(test_yaml_find_node_doclist),
+        cmocka_unit_test(test_yaml_find_node_seq_doclist),
+        cmocka_unit_test(test_yaml_find_node_seq),
+        cmocka_unit_test(test_yaml_get_uint),
+        cmocka_unit_test(test_yaml_get_int),
+        cmocka_unit_test(test_yaml_get_double),
+        cmocka_unit_test(test_yaml_get_bool),
+        cmocka_unit_test(test_yaml_get_string),
+        cmocka_unit_test(test_yaml_get_parser),
+        cmocka_unit_test(test_yaml_duplicated_dict_entry),
+    };
+
+    return cmocka_run_group_tests_name("YAML", tests, NULL, NULL);
+}