From 6040298b429784ca9eb4d0404543d6211fd5f81d Mon Sep 17 00:00:00 2001 From: brawner Date: Mon, 31 Aug 2020 16:03:52 -0700 Subject: [PATCH] Fix mem leaks in unit test from 776 (#779) Signed-off-by: Stephen Brawner --- .../test/test_parse_yaml.cpp | 35 +++++++++++++++---- rcl_yaml_param_parser/test/test_parser.cpp | 16 ++++----- 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/rcl_yaml_param_parser/test/test_parse_yaml.cpp b/rcl_yaml_param_parser/test/test_parse_yaml.cpp index 8330e0be7..7f3e03223 100644 --- a/rcl_yaml_param_parser/test/test_parse_yaml.cpp +++ b/rcl_yaml_param_parser/test/test_parse_yaml.cpp @@ -312,9 +312,12 @@ TEST(test_file_parser, seq_map1) { ASSERT_TRUE(rcutils_exists(path)) << "No test YAML file found at " << path; rcl_params_t * params_hdl = rcl_yaml_node_struct_init(allocator); ASSERT_TRUE(NULL != params_hdl) << rcutils_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + rcl_yaml_node_struct_fini(params_hdl); + }); bool res = rcl_parse_yaml_file(path, params_hdl); EXPECT_FALSE(res); - // No cleanup, rcl_parse_yaml_file takes care of that if it fails. } TEST(test_file_parser, seq_map2) { @@ -336,9 +339,12 @@ TEST(test_file_parser, seq_map2) { ASSERT_TRUE(rcutils_exists(path)) << "No test YAML file found at " << path; rcl_params_t * params_hdl = rcl_yaml_node_struct_init(allocator); ASSERT_TRUE(NULL != params_hdl) << rcutils_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + rcl_yaml_node_struct_fini(params_hdl); + }); bool res = rcl_parse_yaml_file(path, params_hdl); EXPECT_FALSE(res); - // No cleanup, rcl_parse_yaml_file takes care of that if it fails } TEST(test_file_parser, params_with_no_node) { @@ -360,9 +366,12 @@ TEST(test_file_parser, params_with_no_node) { ASSERT_TRUE(rcutils_exists(path)) << "No test YAML file found at " << path; rcl_params_t * params_hdl = rcl_yaml_node_struct_init(allocator); ASSERT_TRUE(NULL != params_hdl) << rcutils_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + rcl_yaml_node_struct_fini(params_hdl); + }); bool res = rcl_parse_yaml_file(path, params_hdl); EXPECT_FALSE(res); - // No cleanup, rcl_parse_yaml_file takes care of that if it fails. } TEST(test_file_parser, no_alias_support) { @@ -384,9 +393,12 @@ TEST(test_file_parser, no_alias_support) { ASSERT_TRUE(rcutils_exists(path)) << "No test YAML file found at " << path; rcl_params_t * params_hdl = rcl_yaml_node_struct_init(allocator); ASSERT_TRUE(NULL != params_hdl) << rcutils_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + rcl_yaml_node_struct_fini(params_hdl); + }); bool res = rcl_parse_yaml_file(path, params_hdl); EXPECT_FALSE(res); - // No cleanup, rcl_parse_yaml_file takes care of that if it fails. } TEST(test_file_parser, empty_string) { @@ -436,9 +448,12 @@ TEST(test_file_parser, no_value1) { ASSERT_TRUE(rcutils_exists(path)) << "No test YAML file found at " << path; rcl_params_t * params_hdl = rcl_yaml_node_struct_init(allocator); ASSERT_TRUE(NULL != params_hdl) << rcutils_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + rcl_yaml_node_struct_fini(params_hdl); + }); bool res = rcl_parse_yaml_file(path, params_hdl); EXPECT_FALSE(res); - // No cleanup, rcl_parse_yaml_file takes care of that if it fails. } TEST(test_file_parser, indented_ns) { @@ -460,9 +475,12 @@ TEST(test_file_parser, indented_ns) { ASSERT_TRUE(rcutils_exists(path)) << "No test YAML file found at " << path; rcl_params_t * params_hdl = rcl_yaml_node_struct_init(allocator); ASSERT_TRUE(NULL != params_hdl) << rcutils_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + rcl_yaml_node_struct_fini(params_hdl); + }); bool res = rcl_parse_yaml_file(path, params_hdl); EXPECT_FALSE(res); - // No cleanup, rcl_parse_yaml_file takes care of that if it fails. } // Regression test for https://github.com/ros2/rcl/issues/419 @@ -485,9 +503,12 @@ TEST(test_file_parser, maximum_number_parameters) { ASSERT_TRUE(rcutils_exists(path)) << "No test YAML file found at " << path; rcl_params_t * params_hdl = rcl_yaml_node_struct_init(allocator); ASSERT_TRUE(NULL != params_hdl) << rcutils_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + rcl_yaml_node_struct_fini(params_hdl); + }); bool res = rcl_parse_yaml_file(path, params_hdl); EXPECT_FALSE(res); - // No cleanup, rcl_parse_yaml_file takes care of that if it fails. } int32_t main(int32_t argc, char ** argv) diff --git a/rcl_yaml_param_parser/test/test_parser.cpp b/rcl_yaml_param_parser/test/test_parser.cpp index 7fbb7f8ce..34dc84485 100644 --- a/rcl_yaml_param_parser/test/test_parser.cpp +++ b/rcl_yaml_param_parser/test/test_parser.cpp @@ -330,11 +330,9 @@ TEST(RclYamlParamParser, test_parse_file_with_bad_allocator) { bool res = rcl_parse_yaml_file(path, params_hdl); // Not verifying res is true or false here, because eventually it will come back with an ok // result. We're just trying to make sure that bad allocations are properly handled - if (res) { - // This is already freed in the case of a non-ok error in rcl_parse_yaml_file - rcl_yaml_node_struct_fini(params_hdl); - params_hdl = NULL; - } + (void)res; + rcl_yaml_node_struct_fini(params_hdl); + params_hdl = NULL; } // Check sporadic failing calloc calls @@ -347,11 +345,9 @@ TEST(RclYamlParamParser, test_parse_file_with_bad_allocator) { bool res = rcl_parse_yaml_file(path, params_hdl); // Not verifying res is true or false here, because eventually it will come back with an ok // result. We're just trying to make sure that bad allocations are properly handled - if (res) { - // This is already freed in the case of a non-ok error in rcl_parse_yaml_file - rcl_yaml_node_struct_fini(params_hdl); - params_hdl = NULL; - } + (void)res; + rcl_yaml_node_struct_fini(params_hdl); + params_hdl = NULL; } }