From da5356f9c4c243b6c02b344e3a8fd765137aeb38 Mon Sep 17 00:00:00 2001 From: "Rule Timothy (VM/EMT3)" Date: Wed, 31 Jan 2024 15:14:47 +0100 Subject: [PATCH] Fix yaml memory leak for duplicate dict keys. Refactor yaml test layout. Signed-off-by: Rule Timothy (VM/EMT3) --- dse/clib/util/yaml.c | 7 +++- tests/util/CMakeLists.txt | 8 ++--- tests/util/__test__.c | 50 +++++++------------------- tests/util/{ => data}/bool.yaml | 0 tests/util/data/dict_dup.yaml | 6 ++++ tests/util/{ => data}/empty_doc.yaml | 0 tests/util/{ => data}/test.yaml | 0 tests/util/{ => data}/uint.yaml | 0 tests/util/{ => data}/values.yaml | 0 tests/util/test_binary.c | 11 ++++++ tests/util/test_yaml.c | 54 +++++++++++++++++++++++++--- 11 files changed, 89 insertions(+), 47 deletions(-) rename tests/util/{ => data}/bool.yaml (100%) create mode 100644 tests/util/data/dict_dup.yaml rename tests/util/{ => data}/empty_doc.yaml (100%) rename tests/util/{ => data}/test.yaml (100%) rename tests/util/{ => data}/uint.yaml (100%) rename tests/util/{ => data}/values.yaml (100%) diff --git a/dse/clib/util/yaml.c b/dse/clib/util/yaml.c index ab49929..e67619f 100644 --- a/dse/clib/util/yaml.c +++ b/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); } diff --git a/tests/util/CMakeLists.txt b/tests/util/CMakeLists.txt index cf9410a..1aae587 100644 --- a/tests/util/CMakeLists.txt +++ b/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( diff --git a/tests/util/__test__.c b/tests/util/__test__.c index b7cbc66..42083df 100644 --- a/tests/util/__test__.c +++ b/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); -} diff --git a/tests/util/bool.yaml b/tests/util/data/bool.yaml similarity index 100% rename from tests/util/bool.yaml rename to tests/util/data/bool.yaml diff --git a/tests/util/data/dict_dup.yaml b/tests/util/data/dict_dup.yaml new file mode 100644 index 0000000..77ae60d --- /dev/null +++ b/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 diff --git a/tests/util/empty_doc.yaml b/tests/util/data/empty_doc.yaml similarity index 100% rename from tests/util/empty_doc.yaml rename to tests/util/data/empty_doc.yaml diff --git a/tests/util/test.yaml b/tests/util/data/test.yaml similarity index 100% rename from tests/util/test.yaml rename to tests/util/data/test.yaml diff --git a/tests/util/uint.yaml b/tests/util/data/uint.yaml similarity index 100% rename from tests/util/uint.yaml rename to tests/util/data/uint.yaml diff --git a/tests/util/values.yaml b/tests/util/data/values.yaml similarity index 100% rename from tests/util/values.yaml rename to tests/util/data/values.yaml diff --git a/tests/util/test_binary.c b/tests/util/test_binary.c index ab92938..56e40c9 100644 --- a/tests/util/test_binary.c +++ b/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); +} diff --git a/tests/util/test_yaml.c b/tests/util/test_yaml.c index 8f7c56a..089411d 100644 --- a/tests/util/test_yaml.c +++ b/tests/util/test_yaml.c @@ -7,11 +7,12 @@ #include #include -#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); +}