From 9ae3aa1d1c0b93b0566738881dc76d79d493846b Mon Sep 17 00:00:00 2001 From: William Woodall Date: Tue, 22 May 2018 03:30:31 -0700 Subject: [PATCH 1/3] changes to rcutils_path_join() --- rcl/src/rcl/node.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rcl/src/rcl/node.c b/rcl/src/rcl/node.c index f0d0eb0ae..909033b7e 100644 --- a/rcl/src/rcl/node.c +++ b/rcl/src/rcl/node.c @@ -99,7 +99,7 @@ const char * rcl_create_node_logger_name( return node_logger_name; } -const char * rcl_get_secure_root(const char * node_name) +const char * rcl_get_secure_root(const char * node_name, const rcl_allocator_t * allocator) { const char * ros_secure_root_env = NULL; if (NULL == node_name) { @@ -115,7 +115,7 @@ const char * rcl_get_secure_root(const char * node_name) if (!ros_secure_root_size) { return NULL; // environment variable was empty } - char * node_secure_root = rcutils_join_path(ros_secure_root_env, node_name); + char * node_secure_root = rcutils_join_path(ros_secure_root_env, node_name, *allocator); if (!rcutils_is_directory(node_secure_root)) { free(node_secure_root); return NULL; @@ -315,7 +315,7 @@ rcl_node_init( node_security_options.enforce_security = RMW_SECURITY_ENFORCEMENT_PERMISSIVE; } else { // if use_security // File discovery magic here - const char * node_secure_root = rcl_get_secure_root(name); + const char * node_secure_root = rcl_get_secure_root(name, allocator); if (node_secure_root) { node_security_options.security_root_path = node_secure_root; } else { From 1b0c9f8eea77e8961cd779a38cad29656aef326c Mon Sep 17 00:00:00 2001 From: William Woodall Date: Fri, 15 Jun 2018 16:10:30 -0700 Subject: [PATCH 2/3] remove uses of malloc/realloc/calloc/free for allocator usage instead --- rcl/src/rcl/node.c | 2 +- rcl/test/rcl/test_rcl.cpp | 26 +++--- rcl/test/rcl/test_time.cpp | 8 +- .../test/test_parse_yaml.cpp | 82 +++++++++++-------- 4 files changed, 67 insertions(+), 51 deletions(-) diff --git a/rcl/src/rcl/node.c b/rcl/src/rcl/node.c index 909033b7e..c64e76015 100644 --- a/rcl/src/rcl/node.c +++ b/rcl/src/rcl/node.c @@ -117,7 +117,7 @@ const char * rcl_get_secure_root(const char * node_name, const rcl_allocator_t * } char * node_secure_root = rcutils_join_path(ros_secure_root_env, node_name, *allocator); if (!rcutils_is_directory(node_secure_root)) { - free(node_secure_root); + allocator->deallocate(node_secure_root, allocator->state); return NULL; } return node_secure_root; diff --git a/rcl/test/rcl/test_rcl.cpp b/rcl/test/rcl/test_rcl.cpp index d00859fd7..3db600462 100644 --- a/rcl/test/rcl/test_rcl.cpp +++ b/rcl/test/rcl/test_rcl.cpp @@ -53,17 +53,20 @@ class CLASSNAME (TestRCLFixture, RMW_IMPLEMENTATION) : public ::testing::Test struct FakeTestArgv { FakeTestArgv() - : argc(2) + : argc(2), argv(nullptr) { - this->argv = reinterpret_cast(malloc(2 * sizeof(char *))); - if (!this->argv) { + this->argv = new char *[2]; + this->argv[0] = rcutils_format_string(allocator, "%s", "foo"); + if (!this->argv[0]) { + delete[] this->argv; + throw std::bad_alloc(); + } + this->argv[1] = rcutils_format_string(allocator, "%s", "bar"); + if (!this->argv[1]) { + allocator.deallocate(this->argv[0], allocator.state); + delete[] this->argv; throw std::bad_alloc(); } - static const size_t size = 10; - this->argv[0] = reinterpret_cast(malloc(size * sizeof(char))); - rcutils_snprintf(this->argv[0], size, "foo"); - this->argv[1] = reinterpret_cast(malloc(size * sizeof(char))); - rcutils_snprintf(this->argv[1], size, "bar"); } ~FakeTestArgv() @@ -72,15 +75,18 @@ struct FakeTestArgv if (this->argc > 0) { size_t unsigned_argc = this->argc; for (size_t i = 0; i < unsigned_argc; --i) { - free(this->argv[i]); + allocator.deallocate(this->argv[i], allocator.state); } } } - free(this->argv); + delete[] this->argv; } int argc; + std::string foo; + std::string bar; char ** argv; + rcutils_allocator_t allocator = rcutils_get_default_allocator(); private: FakeTestArgv(const FakeTestArgv &) = delete; diff --git a/rcl/test/rcl/test_time.cpp b/rcl/test/rcl/test_time.cpp index 85e426930..57ac4ad61 100644 --- a/rcl/test/rcl/test_time.cpp +++ b/rcl/test/rcl/test_time.cpp @@ -193,14 +193,14 @@ TEST(CLASSNAME(rcl_time, RMW_IMPLEMENTATION), default_clock_instanciation) { EXPECT_EQ(retval, RCL_RET_OK) << rcl_get_error_string_safe(); ASSERT_TRUE(rcl_clock_valid(&ros_clock)); - rcl_clock_t * steady_clock = - reinterpret_cast(calloc(1, sizeof(rcl_clock_t))); + rcl_clock_t * steady_clock = reinterpret_cast( + allocator.zero_allocate(1, sizeof(rcl_clock_t), allocator.state)); retval = rcl_steady_clock_init(steady_clock, &allocator); EXPECT_EQ(retval, RCL_RET_OK) << rcl_get_error_string_safe(); ASSERT_TRUE(rcl_clock_valid(steady_clock)); - rcl_clock_t * system_clock = - reinterpret_cast(calloc(1, sizeof(rcl_clock_t))); + rcl_clock_t * system_clock = reinterpret_cast( + allocator.zero_allocate(1, sizeof(rcl_clock_t), allocator.state)); retval = rcl_system_clock_init(system_clock, &allocator); EXPECT_EQ(retval, RCL_RET_OK) << rcl_get_error_string_safe(); ASSERT_TRUE(rcl_clock_valid(system_clock)); diff --git a/rcl_yaml_param_parser/test/test_parse_yaml.cpp b/rcl_yaml_param_parser/test/test_parse_yaml.cpp index 194726fbf..7d6815b3a 100644 --- a/rcl_yaml_param_parser/test/test_parse_yaml.cpp +++ b/rcl_yaml_param_parser/test/test_parse_yaml.cpp @@ -17,6 +17,7 @@ #include "rcl_yaml_param_parser/parser.h" +#include "rcutils/allocator.h" #include "rcutils/error_handling.h" #include "rcutils/filesystem.h" @@ -26,8 +27,9 @@ rcutils_allocator_t allocator = rcutils_get_default_allocator(); TEST(test_file_parser, correct_syntax) { rcutils_reset_error(); EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); - char * test_path = rcutils_join_path(cur_dir, "test"); - char * path = rcutils_join_path(test_path, "correct_config.yaml"); + rcutils_allocator_t allocator = rcutils_get_default_allocator(); + char * test_path = rcutils_join_path(cur_dir, "test", allocator); + char * path = rcutils_join_path(test_path, "correct_config.yaml", allocator); fprintf(stderr, "cur_path: %s\n", path); EXPECT_TRUE(rcutils_exists(path)); rcl_params_t * params_hdl = rcl_yaml_node_struct_init(allocator); @@ -37,15 +39,16 @@ TEST(test_file_parser, correct_syntax) { EXPECT_TRUE(res); rcl_yaml_node_struct_print(params_hdl); rcl_yaml_node_struct_fini(params_hdl); - free(test_path); - free(path); + allocator.deallocate(test_path, allocator.state); + allocator.deallocate(path, allocator.state); } TEST(test_file_parser, multi_ns_correct_syntax) { rcutils_reset_error(); EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); - char * test_path = rcutils_join_path(cur_dir, "test"); - char * path = rcutils_join_path(test_path, "multi_ns_correct.yaml"); + rcutils_allocator_t allocator = rcutils_get_default_allocator(); + char * test_path = rcutils_join_path(cur_dir, "test", allocator); + char * path = rcutils_join_path(test_path, "multi_ns_correct.yaml", allocator); fprintf(stderr, "cur_path: %s\n", path); EXPECT_TRUE(rcutils_exists(path)); rcl_params_t * params_hdl = rcl_yaml_node_struct_init(allocator); @@ -55,15 +58,16 @@ TEST(test_file_parser, multi_ns_correct_syntax) { EXPECT_TRUE(res); rcl_yaml_node_struct_print(params_hdl); rcl_yaml_node_struct_fini(params_hdl); - free(test_path); - free(path); + allocator.deallocate(test_path, allocator.state); + allocator.deallocate(path, allocator.state); } TEST(test_file_parser, seq_map1) { rcutils_reset_error(); EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); - char * test_path = rcutils_join_path(cur_dir, "test"); - char * path = rcutils_join_path(test_path, "seq_map1.yaml"); + rcutils_allocator_t allocator = rcutils_get_default_allocator(); + char * test_path = rcutils_join_path(cur_dir, "test", allocator); + char * path = rcutils_join_path(test_path, "seq_map1.yaml", allocator); fprintf(stderr, "cur_path: %s\n", path); EXPECT_TRUE(rcutils_exists(path)); rcl_params_t * params_hdl = rcl_yaml_node_struct_init(allocator); @@ -71,15 +75,16 @@ TEST(test_file_parser, seq_map1) { bool res = rcl_parse_yaml_file(path, params_hdl); fprintf(stderr, "%s\n", rcutils_get_error_string_safe()); EXPECT_FALSE(res); - free(test_path); - free(path); + allocator.deallocate(test_path, allocator.state); + allocator.deallocate(path, allocator.state); } TEST(test_file_parser, seq_map2) { rcutils_reset_error(); EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); - char * test_path = rcutils_join_path(cur_dir, "test"); - char * path = rcutils_join_path(test_path, "seq_map2.yaml"); + rcutils_allocator_t allocator = rcutils_get_default_allocator(); + char * test_path = rcutils_join_path(cur_dir, "test", allocator); + char * path = rcutils_join_path(test_path, "seq_map2.yaml", allocator); fprintf(stderr, "cur_path: %s\n", path); EXPECT_TRUE(rcutils_exists(path)); rcl_params_t * params_hdl = rcl_yaml_node_struct_init(allocator); @@ -87,15 +92,16 @@ TEST(test_file_parser, seq_map2) { bool res = rcl_parse_yaml_file(path, params_hdl); fprintf(stderr, "%s\n", rcutils_get_error_string_safe()); EXPECT_FALSE(res); - free(test_path); - free(path); + allocator.deallocate(test_path, allocator.state); + allocator.deallocate(path, allocator.state); } TEST(test_file_parser, params_with_no_node) { rcutils_reset_error(); EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); - char * test_path = rcutils_join_path(cur_dir, "test"); - char * path = rcutils_join_path(test_path, "params_with_no_node.yaml"); + rcutils_allocator_t allocator = rcutils_get_default_allocator(); + char * test_path = rcutils_join_path(cur_dir, "test", allocator); + char * path = rcutils_join_path(test_path, "params_with_no_node.yaml", allocator); fprintf(stderr, "cur_path: %s\n", path); EXPECT_TRUE(rcutils_exists(path)); rcl_params_t * params_hdl = rcl_yaml_node_struct_init(allocator); @@ -103,15 +109,16 @@ TEST(test_file_parser, params_with_no_node) { bool res = rcl_parse_yaml_file(path, params_hdl); fprintf(stderr, "%s\n", rcutils_get_error_string_safe()); EXPECT_FALSE(res); - free(test_path); - free(path); + allocator.deallocate(test_path, allocator.state); + allocator.deallocate(path, allocator.state); } TEST(test_file_parser, no_alias_support) { rcutils_reset_error(); EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); - char * test_path = rcutils_join_path(cur_dir, "test"); - char * path = rcutils_join_path(test_path, "no_alias_support.yaml"); + rcutils_allocator_t allocator = rcutils_get_default_allocator(); + char * test_path = rcutils_join_path(cur_dir, "test", allocator); + char * path = rcutils_join_path(test_path, "no_alias_support.yaml", allocator); fprintf(stderr, "cur_path: %s\n", path); EXPECT_TRUE(rcutils_exists(path)); rcl_params_t * params_hdl = rcl_yaml_node_struct_init(allocator); @@ -119,15 +126,16 @@ TEST(test_file_parser, no_alias_support) { bool res = rcl_parse_yaml_file(path, params_hdl); fprintf(stderr, "%s\n", rcutils_get_error_string_safe()); EXPECT_FALSE(res); - free(test_path); - free(path); + allocator.deallocate(test_path, allocator.state); + allocator.deallocate(path, allocator.state); } TEST(test_file_parser, max_string_sz) { rcutils_reset_error(); EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); - char * test_path = rcutils_join_path(cur_dir, "test"); - char * path = rcutils_join_path(test_path, "max_string_sz.yaml"); + rcutils_allocator_t allocator = rcutils_get_default_allocator(); + char * test_path = rcutils_join_path(cur_dir, "test", allocator); + char * path = rcutils_join_path(test_path, "max_string_sz.yaml", allocator); fprintf(stderr, "cur_path: %s\n", path); EXPECT_TRUE(rcutils_exists(path)); rcl_params_t * params_hdl = rcl_yaml_node_struct_init(allocator); @@ -135,15 +143,16 @@ TEST(test_file_parser, max_string_sz) { bool res = rcl_parse_yaml_file(path, params_hdl); fprintf(stderr, "%s\n", rcutils_get_error_string_safe()); EXPECT_FALSE(res); - free(test_path); - free(path); + allocator.deallocate(test_path, allocator.state); + allocator.deallocate(path, allocator.state); } TEST(test_file_parser, no_value1) { rcutils_reset_error(); EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); - char * test_path = rcutils_join_path(cur_dir, "test"); - char * path = rcutils_join_path(test_path, "no_value1.yaml"); + rcutils_allocator_t allocator = rcutils_get_default_allocator(); + char * test_path = rcutils_join_path(cur_dir, "test", allocator); + char * path = rcutils_join_path(test_path, "no_value1.yaml", allocator); fprintf(stderr, "cur_path: %s\n", path); EXPECT_TRUE(rcutils_exists(path)); rcl_params_t * params_hdl = rcl_yaml_node_struct_init(allocator); @@ -151,15 +160,16 @@ TEST(test_file_parser, no_value1) { bool res = rcl_parse_yaml_file(path, params_hdl); fprintf(stderr, "%s\n", rcutils_get_error_string_safe()); EXPECT_FALSE(res); - free(test_path); - free(path); + allocator.deallocate(test_path, allocator.state); + allocator.deallocate(path, allocator.state); } TEST(test_file_parser, indented_ns) { rcutils_reset_error(); EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); - char * test_path = rcutils_join_path(cur_dir, "test"); - char * path = rcutils_join_path(test_path, "indented_name_space.yaml"); + rcutils_allocator_t allocator = rcutils_get_default_allocator(); + char * test_path = rcutils_join_path(cur_dir, "test", allocator); + char * path = rcutils_join_path(test_path, "indented_name_space.yaml", allocator); fprintf(stderr, "cur_path: %s\n", path); EXPECT_TRUE(rcutils_exists(path)); rcl_params_t * params_hdl = rcl_yaml_node_struct_init(allocator); @@ -167,8 +177,8 @@ TEST(test_file_parser, indented_ns) { bool res = rcl_parse_yaml_file(path, params_hdl); fprintf(stderr, "%s\n", rcutils_get_error_string_safe()); EXPECT_FALSE(res); - free(test_path); - free(path); + allocator.deallocate(test_path, allocator.state); + allocator.deallocate(path, allocator.state); } int32_t main(int32_t argc, char ** argv) From 56e93cc8df212718b2039a60e375a48dc65955c8 Mon Sep 17 00:00:00 2001 From: William Woodall Date: Sat, 16 Jun 2018 18:40:48 -0700 Subject: [PATCH 3/3] remove unused class members --- rcl/test/rcl/test_rcl.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/rcl/test/rcl/test_rcl.cpp b/rcl/test/rcl/test_rcl.cpp index 5a217c380..3b0ca5efa 100644 --- a/rcl/test/rcl/test_rcl.cpp +++ b/rcl/test/rcl/test_rcl.cpp @@ -88,10 +88,7 @@ struct FakeTestArgv rcutils_allocator_t allocator; int argc; - std::string foo; - std::string bar; char ** argv; - rcutils_allocator_t allocator = rcutils_get_default_allocator(); private: FakeTestArgv(const FakeTestArgv &) = delete;