From e8e6e476231de0a99a8451b6ce4edf52221adff6 Mon Sep 17 00:00:00 2001 From: Chen Lihui Date: Tue, 22 Sep 2020 17:39:49 +0800 Subject: [PATCH 01/14] Fix for allowing '//' in node name Signed-off-by: Chen Lihui --- rcl_yaml_param_parser/CMakeLists.txt | 4 +- rcl_yaml_param_parser/package.xml | 1 + rcl_yaml_param_parser/src/namespace.c | 46 +++++++++++++++++++ .../test/empty_name_in_ns.yaml | 14 ++++++ .../test/test_parse_yaml.cpp | 27 +++++++++++ 5 files changed, 91 insertions(+), 1 deletion(-) create mode 100644 rcl_yaml_param_parser/test/empty_name_in_ns.yaml diff --git a/rcl_yaml_param_parser/CMakeLists.txt b/rcl_yaml_param_parser/CMakeLists.txt index 2619eb627..dc8ad6dd3 100644 --- a/rcl_yaml_param_parser/CMakeLists.txt +++ b/rcl_yaml_param_parser/CMakeLists.txt @@ -4,6 +4,7 @@ project(rcl_yaml_param_parser) find_package(ament_cmake_ros REQUIRED) find_package(rcutils REQUIRED) +find_package(rmw REQUIRED) find_package(libyaml_vendor REQUIRED) # Default to C++14 @@ -30,7 +31,7 @@ add_library( target_include_directories(${PROJECT_NAME} PUBLIC "$" "$") -ament_target_dependencies(${PROJECT_NAME} "libyaml_vendor" "rcutils") +ament_target_dependencies(${PROJECT_NAME} "libyaml_vendor" "rcutils" "rmw") # Set the visibility to hidden by default if possible if(CMAKE_C_COMPILER_ID STREQUAL "GNU" OR CMAKE_C_COMPILER_ID MATCHES "Clang") @@ -181,6 +182,7 @@ if(BUILD_TESTING) endif() ament_export_dependencies(ament_cmake libyaml_vendor) +ament_export_dependencies(rmw) ament_export_include_directories(include) install( DIRECTORY include/ diff --git a/rcl_yaml_param_parser/package.xml b/rcl_yaml_param_parser/package.xml index 7d522bdd1..b4579d381 100644 --- a/rcl_yaml_param_parser/package.xml +++ b/rcl_yaml_param_parser/package.xml @@ -14,6 +14,7 @@ libyaml_vendor yaml + rmw rcutils ament_cmake_gtest diff --git a/rcl_yaml_param_parser/src/namespace.c b/rcl_yaml_param_parser/src/namespace.c index 0ca3fed67..2e377b3e6 100644 --- a/rcl_yaml_param_parser/src/namespace.c +++ b/rcl_yaml_param_parser/src/namespace.c @@ -14,8 +14,48 @@ #include "./impl/namespace.h" +#include "rcutils/format_string.h" #include "rcutils/strdup.h" +#include "rmw/error_handling.h" +#include "rmw/validate_namespace.h" + +/// +/// Validate a name whether it is a valid namespace +/// +static rcutils_ret_t +__validate_namespace(const char * name, rcutils_allocator_t allocator) +{ + // rmw_validate_namespace will report RMW_NAMESPACE_INVALID_NOT_ABSOLUTE + // so add prefix '/' to name to make an absolute name if necessary + char * absolute_name = NULL; + if (name[0] != '/') { + absolute_name = rcutils_format_string(allocator, "/%s", name); + if (NULL == absolute_name) { + RCUTILS_SET_ERROR_MSG("failed to allocate memory for absolute name"); + return RCUTILS_RET_BAD_ALLOC; + } + } + int validation_result = 0; + rmw_ret_t ret; + if (absolute_name) { + ret = rmw_validate_namespace(absolute_name, &validation_result, NULL); + allocator.deallocate(absolute_name, allocator.state); + } else { + ret = rmw_validate_namespace(name, &validation_result, NULL); + } + if (ret != RMW_RET_OK) { + RCUTILS_SET_ERROR_MSG(rmw_get_error_string().str); + return RCUTILS_RET_ERROR; + } else { + if (validation_result != RMW_NAMESPACE_VALID) { + RCUTILS_SET_ERROR_MSG(rmw_namespace_validation_result_string(validation_result)); + return RCUTILS_RET_INVALID_ARGUMENT; + } + } + return RCUTILS_RET_OK; +} + rcutils_ret_t add_name_to_ns( namespace_tracker_t * ns_tracker, const char * name, @@ -49,6 +89,12 @@ rcutils_ret_t add_name_to_ns( if (NULL == name) { return RCUTILS_RET_INVALID_ARGUMENT; } + if (namespace_type == NS_TYPE_NODE) { + rcutils_ret_t ret = __validate_namespace(name, allocator); + if (RCUTILS_RET_OK != ret) { + return ret; + } + } if (0U == *cur_count) { cur_ns = rcutils_strdup(name, allocator); if (NULL == cur_ns) { diff --git a/rcl_yaml_param_parser/test/empty_name_in_ns.yaml b/rcl_yaml_param_parser/test/empty_name_in_ns.yaml new file mode 100644 index 000000000..34230f409 --- /dev/null +++ b/rcl_yaml_param_parser/test/empty_name_in_ns.yaml @@ -0,0 +1,14 @@ +lidar_ns//lidar_w: + ros__parameters: + id: 10 + name: front_lidar + ports: [2438, 2439, 2440] + driver1: + dx: 4.56 + dy: 2.30 + fr_sensor_specs: [12, 3, 0, 7] + bk_sensor_specs: [12.1, -2.3, 5.2, 9.0] + is_front: true + driver2: + dx: 1.23 + dy: 0.45 diff --git a/rcl_yaml_param_parser/test/test_parse_yaml.cpp b/rcl_yaml_param_parser/test/test_parse_yaml.cpp index 89e1ec9e6..f53b0194f 100644 --- a/rcl_yaml_param_parser/test/test_parse_yaml.cpp +++ b/rcl_yaml_param_parser/test/test_parse_yaml.cpp @@ -531,6 +531,33 @@ TEST(test_file_parser, special_float_point) { EXPECT_TRUE(std::isinf(param_value->double_array_value->values[6])); } +TEST(test_file_parser, empty_name_in_ns) { + rcutils_reset_error(); + EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); + rcutils_allocator_t allocator = rcutils_get_default_allocator(); + char * test_path = rcutils_join_path(cur_dir, "test", allocator); + ASSERT_TRUE(NULL != test_path) << rcutils_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + allocator.deallocate(test_path, allocator.state); + }); + char * path = rcutils_join_path(test_path, "empty_name_in_ns.yaml", allocator); + ASSERT_TRUE(NULL != path) << rcutils_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + allocator.deallocate(path, allocator.state); + }); + EXPECT_TRUE(rcutils_exists(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); +} + int32_t main(int32_t argc, char ** argv) { ::testing::InitGoogleTest(&argc, argv); From 52bb028ffbb8b225e420fbef11e7c73f09d50bdb Mon Sep 17 00:00:00 2001 From: Chen Lihui Date: Wed, 23 Sep 2020 10:19:23 +0800 Subject: [PATCH 02/14] Remove rmw dependency, just to check two slashes Signed-off-by: Chen Lihui --- rcl_yaml_param_parser/CMakeLists.txt | 4 +--- rcl_yaml_param_parser/package.xml | 1 - rcl_yaml_param_parser/src/namespace.c | 33 +++------------------------ 3 files changed, 4 insertions(+), 34 deletions(-) diff --git a/rcl_yaml_param_parser/CMakeLists.txt b/rcl_yaml_param_parser/CMakeLists.txt index dc8ad6dd3..2619eb627 100644 --- a/rcl_yaml_param_parser/CMakeLists.txt +++ b/rcl_yaml_param_parser/CMakeLists.txt @@ -4,7 +4,6 @@ project(rcl_yaml_param_parser) find_package(ament_cmake_ros REQUIRED) find_package(rcutils REQUIRED) -find_package(rmw REQUIRED) find_package(libyaml_vendor REQUIRED) # Default to C++14 @@ -31,7 +30,7 @@ add_library( target_include_directories(${PROJECT_NAME} PUBLIC "$" "$") -ament_target_dependencies(${PROJECT_NAME} "libyaml_vendor" "rcutils" "rmw") +ament_target_dependencies(${PROJECT_NAME} "libyaml_vendor" "rcutils") # Set the visibility to hidden by default if possible if(CMAKE_C_COMPILER_ID STREQUAL "GNU" OR CMAKE_C_COMPILER_ID MATCHES "Clang") @@ -182,7 +181,6 @@ if(BUILD_TESTING) endif() ament_export_dependencies(ament_cmake libyaml_vendor) -ament_export_dependencies(rmw) ament_export_include_directories(include) install( DIRECTORY include/ diff --git a/rcl_yaml_param_parser/package.xml b/rcl_yaml_param_parser/package.xml index b4579d381..7d522bdd1 100644 --- a/rcl_yaml_param_parser/package.xml +++ b/rcl_yaml_param_parser/package.xml @@ -14,7 +14,6 @@ libyaml_vendor yaml - rmw rcutils ament_cmake_gtest diff --git a/rcl_yaml_param_parser/src/namespace.c b/rcl_yaml_param_parser/src/namespace.c index 2e377b3e6..7b9949c68 100644 --- a/rcl_yaml_param_parser/src/namespace.c +++ b/rcl_yaml_param_parser/src/namespace.c @@ -14,45 +14,18 @@ #include "./impl/namespace.h" -#include "rcutils/format_string.h" #include "rcutils/strdup.h" -#include "rmw/error_handling.h" -#include "rmw/validate_namespace.h" - /// /// Validate a name whether it is a valid namespace /// static rcutils_ret_t __validate_namespace(const char * name, rcutils_allocator_t allocator) { - // rmw_validate_namespace will report RMW_NAMESPACE_INVALID_NOT_ABSOLUTE - // so add prefix '/' to name to make an absolute name if necessary - char * absolute_name = NULL; - if (name[0] != '/') { - absolute_name = rcutils_format_string(allocator, "/%s", name); - if (NULL == absolute_name) { - RCUTILS_SET_ERROR_MSG("failed to allocate memory for absolute name"); - return RCUTILS_RET_BAD_ALLOC; - } - } - int validation_result = 0; - rmw_ret_t ret; - if (absolute_name) { - ret = rmw_validate_namespace(absolute_name, &validation_result, NULL); - allocator.deallocate(absolute_name, allocator.state); - } else { - ret = rmw_validate_namespace(name, &validation_result, NULL); - } - if (ret != RMW_RET_OK) { - RCUTILS_SET_ERROR_MSG(rmw_get_error_string().str); - return RCUTILS_RET_ERROR; - } else { - if (validation_result != RMW_NAMESPACE_VALID) { - RCUTILS_SET_ERROR_MSG(rmw_namespace_validation_result_string(validation_result)); - return RCUTILS_RET_INVALID_ARGUMENT; - } + if (strstr(name, "//") != NULL) { + return RCUTILS_RET_INVALID_ARGUMENT; } + return RCUTILS_RET_OK; } From b84754426068dc646339626e4a3400ca906e1116 Mon Sep 17 00:00:00 2001 From: Chen Lihui Date: Wed, 23 Sep 2020 10:30:38 +0800 Subject: [PATCH 03/14] Fix compile warning Signed-off-by: Chen Lihui --- rcl_yaml_param_parser/src/namespace.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rcl_yaml_param_parser/src/namespace.c b/rcl_yaml_param_parser/src/namespace.c index 7b9949c68..b37ac69b9 100644 --- a/rcl_yaml_param_parser/src/namespace.c +++ b/rcl_yaml_param_parser/src/namespace.c @@ -20,7 +20,7 @@ /// Validate a name whether it is a valid namespace /// static rcutils_ret_t -__validate_namespace(const char * name, rcutils_allocator_t allocator) +__validate_namespace(const char * name) { if (strstr(name, "//") != NULL) { return RCUTILS_RET_INVALID_ARGUMENT; @@ -63,7 +63,7 @@ rcutils_ret_t add_name_to_ns( return RCUTILS_RET_INVALID_ARGUMENT; } if (namespace_type == NS_TYPE_NODE) { - rcutils_ret_t ret = __validate_namespace(name, allocator); + rcutils_ret_t ret = __validate_namespace(name); if (RCUTILS_RET_OK != ret) { return ret; } From deba99e6c0775b4198bd88949975d20494053fbd Mon Sep 17 00:00:00 2001 From: Chen Lihui Date: Mon, 12 Oct 2020 11:09:57 +0800 Subject: [PATCH 04/14] Revert "Fix compile warning" This reverts commit b84754426068dc646339626e4a3400ca906e1116. Signed-off-by: Chen Lihui --- rcl_yaml_param_parser/src/namespace.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rcl_yaml_param_parser/src/namespace.c b/rcl_yaml_param_parser/src/namespace.c index b37ac69b9..7b9949c68 100644 --- a/rcl_yaml_param_parser/src/namespace.c +++ b/rcl_yaml_param_parser/src/namespace.c @@ -20,7 +20,7 @@ /// Validate a name whether it is a valid namespace /// static rcutils_ret_t -__validate_namespace(const char * name) +__validate_namespace(const char * name, rcutils_allocator_t allocator) { if (strstr(name, "//") != NULL) { return RCUTILS_RET_INVALID_ARGUMENT; @@ -63,7 +63,7 @@ rcutils_ret_t add_name_to_ns( return RCUTILS_RET_INVALID_ARGUMENT; } if (namespace_type == NS_TYPE_NODE) { - rcutils_ret_t ret = __validate_namespace(name); + rcutils_ret_t ret = __validate_namespace(name, allocator); if (RCUTILS_RET_OK != ret) { return ret; } From 83313f8bb881f39bbc9d9d3f67bf1cd22f97131c Mon Sep 17 00:00:00 2001 From: Chen Lihui Date: Mon, 12 Oct 2020 11:10:10 +0800 Subject: [PATCH 05/14] Revert "Remove rmw dependency, just to check two slashes" This reverts commit 52bb028ffbb8b225e420fbef11e7c73f09d50bdb. Signed-off-by: Chen Lihui --- rcl_yaml_param_parser/CMakeLists.txt | 4 +++- rcl_yaml_param_parser/package.xml | 1 + rcl_yaml_param_parser/src/namespace.c | 33 ++++++++++++++++++++++++--- 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/rcl_yaml_param_parser/CMakeLists.txt b/rcl_yaml_param_parser/CMakeLists.txt index 2619eb627..dc8ad6dd3 100644 --- a/rcl_yaml_param_parser/CMakeLists.txt +++ b/rcl_yaml_param_parser/CMakeLists.txt @@ -4,6 +4,7 @@ project(rcl_yaml_param_parser) find_package(ament_cmake_ros REQUIRED) find_package(rcutils REQUIRED) +find_package(rmw REQUIRED) find_package(libyaml_vendor REQUIRED) # Default to C++14 @@ -30,7 +31,7 @@ add_library( target_include_directories(${PROJECT_NAME} PUBLIC "$" "$") -ament_target_dependencies(${PROJECT_NAME} "libyaml_vendor" "rcutils") +ament_target_dependencies(${PROJECT_NAME} "libyaml_vendor" "rcutils" "rmw") # Set the visibility to hidden by default if possible if(CMAKE_C_COMPILER_ID STREQUAL "GNU" OR CMAKE_C_COMPILER_ID MATCHES "Clang") @@ -181,6 +182,7 @@ if(BUILD_TESTING) endif() ament_export_dependencies(ament_cmake libyaml_vendor) +ament_export_dependencies(rmw) ament_export_include_directories(include) install( DIRECTORY include/ diff --git a/rcl_yaml_param_parser/package.xml b/rcl_yaml_param_parser/package.xml index 7d522bdd1..b4579d381 100644 --- a/rcl_yaml_param_parser/package.xml +++ b/rcl_yaml_param_parser/package.xml @@ -14,6 +14,7 @@ libyaml_vendor yaml + rmw rcutils ament_cmake_gtest diff --git a/rcl_yaml_param_parser/src/namespace.c b/rcl_yaml_param_parser/src/namespace.c index 7b9949c68..2e377b3e6 100644 --- a/rcl_yaml_param_parser/src/namespace.c +++ b/rcl_yaml_param_parser/src/namespace.c @@ -14,18 +14,45 @@ #include "./impl/namespace.h" +#include "rcutils/format_string.h" #include "rcutils/strdup.h" +#include "rmw/error_handling.h" +#include "rmw/validate_namespace.h" + /// /// Validate a name whether it is a valid namespace /// static rcutils_ret_t __validate_namespace(const char * name, rcutils_allocator_t allocator) { - if (strstr(name, "//") != NULL) { - return RCUTILS_RET_INVALID_ARGUMENT; + // rmw_validate_namespace will report RMW_NAMESPACE_INVALID_NOT_ABSOLUTE + // so add prefix '/' to name to make an absolute name if necessary + char * absolute_name = NULL; + if (name[0] != '/') { + absolute_name = rcutils_format_string(allocator, "/%s", name); + if (NULL == absolute_name) { + RCUTILS_SET_ERROR_MSG("failed to allocate memory for absolute name"); + return RCUTILS_RET_BAD_ALLOC; + } + } + int validation_result = 0; + rmw_ret_t ret; + if (absolute_name) { + ret = rmw_validate_namespace(absolute_name, &validation_result, NULL); + allocator.deallocate(absolute_name, allocator.state); + } else { + ret = rmw_validate_namespace(name, &validation_result, NULL); + } + if (ret != RMW_RET_OK) { + RCUTILS_SET_ERROR_MSG(rmw_get_error_string().str); + return RCUTILS_RET_ERROR; + } else { + if (validation_result != RMW_NAMESPACE_VALID) { + RCUTILS_SET_ERROR_MSG(rmw_namespace_validation_result_string(validation_result)); + return RCUTILS_RET_INVALID_ARGUMENT; + } } - return RCUTILS_RET_OK; } From 682d4db9123ae2d9683e6ae9cf206e2a8badeecf Mon Sep 17 00:00:00 2001 From: Chen Lihui Date: Mon, 12 Oct 2020 13:53:09 +0800 Subject: [PATCH 06/14] Update for a more general check Signed-off-by: Chen Lihui --- rcl_yaml_param_parser/src/namespace.c | 46 ---------- rcl_yaml_param_parser/src/parse.c | 123 ++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 46 deletions(-) diff --git a/rcl_yaml_param_parser/src/namespace.c b/rcl_yaml_param_parser/src/namespace.c index 2e377b3e6..0ca3fed67 100644 --- a/rcl_yaml_param_parser/src/namespace.c +++ b/rcl_yaml_param_parser/src/namespace.c @@ -14,48 +14,8 @@ #include "./impl/namespace.h" -#include "rcutils/format_string.h" #include "rcutils/strdup.h" -#include "rmw/error_handling.h" -#include "rmw/validate_namespace.h" - -/// -/// Validate a name whether it is a valid namespace -/// -static rcutils_ret_t -__validate_namespace(const char * name, rcutils_allocator_t allocator) -{ - // rmw_validate_namespace will report RMW_NAMESPACE_INVALID_NOT_ABSOLUTE - // so add prefix '/' to name to make an absolute name if necessary - char * absolute_name = NULL; - if (name[0] != '/') { - absolute_name = rcutils_format_string(allocator, "/%s", name); - if (NULL == absolute_name) { - RCUTILS_SET_ERROR_MSG("failed to allocate memory for absolute name"); - return RCUTILS_RET_BAD_ALLOC; - } - } - int validation_result = 0; - rmw_ret_t ret; - if (absolute_name) { - ret = rmw_validate_namespace(absolute_name, &validation_result, NULL); - allocator.deallocate(absolute_name, allocator.state); - } else { - ret = rmw_validate_namespace(name, &validation_result, NULL); - } - if (ret != RMW_RET_OK) { - RCUTILS_SET_ERROR_MSG(rmw_get_error_string().str); - return RCUTILS_RET_ERROR; - } else { - if (validation_result != RMW_NAMESPACE_VALID) { - RCUTILS_SET_ERROR_MSG(rmw_namespace_validation_result_string(validation_result)); - return RCUTILS_RET_INVALID_ARGUMENT; - } - } - return RCUTILS_RET_OK; -} - rcutils_ret_t add_name_to_ns( namespace_tracker_t * ns_tracker, const char * name, @@ -89,12 +49,6 @@ rcutils_ret_t add_name_to_ns( if (NULL == name) { return RCUTILS_RET_INVALID_ARGUMENT; } - if (namespace_type == NS_TYPE_NODE) { - rcutils_ret_t ret = __validate_namespace(name, allocator); - if (RCUTILS_RET_OK != ret) { - return ret; - } - } if (0U == *cur_count) { cur_ns = rcutils_strdup(name, allocator); if (NULL == cur_ns) { diff --git a/rcl_yaml_param_parser/src/parse.c b/rcl_yaml_param_parser/src/parse.c index c60e382f7..1936676d5 100644 --- a/rcl_yaml_param_parser/src/parse.c +++ b/rcl_yaml_param_parser/src/parse.c @@ -18,8 +18,13 @@ #include #include "rcutils/allocator.h" +#include "rcutils/format_string.h" #include "rcutils/strdup.h" +#include "rmw/error_handling.h" +#include "rmw/validate_namespace.h" +#include "rmw/validate_node_name.h" + #include "./impl/add_to_arrays.h" #include "./impl/parse.h" #include "./impl/namespace.h" @@ -404,6 +409,118 @@ rcutils_ret_t parse_value( return ret; } +/// +/// Check a name space whether it is valid +/// +static rcutils_ret_t +__validate_namespace(const char * namespace) +{ + int validation_result = 0; + rmw_ret_t ret; + ret = rmw_validate_namespace(namespace, &validation_result, NULL); + if (ret != RMW_RET_OK) { + RCUTILS_SET_ERROR_MSG(rmw_get_error_string().str); + return RCUTILS_RET_ERROR; + } + if (validation_result != RMW_NAMESPACE_VALID) { + RCUTILS_SET_ERROR_MSG(rmw_namespace_validation_result_string(validation_result)); + return RCUTILS_RET_INVALID_ARGUMENT; + } + + return RCUTILS_RET_OK; +} + +/// +/// Check a node name whether it is valid +/// +static rcutils_ret_t +__validate_nodename(const char * name) +{ + int validation_result = 0; + rmw_ret_t ret; + ret = rmw_validate_node_name(name, &validation_result, NULL); + if (ret != RMW_RET_OK) { + RCUTILS_SET_ERROR_MSG(rmw_get_error_string().str); + return RCUTILS_RET_ERROR; + } + if (validation_result != RMW_NODE_NAME_VALID) { + RCUTILS_SET_ERROR_MSG(rmw_node_name_validation_result_string(validation_result)); + return RCUTILS_RET_ERROR; + } + + return RCUTILS_RET_OK; +} + +/// +/// Check a name (namespace/node_name) whether it is valid +/// +static rcutils_ret_t +__validate_name(const char * name, rcutils_allocator_t allocator) +{ + // special rules + if (strcmp(name, "/**") == 0) { + return RCUTILS_RET_OK; + } + + rcutils_ret_t ret = RCUTILS_RET_OK; + char * separator_pos = rindex(name, '/'); + char * namespace = NULL; + char * node_name = NULL; + char * absolute_namespace = NULL; + if (NULL == separator_pos) { + node_name = rcutils_strdup(name, allocator); + if (NULL == node_name) { + ret = RCUTILS_RET_BAD_ALLOC; + goto clean; + } + } else { + namespace = rcutils_strndup(name, separator_pos - name, allocator); + if (NULL == namespace) { + ret = RCUTILS_RET_BAD_ALLOC; + goto clean; + } + if (namespace[0] != '/') { + absolute_namespace = rcutils_format_string(allocator, "/%s", namespace); + if (NULL == absolute_namespace) { + ret = RCUTILS_RET_BAD_ALLOC; + goto clean; + } + } + node_name = rcutils_strdup(separator_pos + 1, allocator); + if (NULL == node_name) { + ret = RCUTILS_RET_BAD_ALLOC; + goto clean; + } + } + + if (absolute_namespace) { + ret = __validate_namespace(absolute_namespace); + } else if (namespace) { + ret = __validate_namespace(namespace); + } + + if (ret != RCUTILS_RET_OK) { + goto clean; + } + + ret = __validate_nodename(node_name); + if (ret != RCUTILS_RET_OK) { + goto clean; + } + +clean: + if (absolute_namespace) { + allocator.deallocate(absolute_namespace, allocator.state); + } + if (node_name) { + allocator.deallocate(node_name, allocator.state); + } + if (namespace) { + allocator.deallocate(namespace, allocator.state); + } + return ret; +} + /// /// Parse the key part of the pair /// @@ -466,6 +583,12 @@ rcutils_ret_t parse_key( break; } + ret = __validate_name(node_name_ns, allocator); + if (RCUTILS_RET_OK != ret) { + allocator.deallocate(node_name_ns, allocator.state); + break; + } + ret = find_node(node_name_ns, params_st, node_idx); allocator.deallocate(node_name_ns, allocator.state); if (RCUTILS_RET_OK != ret) { From c91c40eec9619d2ced3cc1d7570c20b682f749a3 Mon Sep 17 00:00:00 2001 From: Chen Lihui Date: Tue, 13 Oct 2020 09:44:37 +0800 Subject: [PATCH 07/14] Add a test case for wildcards Signed-off-by: Chen Lihui --- rcl_yaml_param_parser/src/parse.c | 15 ++++++----- .../test/test_parse_yaml.cpp | 27 +++++++++++++++++++ rcl_yaml_param_parser/test/wildcards.yaml | 7 +++++ 3 files changed, 43 insertions(+), 6 deletions(-) create mode 100644 rcl_yaml_param_parser/test/wildcards.yaml diff --git a/rcl_yaml_param_parser/src/parse.c b/rcl_yaml_param_parser/src/parse.c index 1936676d5..3ce255672 100644 --- a/rcl_yaml_param_parser/src/parse.c +++ b/rcl_yaml_param_parser/src/parse.c @@ -418,11 +418,11 @@ __validate_namespace(const char * namespace) int validation_result = 0; rmw_ret_t ret; ret = rmw_validate_namespace(namespace, &validation_result, NULL); - if (ret != RMW_RET_OK) { + if (RMW_RET_OK != ret) { RCUTILS_SET_ERROR_MSG(rmw_get_error_string().str); return RCUTILS_RET_ERROR; } - if (validation_result != RMW_NAMESPACE_VALID) { + if (RMW_NAMESPACE_VALID != validation_result) { RCUTILS_SET_ERROR_MSG(rmw_namespace_validation_result_string(validation_result)); return RCUTILS_RET_INVALID_ARGUMENT; } @@ -439,11 +439,11 @@ __validate_nodename(const char * name) int validation_result = 0; rmw_ret_t ret; ret = rmw_validate_node_name(name, &validation_result, NULL); - if (ret != RMW_RET_OK) { + if (RMW_RET_OK != ret) { RCUTILS_SET_ERROR_MSG(rmw_get_error_string().str); return RCUTILS_RET_ERROR; } - if (validation_result != RMW_NODE_NAME_VALID) { + if (RMW_NODE_NAME_VALID != validation_result) { RCUTILS_SET_ERROR_MSG(rmw_node_name_validation_result_string(validation_result)); return RCUTILS_RET_ERROR; } @@ -451,6 +451,9 @@ __validate_nodename(const char * name) return RCUTILS_RET_OK; } +/// +/// TODO(iuhilnehc-ynos): add more extra special rules when the design is determined +/// /// /// Check a name (namespace/node_name) whether it is valid /// @@ -499,12 +502,12 @@ __validate_name(const char * name, rcutils_allocator_t allocator) ret = __validate_namespace(namespace); } - if (ret != RCUTILS_RET_OK) { + if (RCUTILS_RET_OK != ret) { goto clean; } ret = __validate_nodename(node_name); - if (ret != RCUTILS_RET_OK) { + if (RCUTILS_RET_OK != ret) { goto clean; } diff --git a/rcl_yaml_param_parser/test/test_parse_yaml.cpp b/rcl_yaml_param_parser/test/test_parse_yaml.cpp index f53b0194f..ca5c17af9 100644 --- a/rcl_yaml_param_parser/test/test_parse_yaml.cpp +++ b/rcl_yaml_param_parser/test/test_parse_yaml.cpp @@ -558,6 +558,33 @@ TEST(test_file_parser, empty_name_in_ns) { EXPECT_FALSE(res); } +TEST(test_file_parser, wildcards) { + rcutils_reset_error(); + EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); + rcutils_allocator_t allocator = rcutils_get_default_allocator(); + char * test_path = rcutils_join_path(cur_dir, "test", allocator); + ASSERT_TRUE(NULL != test_path) << rcutils_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + allocator.deallocate(test_path, allocator.state); + }); + char * path = rcutils_join_path(test_path, "wildcards.yaml", allocator); + ASSERT_TRUE(NULL != path) << rcutils_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + allocator.deallocate(path, allocator.state); + }); + EXPECT_TRUE(rcutils_exists(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_TRUE(res) << rcutils_get_error_string().str; +} + int32_t main(int32_t argc, char ** argv) { ::testing::InitGoogleTest(&argc, argv); diff --git a/rcl_yaml_param_parser/test/wildcards.yaml b/rcl_yaml_param_parser/test/wildcards.yaml new file mode 100644 index 000000000..fb4e71486 --- /dev/null +++ b/rcl_yaml_param_parser/test/wildcards.yaml @@ -0,0 +1,7 @@ +# config/test_yaml +--- + +/**: + ros__parameters: + id: 10 + name: "front lidar" From 94f127885f9227d10039e01940b4a39653e289f8 Mon Sep 17 00:00:00 2001 From: Chen Lihui Date: Tue, 13 Oct 2020 11:16:17 +0800 Subject: [PATCH 08/14] refactor for __validate_name Signed-off-by: Chen Lihui --- rcl_yaml_param_parser/src/parse.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/rcl_yaml_param_parser/src/parse.c b/rcl_yaml_param_parser/src/parse.c index 3ce255672..99aff76ee 100644 --- a/rcl_yaml_param_parser/src/parse.c +++ b/rcl_yaml_param_parser/src/parse.c @@ -467,7 +467,6 @@ __validate_name(const char * name, rcutils_allocator_t allocator) rcutils_ret_t ret = RCUTILS_RET_OK; char * separator_pos = rindex(name, '/'); - char * namespace = NULL; char * node_name = NULL; char * absolute_namespace = NULL; if (NULL == separator_pos) { @@ -477,6 +476,7 @@ __validate_name(const char * name, rcutils_allocator_t allocator) goto clean; } } else { + char * namespace = NULL; namespace = rcutils_strndup(name, separator_pos - name, allocator); if (NULL == namespace) { ret = RCUTILS_RET_BAD_ALLOC; @@ -484,10 +484,13 @@ __validate_name(const char * name, rcutils_allocator_t allocator) } if (namespace[0] != '/') { absolute_namespace = rcutils_format_string(allocator, "/%s", namespace); + allocator.deallocate(namespace, allocator.state); if (NULL == absolute_namespace) { ret = RCUTILS_RET_BAD_ALLOC; goto clean; } + } else { + absolute_namespace = namespace; } node_name = rcutils_strdup(separator_pos + 1, allocator); if (NULL == node_name) { @@ -498,12 +501,9 @@ __validate_name(const char * name, rcutils_allocator_t allocator) if (absolute_namespace) { ret = __validate_namespace(absolute_namespace); - } else if (namespace) { - ret = __validate_namespace(namespace); - } - - if (RCUTILS_RET_OK != ret) { - goto clean; + if (RCUTILS_RET_OK != ret) { + goto clean; + } } ret = __validate_nodename(node_name); @@ -518,9 +518,6 @@ __validate_name(const char * name, rcutils_allocator_t allocator) if (node_name) { allocator.deallocate(node_name, allocator.state); } - if (namespace) { - allocator.deallocate(namespace, allocator.state); - } return ret; } From 07a92fa2ea4e40ff5b1928489a6547461179a721 Mon Sep 17 00:00:00 2001 From: Chen Lihui Date: Tue, 13 Oct 2020 13:40:02 +0800 Subject: [PATCH 09/14] Add extra special rules for wildcards Signed-off-by: Chen Lihui --- rcl_yaml_param_parser/src/parse.c | 21 ++--- .../test/test_parse_yaml.cpp | 81 +++++++++++++++++++ .../test/wildcards_node.yaml | 8 ++ rcl_yaml_param_parser/test/wildcards_ns.yaml | 8 ++ .../test/wildcards_separated.yaml | 8 ++ 5 files changed, 116 insertions(+), 10 deletions(-) create mode 100644 rcl_yaml_param_parser/test/wildcards_node.yaml create mode 100644 rcl_yaml_param_parser/test/wildcards_ns.yaml create mode 100644 rcl_yaml_param_parser/test/wildcards_separated.yaml diff --git a/rcl_yaml_param_parser/src/parse.c b/rcl_yaml_param_parser/src/parse.c index 99aff76ee..77034a98f 100644 --- a/rcl_yaml_param_parser/src/parse.c +++ b/rcl_yaml_param_parser/src/parse.c @@ -451,9 +451,6 @@ __validate_nodename(const char * name) return RCUTILS_RET_OK; } -/// -/// TODO(iuhilnehc-ynos): add more extra special rules when the design is determined -/// /// /// Check a name (namespace/node_name) whether it is valid /// @@ -461,7 +458,7 @@ static rcutils_ret_t __validate_name(const char * name, rcutils_allocator_t allocator) { // special rules - if (strcmp(name, "/**") == 0) { + if (0 == strcmp(name, "/**")) { return RCUTILS_RET_OK; } @@ -500,15 +497,19 @@ __validate_name(const char * name, rcutils_allocator_t allocator) } if (absolute_namespace) { - ret = __validate_namespace(absolute_namespace); - if (RCUTILS_RET_OK != ret) { - goto clean; + if (0 != strcmp(absolute_namespace, "/**")) { + ret = __validate_namespace(absolute_namespace); + if (RCUTILS_RET_OK != ret) { + goto clean; + } } } - ret = __validate_nodename(node_name); - if (RCUTILS_RET_OK != ret) { - goto clean; + if (0 != strcmp(node_name, "*")) { + ret = __validate_nodename(node_name); + if (RCUTILS_RET_OK != ret) { + goto clean; + } } clean: diff --git a/rcl_yaml_param_parser/test/test_parse_yaml.cpp b/rcl_yaml_param_parser/test/test_parse_yaml.cpp index ca5c17af9..874cceabe 100644 --- a/rcl_yaml_param_parser/test/test_parse_yaml.cpp +++ b/rcl_yaml_param_parser/test/test_parse_yaml.cpp @@ -585,6 +585,87 @@ TEST(test_file_parser, wildcards) { EXPECT_TRUE(res) << rcutils_get_error_string().str; } +TEST(test_file_parser, wildcards_ns) { + rcutils_reset_error(); + EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); + rcutils_allocator_t allocator = rcutils_get_default_allocator(); + char * test_path = rcutils_join_path(cur_dir, "test", allocator); + ASSERT_TRUE(NULL != test_path) << rcutils_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + allocator.deallocate(test_path, allocator.state); + }); + char * path = rcutils_join_path(test_path, "wildcards_ns.yaml", allocator); + ASSERT_TRUE(NULL != path) << rcutils_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + allocator.deallocate(path, allocator.state); + }); + EXPECT_TRUE(rcutils_exists(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_TRUE(res) << rcutils_get_error_string().str; +} + +TEST(test_file_parser, wildcards_node) { + rcutils_reset_error(); + EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); + rcutils_allocator_t allocator = rcutils_get_default_allocator(); + char * test_path = rcutils_join_path(cur_dir, "test", allocator); + ASSERT_TRUE(NULL != test_path) << rcutils_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + allocator.deallocate(test_path, allocator.state); + }); + char * path = rcutils_join_path(test_path, "wildcards_node.yaml", allocator); + ASSERT_TRUE(NULL != path) << rcutils_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + allocator.deallocate(path, allocator.state); + }); + EXPECT_TRUE(rcutils_exists(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_TRUE(res) << rcutils_get_error_string().str; +} + +TEST(test_file_parser, wildcards_separated) { + rcutils_reset_error(); + EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); + rcutils_allocator_t allocator = rcutils_get_default_allocator(); + char * test_path = rcutils_join_path(cur_dir, "test", allocator); + ASSERT_TRUE(NULL != test_path) << rcutils_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + allocator.deallocate(test_path, allocator.state); + }); + char * path = rcutils_join_path(test_path, "wildcards_separated.yaml", allocator); + ASSERT_TRUE(NULL != path) << rcutils_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + allocator.deallocate(path, allocator.state); + }); + EXPECT_TRUE(rcutils_exists(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_TRUE(res) << rcutils_get_error_string().str; +} + int32_t main(int32_t argc, char ** argv) { ::testing::InitGoogleTest(&argc, argv); diff --git a/rcl_yaml_param_parser/test/wildcards_node.yaml b/rcl_yaml_param_parser/test/wildcards_node.yaml new file mode 100644 index 000000000..abd8923e9 --- /dev/null +++ b/rcl_yaml_param_parser/test/wildcards_node.yaml @@ -0,0 +1,8 @@ +# config/test_yaml +--- + +/foo: + "*": + ros__parameters: + id: 10 + name: "front lidar" diff --git a/rcl_yaml_param_parser/test/wildcards_ns.yaml b/rcl_yaml_param_parser/test/wildcards_ns.yaml new file mode 100644 index 000000000..411986b66 --- /dev/null +++ b/rcl_yaml_param_parser/test/wildcards_ns.yaml @@ -0,0 +1,8 @@ +# config/test_yaml +--- + +/**: + some_node: + ros__parameters: + id: 10 + name: "front lidar" diff --git a/rcl_yaml_param_parser/test/wildcards_separated.yaml b/rcl_yaml_param_parser/test/wildcards_separated.yaml new file mode 100644 index 000000000..f254a1230 --- /dev/null +++ b/rcl_yaml_param_parser/test/wildcards_separated.yaml @@ -0,0 +1,8 @@ +# config/test_yaml +--- + +/**: + "*": + ros__parameters: + id: 10 + name: "front lidar" From 97185bc425992b06d1c0491a6c40a0dd28954392 Mon Sep 17 00:00:00 2001 From: Chen Lihui Date: Tue, 13 Oct 2020 16:36:37 +0800 Subject: [PATCH 10/14] Update for /* wildcards Signed-off-by: Chen Lihui --- rcl_yaml_param_parser/src/parse.c | 25 ++++-- .../test/test_parse_yaml.cpp | 81 +++++++++++++++++++ .../test/wildcards_node_slash.yaml | 8 ++ .../test/wildcards_ns_single.yaml | 8 ++ .../test/wildcards_single.yaml | 7 ++ 5 files changed, 121 insertions(+), 8 deletions(-) create mode 100644 rcl_yaml_param_parser/test/wildcards_node_slash.yaml create mode 100644 rcl_yaml_param_parser/test/wildcards_ns_single.yaml create mode 100644 rcl_yaml_param_parser/test/wildcards_single.yaml diff --git a/rcl_yaml_param_parser/src/parse.c b/rcl_yaml_param_parser/src/parse.c index 77034a98f..86e23218a 100644 --- a/rcl_yaml_param_parser/src/parse.c +++ b/rcl_yaml_param_parser/src/parse.c @@ -458,7 +458,7 @@ static rcutils_ret_t __validate_name(const char * name, rcutils_allocator_t allocator) { // special rules - if (0 == strcmp(name, "/**")) { + if (0 == strcmp(name, "/**") || 0 == strcmp(name, "/*")) { return RCUTILS_RET_OK; } @@ -473,8 +473,20 @@ __validate_name(const char * name, rcutils_allocator_t allocator) goto clean; } } else { + node_name = rcutils_strdup(separator_pos + 1, allocator); + if (NULL == node_name) { + ret = RCUTILS_RET_BAD_ALLOC; + goto clean; + } + char * namespace = NULL; - namespace = rcutils_strndup(name, separator_pos - name, allocator); + // Besides "*", node name also support /*. + if (0 == strcmp(node_name, "*") && '/' == name[separator_pos - name]) { + namespace = rcutils_strndup(name, separator_pos - name - 1, allocator); + } else { + namespace = rcutils_strndup(name, separator_pos - name, allocator); + } + if (NULL == namespace) { ret = RCUTILS_RET_BAD_ALLOC; goto clean; @@ -489,15 +501,12 @@ __validate_name(const char * name, rcutils_allocator_t allocator) } else { absolute_namespace = namespace; } - node_name = rcutils_strdup(separator_pos + 1, allocator); - if (NULL == node_name) { - ret = RCUTILS_RET_BAD_ALLOC; - goto clean; - } } if (absolute_namespace) { - if (0 != strcmp(absolute_namespace, "/**")) { + if (0 != strcmp(absolute_namespace, "/**") && + 0 != strcmp(absolute_namespace, "/*")) + { ret = __validate_namespace(absolute_namespace); if (RCUTILS_RET_OK != ret) { goto clean; diff --git a/rcl_yaml_param_parser/test/test_parse_yaml.cpp b/rcl_yaml_param_parser/test/test_parse_yaml.cpp index 874cceabe..597291bf1 100644 --- a/rcl_yaml_param_parser/test/test_parse_yaml.cpp +++ b/rcl_yaml_param_parser/test/test_parse_yaml.cpp @@ -585,6 +585,33 @@ TEST(test_file_parser, wildcards) { EXPECT_TRUE(res) << rcutils_get_error_string().str; } +TEST(test_file_parser, wildcards_single) { + rcutils_reset_error(); + EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); + rcutils_allocator_t allocator = rcutils_get_default_allocator(); + char * test_path = rcutils_join_path(cur_dir, "test", allocator); + ASSERT_TRUE(NULL != test_path) << rcutils_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + allocator.deallocate(test_path, allocator.state); + }); + char * path = rcutils_join_path(test_path, "wildcards_single.yaml", allocator); + ASSERT_TRUE(NULL != path) << rcutils_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + allocator.deallocate(path, allocator.state); + }); + EXPECT_TRUE(rcutils_exists(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_TRUE(res) << rcutils_get_error_string().str; +} + TEST(test_file_parser, wildcards_ns) { rcutils_reset_error(); EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); @@ -612,6 +639,33 @@ TEST(test_file_parser, wildcards_ns) { EXPECT_TRUE(res) << rcutils_get_error_string().str; } +TEST(test_file_parser, wildcards_ns_single) { + rcutils_reset_error(); + EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); + rcutils_allocator_t allocator = rcutils_get_default_allocator(); + char * test_path = rcutils_join_path(cur_dir, "test", allocator); + ASSERT_TRUE(NULL != test_path) << rcutils_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + allocator.deallocate(test_path, allocator.state); + }); + char * path = rcutils_join_path(test_path, "wildcards_ns_single.yaml", allocator); + ASSERT_TRUE(NULL != path) << rcutils_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + allocator.deallocate(path, allocator.state); + }); + EXPECT_TRUE(rcutils_exists(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_TRUE(res) << rcutils_get_error_string().str; +} + TEST(test_file_parser, wildcards_node) { rcutils_reset_error(); EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); @@ -639,6 +693,33 @@ TEST(test_file_parser, wildcards_node) { EXPECT_TRUE(res) << rcutils_get_error_string().str; } +TEST(test_file_parser, wildcards_node_slash) { + rcutils_reset_error(); + EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); + rcutils_allocator_t allocator = rcutils_get_default_allocator(); + char * test_path = rcutils_join_path(cur_dir, "test", allocator); + ASSERT_TRUE(NULL != test_path) << rcutils_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + allocator.deallocate(test_path, allocator.state); + }); + char * path = rcutils_join_path(test_path, "wildcards_node_slash.yaml", allocator); + ASSERT_TRUE(NULL != path) << rcutils_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + allocator.deallocate(path, allocator.state); + }); + EXPECT_TRUE(rcutils_exists(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_TRUE(res) << rcutils_get_error_string().str; +} + TEST(test_file_parser, wildcards_separated) { rcutils_reset_error(); EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); diff --git a/rcl_yaml_param_parser/test/wildcards_node_slash.yaml b/rcl_yaml_param_parser/test/wildcards_node_slash.yaml new file mode 100644 index 000000000..8175f5f50 --- /dev/null +++ b/rcl_yaml_param_parser/test/wildcards_node_slash.yaml @@ -0,0 +1,8 @@ +# config/test_yaml +--- + +/foo: + /*: + ros__parameters: + id: 10 + name: "front lidar" diff --git a/rcl_yaml_param_parser/test/wildcards_ns_single.yaml b/rcl_yaml_param_parser/test/wildcards_ns_single.yaml new file mode 100644 index 000000000..bb3f811c0 --- /dev/null +++ b/rcl_yaml_param_parser/test/wildcards_ns_single.yaml @@ -0,0 +1,8 @@ +# config/test_yaml +--- + +/*: + some_node: + ros__parameters: + id: 10 + name: "front lidar" diff --git a/rcl_yaml_param_parser/test/wildcards_single.yaml b/rcl_yaml_param_parser/test/wildcards_single.yaml new file mode 100644 index 000000000..52edeb85c --- /dev/null +++ b/rcl_yaml_param_parser/test/wildcards_single.yaml @@ -0,0 +1,7 @@ +# config/test_yaml +--- + +/*: + ros__parameters: + id: 10 + name: "front lidar" From 1b2f77833e1cf4923e0d7a5c9cd8323742132570 Mon Sep 17 00:00:00 2001 From: Chen Lihui Date: Tue, 13 Oct 2020 16:44:42 +0800 Subject: [PATCH 11/14] Add more test case Signed-off-by: Chen Lihui --- .../test/test_parse_yaml.cpp | 27 +++++++++++++++++++ .../test/wildcards_separated_slash.yaml | 8 ++++++ 2 files changed, 35 insertions(+) create mode 100644 rcl_yaml_param_parser/test/wildcards_separated_slash.yaml diff --git a/rcl_yaml_param_parser/test/test_parse_yaml.cpp b/rcl_yaml_param_parser/test/test_parse_yaml.cpp index 597291bf1..842f6fa34 100644 --- a/rcl_yaml_param_parser/test/test_parse_yaml.cpp +++ b/rcl_yaml_param_parser/test/test_parse_yaml.cpp @@ -747,6 +747,33 @@ TEST(test_file_parser, wildcards_separated) { EXPECT_TRUE(res) << rcutils_get_error_string().str; } +TEST(test_file_parser, wildcards_separated_slash) { + rcutils_reset_error(); + EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); + rcutils_allocator_t allocator = rcutils_get_default_allocator(); + char * test_path = rcutils_join_path(cur_dir, "test", allocator); + ASSERT_TRUE(NULL != test_path) << rcutils_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + allocator.deallocate(test_path, allocator.state); + }); + char * path = rcutils_join_path(test_path, "wildcards_separated_slash.yaml", allocator); + ASSERT_TRUE(NULL != path) << rcutils_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + allocator.deallocate(path, allocator.state); + }); + EXPECT_TRUE(rcutils_exists(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_TRUE(res) << rcutils_get_error_string().str; +} + int32_t main(int32_t argc, char ** argv) { ::testing::InitGoogleTest(&argc, argv); diff --git a/rcl_yaml_param_parser/test/wildcards_separated_slash.yaml b/rcl_yaml_param_parser/test/wildcards_separated_slash.yaml new file mode 100644 index 000000000..1add948d8 --- /dev/null +++ b/rcl_yaml_param_parser/test/wildcards_separated_slash.yaml @@ -0,0 +1,8 @@ +# config/test_yaml +--- + +/**: + /*: + ros__parameters: + id: 10 + name: "front lidar" From dafd819601cfd84d22e7e70c8feb72371a102de3 Mon Sep 17 00:00:00 2001 From: Chen Lihui Date: Wed, 14 Oct 2020 13:42:44 +0800 Subject: [PATCH 12/14] Update for allowing wildcards at any location and add test cases for partial matches Signed-off-by: Chen Lihui --- rcl_yaml_param_parser/src/parse.c | 55 +++-- .../test/test_parse_yaml.cpp | 188 ++++-------------- rcl_yaml_param_parser/test/wildcards.yaml | 73 +++++++ rcl_yaml_param_parser/test/wildcards_ns.yaml | 8 - ...d_slash.yaml => wildcards_partial_01.yaml} | 3 +- ...parated.yaml => wildcards_partial_02.yaml} | 3 +- ...ds_node.yaml => wildcards_partial_03.yaml} | 3 +- ..._single.yaml => wildcards_partial_04.yaml} | 3 +- .../test/wildcards_partial_05.yaml | 7 + .../test/wildcards_partial_06.yaml | 7 + .../test/wildcards_partial_07.yaml | 7 + .../test/wildcards_partial_08.yaml | 7 + .../test/wildcards_partial_09.yaml | 7 + .../test/wildcards_partial_10.yaml | 7 + .../test/wildcards_partial_11.yaml | 7 + .../test/wildcards_partial_12.yaml | 7 + .../test/wildcards_single.yaml | 7 - 17 files changed, 206 insertions(+), 193 deletions(-) delete mode 100644 rcl_yaml_param_parser/test/wildcards_ns.yaml rename rcl_yaml_param_parser/test/{wildcards_separated_slash.yaml => wildcards_partial_01.yaml} (88%) rename rcl_yaml_param_parser/test/{wildcards_separated.yaml => wildcards_partial_02.yaml} (87%) rename rcl_yaml_param_parser/test/{wildcards_node.yaml => wildcards_partial_03.yaml} (86%) rename rcl_yaml_param_parser/test/{wildcards_ns_single.yaml => wildcards_partial_04.yaml} (83%) create mode 100644 rcl_yaml_param_parser/test/wildcards_partial_05.yaml create mode 100644 rcl_yaml_param_parser/test/wildcards_partial_06.yaml create mode 100644 rcl_yaml_param_parser/test/wildcards_partial_07.yaml create mode 100644 rcl_yaml_param_parser/test/wildcards_partial_08.yaml create mode 100644 rcl_yaml_param_parser/test/wildcards_partial_09.yaml create mode 100644 rcl_yaml_param_parser/test/wildcards_partial_10.yaml create mode 100644 rcl_yaml_param_parser/test/wildcards_partial_11.yaml create mode 100644 rcl_yaml_param_parser/test/wildcards_partial_12.yaml delete mode 100644 rcl_yaml_param_parser/test/wildcards_single.yaml diff --git a/rcl_yaml_param_parser/src/parse.c b/rcl_yaml_param_parser/src/parse.c index 86e23218a..916194621 100644 --- a/rcl_yaml_param_parser/src/parse.c +++ b/rcl_yaml_param_parser/src/parse.c @@ -473,20 +473,8 @@ __validate_name(const char * name, rcutils_allocator_t allocator) goto clean; } } else { - node_name = rcutils_strdup(separator_pos + 1, allocator); - if (NULL == node_name) { - ret = RCUTILS_RET_BAD_ALLOC; - goto clean; - } - - char * namespace = NULL; - // Besides "*", node name also support /*. - if (0 == strcmp(node_name, "*") && '/' == name[separator_pos - name]) { - namespace = rcutils_strndup(name, separator_pos - name - 1, allocator); - } else { - namespace = rcutils_strndup(name, separator_pos - name, allocator); - } - + // substring namespace including the last '/' + char * namespace = rcutils_strndup(name, separator_pos - name + 1, allocator); if (NULL == namespace) { ret = RCUTILS_RET_BAD_ALLOC; goto clean; @@ -501,20 +489,51 @@ __validate_name(const char * name, rcutils_allocator_t allocator) } else { absolute_namespace = namespace; } + + node_name = rcutils_strdup(separator_pos + 1, allocator); + if (NULL == node_name) { + ret = RCUTILS_RET_BAD_ALLOC; + goto clean; + } } if (absolute_namespace) { - if (0 != strcmp(absolute_namespace, "/**") && - 0 != strcmp(absolute_namespace, "/*")) - { + size_t i = 0; + separator_pos = index(absolute_namespace + i + 1, '/'); + if (NULL == separator_pos) { ret = __validate_namespace(absolute_namespace); if (RCUTILS_RET_OK != ret) { goto clean; } + } else { + do { + size_t len = separator_pos - absolute_namespace - i; + char * namespace = rcutils_strndup(absolute_namespace + i, len, allocator); + if (NULL == namespace) { + ret = RCUTILS_RET_BAD_ALLOC; + goto clean; + } + if (0 == strcmp(namespace, "/")) { + RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( + "%s contains repeated forward slash", absolute_namespace); + allocator.deallocate(namespace, allocator.state); + ret = RCUTILS_RET_INVALID_ARGUMENT; + goto clean; + } + if (0 != strcmp(namespace, "/**") && 0 != strcmp(namespace, "/*")) { + ret = __validate_namespace(namespace); + if (RCUTILS_RET_OK != ret) { + allocator.deallocate(namespace, allocator.state); + goto clean; + } + } + allocator.deallocate(namespace, allocator.state); + i += len; + } while (NULL != (separator_pos = index(absolute_namespace + i + 1, '/'))); } } - if (0 != strcmp(node_name, "*")) { + if (0 != strcmp(node_name, "*") && 0 != strcmp(node_name, "**")) { ret = __validate_nodename(node_name); if (RCUTILS_RET_OK != ret) { goto clean; diff --git a/rcl_yaml_param_parser/test/test_parse_yaml.cpp b/rcl_yaml_param_parser/test/test_parse_yaml.cpp index 842f6fa34..67ba2ebaa 100644 --- a/rcl_yaml_param_parser/test/test_parse_yaml.cpp +++ b/rcl_yaml_param_parser/test/test_parse_yaml.cpp @@ -15,6 +15,8 @@ #include #include +#include +#include #include "osrf_testing_tools_cpp/scope_exit.hpp" @@ -585,114 +587,6 @@ TEST(test_file_parser, wildcards) { EXPECT_TRUE(res) << rcutils_get_error_string().str; } -TEST(test_file_parser, wildcards_single) { - rcutils_reset_error(); - EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); - rcutils_allocator_t allocator = rcutils_get_default_allocator(); - char * test_path = rcutils_join_path(cur_dir, "test", allocator); - ASSERT_TRUE(NULL != test_path) << rcutils_get_error_string().str; - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( - { - allocator.deallocate(test_path, allocator.state); - }); - char * path = rcutils_join_path(test_path, "wildcards_single.yaml", allocator); - ASSERT_TRUE(NULL != path) << rcutils_get_error_string().str; - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( - { - allocator.deallocate(path, allocator.state); - }); - EXPECT_TRUE(rcutils_exists(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_TRUE(res) << rcutils_get_error_string().str; -} - -TEST(test_file_parser, wildcards_ns) { - rcutils_reset_error(); - EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); - rcutils_allocator_t allocator = rcutils_get_default_allocator(); - char * test_path = rcutils_join_path(cur_dir, "test", allocator); - ASSERT_TRUE(NULL != test_path) << rcutils_get_error_string().str; - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( - { - allocator.deallocate(test_path, allocator.state); - }); - char * path = rcutils_join_path(test_path, "wildcards_ns.yaml", allocator); - ASSERT_TRUE(NULL != path) << rcutils_get_error_string().str; - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( - { - allocator.deallocate(path, allocator.state); - }); - EXPECT_TRUE(rcutils_exists(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_TRUE(res) << rcutils_get_error_string().str; -} - -TEST(test_file_parser, wildcards_ns_single) { - rcutils_reset_error(); - EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); - rcutils_allocator_t allocator = rcutils_get_default_allocator(); - char * test_path = rcutils_join_path(cur_dir, "test", allocator); - ASSERT_TRUE(NULL != test_path) << rcutils_get_error_string().str; - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( - { - allocator.deallocate(test_path, allocator.state); - }); - char * path = rcutils_join_path(test_path, "wildcards_ns_single.yaml", allocator); - ASSERT_TRUE(NULL != path) << rcutils_get_error_string().str; - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( - { - allocator.deallocate(path, allocator.state); - }); - EXPECT_TRUE(rcutils_exists(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_TRUE(res) << rcutils_get_error_string().str; -} - -TEST(test_file_parser, wildcards_node) { - rcutils_reset_error(); - EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); - rcutils_allocator_t allocator = rcutils_get_default_allocator(); - char * test_path = rcutils_join_path(cur_dir, "test", allocator); - ASSERT_TRUE(NULL != test_path) << rcutils_get_error_string().str; - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( - { - allocator.deallocate(test_path, allocator.state); - }); - char * path = rcutils_join_path(test_path, "wildcards_node.yaml", allocator); - ASSERT_TRUE(NULL != path) << rcutils_get_error_string().str; - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( - { - allocator.deallocate(path, allocator.state); - }); - EXPECT_TRUE(rcutils_exists(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_TRUE(res) << rcutils_get_error_string().str; -} - TEST(test_file_parser, wildcards_node_slash) { rcutils_reset_error(); EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); @@ -717,37 +611,10 @@ TEST(test_file_parser, wildcards_node_slash) { rcl_yaml_node_struct_fini(params_hdl); }); bool res = rcl_parse_yaml_file(path, params_hdl); - EXPECT_TRUE(res) << rcutils_get_error_string().str; -} - -TEST(test_file_parser, wildcards_separated) { - rcutils_reset_error(); - EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); - rcutils_allocator_t allocator = rcutils_get_default_allocator(); - char * test_path = rcutils_join_path(cur_dir, "test", allocator); - ASSERT_TRUE(NULL != test_path) << rcutils_get_error_string().str; - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( - { - allocator.deallocate(test_path, allocator.state); - }); - char * path = rcutils_join_path(test_path, "wildcards_separated.yaml", allocator); - ASSERT_TRUE(NULL != path) << rcutils_get_error_string().str; - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( - { - allocator.deallocate(path, allocator.state); - }); - EXPECT_TRUE(rcutils_exists(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_TRUE(res) << rcutils_get_error_string().str; + EXPECT_FALSE(res); } -TEST(test_file_parser, wildcards_separated_slash) { +TEST(test_file_parser, wildcards_partial) { rcutils_reset_error(); EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); rcutils_allocator_t allocator = rcutils_get_default_allocator(); @@ -757,21 +624,38 @@ TEST(test_file_parser, wildcards_separated_slash) { { allocator.deallocate(test_path, allocator.state); }); - char * path = rcutils_join_path(test_path, "wildcards_separated_slash.yaml", allocator); - ASSERT_TRUE(NULL != path) << rcutils_get_error_string().str; - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( - { - allocator.deallocate(path, allocator.state); - }); - EXPECT_TRUE(rcutils_exists(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_TRUE(res) << rcutils_get_error_string().str; + const std::vector filenames = { + "wildcards_partial_01.yaml", + "wildcards_partial_02.yaml", + "wildcards_partial_03.yaml", + "wildcards_partial_04.yaml", + "wildcards_partial_05.yaml", + "wildcards_partial_06.yaml", + "wildcards_partial_07.yaml", + "wildcards_partial_08.yaml", + "wildcards_partial_09.yaml", + "wildcards_partial_10.yaml", + "wildcards_partial_11.yaml", + "wildcards_partial_12.yaml" + }; + + for (auto & filename : filenames) { + char * path = rcutils_join_path(test_path, filename.c_str(), allocator); + ASSERT_TRUE(NULL != path) << rcutils_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + allocator.deallocate(path, allocator.state); + }); + EXPECT_TRUE(rcutils_exists(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); + } } int32_t main(int32_t argc, char ** argv) diff --git a/rcl_yaml_param_parser/test/wildcards.yaml b/rcl_yaml_param_parser/test/wildcards.yaml index fb4e71486..6507032c4 100644 --- a/rcl_yaml_param_parser/test/wildcards.yaml +++ b/rcl_yaml_param_parser/test/wildcards.yaml @@ -5,3 +5,76 @@ ros__parameters: id: 10 name: "front lidar" +/*: + ros__parameters: + id: 10 + name: "front lidar" +/**/some_node1: + ros__parameters: + id: 10 + name: "front lidar" +/*/some_node2: + ros__parameters: + id: 10 + name: "front lidar" +/**: + some_node3: + ros__parameters: + id: 10 + name: "front lidar" +/*: + some_node4: + ros__parameters: + id: 10 + name: "front lidar" +/foo1/**: + ros__parameters: + id: 10 + name: "front lidar" +/foo2/*: + ros__parameters: + id: 10 + name: "front lidar" +/foo3/**: + some_node5: + ros__parameters: + id: 10 + name: "front lidar" +/foo4/*: + some_node6: + ros__parameters: + id: 10 + name: "front lidar" +/foo5: + "*": + ros__parameters: + id: 10 + name: "front lidar" +/foo6/**/some_node6: + ros__parameters: + id: 10 + name: "front lidar" +/foo7/*/some_node7: + ros__parameters: + id: 10 + name: "front lidar" +/foo8/**/bar: + some_node8: + ros__parameters: + id: 10 + name: "front lidar" +/foo9/*/bar: + some_node9: + ros__parameters: + id: 10 + name: "front lidar" +/foo10/**/bar: + bar1/bar2/**: + ros__parameters: + id: 10 + name: "front lidar" +/foo11/*/bar: + bar1/bar2/*: + ros__parameters: + id: 10 + name: "front lidar" diff --git a/rcl_yaml_param_parser/test/wildcards_ns.yaml b/rcl_yaml_param_parser/test/wildcards_ns.yaml deleted file mode 100644 index 411986b66..000000000 --- a/rcl_yaml_param_parser/test/wildcards_ns.yaml +++ /dev/null @@ -1,8 +0,0 @@ -# config/test_yaml ---- - -/**: - some_node: - ros__parameters: - id: 10 - name: "front lidar" diff --git a/rcl_yaml_param_parser/test/wildcards_separated_slash.yaml b/rcl_yaml_param_parser/test/wildcards_partial_01.yaml similarity index 88% rename from rcl_yaml_param_parser/test/wildcards_separated_slash.yaml rename to rcl_yaml_param_parser/test/wildcards_partial_01.yaml index 1add948d8..129b382ca 100644 --- a/rcl_yaml_param_parser/test/wildcards_separated_slash.yaml +++ b/rcl_yaml_param_parser/test/wildcards_partial_01.yaml @@ -1,8 +1,7 @@ # config/test_yaml --- -/**: - /*: +/foo**: ros__parameters: id: 10 name: "front lidar" diff --git a/rcl_yaml_param_parser/test/wildcards_separated.yaml b/rcl_yaml_param_parser/test/wildcards_partial_02.yaml similarity index 87% rename from rcl_yaml_param_parser/test/wildcards_separated.yaml rename to rcl_yaml_param_parser/test/wildcards_partial_02.yaml index f254a1230..94dc0ebfe 100644 --- a/rcl_yaml_param_parser/test/wildcards_separated.yaml +++ b/rcl_yaml_param_parser/test/wildcards_partial_02.yaml @@ -1,8 +1,7 @@ # config/test_yaml --- -/**: - "*": +/**foo: ros__parameters: id: 10 name: "front lidar" diff --git a/rcl_yaml_param_parser/test/wildcards_node.yaml b/rcl_yaml_param_parser/test/wildcards_partial_03.yaml similarity index 86% rename from rcl_yaml_param_parser/test/wildcards_node.yaml rename to rcl_yaml_param_parser/test/wildcards_partial_03.yaml index abd8923e9..073ca5df4 100644 --- a/rcl_yaml_param_parser/test/wildcards_node.yaml +++ b/rcl_yaml_param_parser/test/wildcards_partial_03.yaml @@ -1,8 +1,7 @@ # config/test_yaml --- -/foo: - "*": +/f**oo: ros__parameters: id: 10 name: "front lidar" diff --git a/rcl_yaml_param_parser/test/wildcards_ns_single.yaml b/rcl_yaml_param_parser/test/wildcards_partial_04.yaml similarity index 83% rename from rcl_yaml_param_parser/test/wildcards_ns_single.yaml rename to rcl_yaml_param_parser/test/wildcards_partial_04.yaml index bb3f811c0..db100762c 100644 --- a/rcl_yaml_param_parser/test/wildcards_ns_single.yaml +++ b/rcl_yaml_param_parser/test/wildcards_partial_04.yaml @@ -1,8 +1,7 @@ # config/test_yaml --- -/*: - some_node: +/foo*: ros__parameters: id: 10 name: "front lidar" diff --git a/rcl_yaml_param_parser/test/wildcards_partial_05.yaml b/rcl_yaml_param_parser/test/wildcards_partial_05.yaml new file mode 100644 index 000000000..63998422d --- /dev/null +++ b/rcl_yaml_param_parser/test/wildcards_partial_05.yaml @@ -0,0 +1,7 @@ +# config/test_yaml +--- + +/*foo: + ros__parameters: + id: 10 + name: "front lidar" diff --git a/rcl_yaml_param_parser/test/wildcards_partial_06.yaml b/rcl_yaml_param_parser/test/wildcards_partial_06.yaml new file mode 100644 index 000000000..785abd55f --- /dev/null +++ b/rcl_yaml_param_parser/test/wildcards_partial_06.yaml @@ -0,0 +1,7 @@ +# config/test_yaml +--- + +/f*oo: + ros__parameters: + id: 10 + name: "front lidar" diff --git a/rcl_yaml_param_parser/test/wildcards_partial_07.yaml b/rcl_yaml_param_parser/test/wildcards_partial_07.yaml new file mode 100644 index 000000000..5833f75fb --- /dev/null +++ b/rcl_yaml_param_parser/test/wildcards_partial_07.yaml @@ -0,0 +1,7 @@ +# config/test_yaml +--- + +/**/node**: + ros__parameters: + id: 10 + name: "front lidar" diff --git a/rcl_yaml_param_parser/test/wildcards_partial_08.yaml b/rcl_yaml_param_parser/test/wildcards_partial_08.yaml new file mode 100644 index 000000000..b8028c5b7 --- /dev/null +++ b/rcl_yaml_param_parser/test/wildcards_partial_08.yaml @@ -0,0 +1,7 @@ +# config/test_yaml +--- + +/**/**node: + ros__parameters: + id: 10 + name: "front lidar" diff --git a/rcl_yaml_param_parser/test/wildcards_partial_09.yaml b/rcl_yaml_param_parser/test/wildcards_partial_09.yaml new file mode 100644 index 000000000..68ab60ba0 --- /dev/null +++ b/rcl_yaml_param_parser/test/wildcards_partial_09.yaml @@ -0,0 +1,7 @@ +# config/test_yaml +--- + +/**/n**ode: + ros__parameters: + id: 10 + name: "front lidar" diff --git a/rcl_yaml_param_parser/test/wildcards_partial_10.yaml b/rcl_yaml_param_parser/test/wildcards_partial_10.yaml new file mode 100644 index 000000000..4acee370e --- /dev/null +++ b/rcl_yaml_param_parser/test/wildcards_partial_10.yaml @@ -0,0 +1,7 @@ +# config/test_yaml +--- + +/**/*node: + ros__parameters: + id: 10 + name: "front lidar" diff --git a/rcl_yaml_param_parser/test/wildcards_partial_11.yaml b/rcl_yaml_param_parser/test/wildcards_partial_11.yaml new file mode 100644 index 000000000..1fda39672 --- /dev/null +++ b/rcl_yaml_param_parser/test/wildcards_partial_11.yaml @@ -0,0 +1,7 @@ +# config/test_yaml +--- + +/**/node*: + ros__parameters: + id: 10 + name: "front lidar" diff --git a/rcl_yaml_param_parser/test/wildcards_partial_12.yaml b/rcl_yaml_param_parser/test/wildcards_partial_12.yaml new file mode 100644 index 000000000..b3bc972dc --- /dev/null +++ b/rcl_yaml_param_parser/test/wildcards_partial_12.yaml @@ -0,0 +1,7 @@ +# config/test_yaml +--- + +/**/n*ode: + ros__parameters: + id: 10 + name: "front lidar" diff --git a/rcl_yaml_param_parser/test/wildcards_single.yaml b/rcl_yaml_param_parser/test/wildcards_single.yaml deleted file mode 100644 index 52edeb85c..000000000 --- a/rcl_yaml_param_parser/test/wildcards_single.yaml +++ /dev/null @@ -1,7 +0,0 @@ -# config/test_yaml ---- - -/*: - ros__parameters: - id: 10 - name: "front lidar" From 72f8dfc6c5003e968a8997d4f447be203cf1b095 Mon Sep 17 00:00:00 2001 From: Chen Lihui Date: Fri, 16 Oct 2020 10:51:44 +0800 Subject: [PATCH 13/14] Fix for ci on arch64 and windows platform Signed-off-by: Chen Lihui --- rcl_yaml_param_parser/CMakeLists.txt | 1 + rcl_yaml_param_parser/src/parse.c | 60 ++++++++++++++++++++-------- 2 files changed, 44 insertions(+), 17 deletions(-) diff --git a/rcl_yaml_param_parser/CMakeLists.txt b/rcl_yaml_param_parser/CMakeLists.txt index dc8ad6dd3..218801b5c 100644 --- a/rcl_yaml_param_parser/CMakeLists.txt +++ b/rcl_yaml_param_parser/CMakeLists.txt @@ -127,6 +127,7 @@ if(BUILD_TESTING) ament_add_gtest(test_parser_multiple_nodes test/test_parser_multiple_nodes.cpp WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}" + TIMEOUT 240 # Large timeout to wait for fault injection tests ) if(TARGET test_parser_multiple_nodes) ament_target_dependencies(test_parser_multiple_nodes diff --git a/rcl_yaml_param_parser/src/parse.c b/rcl_yaml_param_parser/src/parse.c index 916194621..c04fa8509 100644 --- a/rcl_yaml_param_parser/src/parse.c +++ b/rcl_yaml_param_parser/src/parse.c @@ -31,6 +31,41 @@ #include "./impl/node_params.h" #include "rcl_yaml_param_parser/parser.h" +/// +/// Check a name space whether it is valid +/// +/// \param[in] namespace the namespace to check +/// \return RCUTILS_RET_OK if namespace is valid, or +/// \return RCUTILS_RET_INVALID_ARGUMENT if namespace is not valid, or +/// \return RCUTILS_RET_ERROR if an unspecified error occurred. +RCL_YAML_PARAM_PARSER_LOCAL +rcutils_ret_t +__validate_namespace(const char * namespace); + +/// +/// Check a node name whether it is valid +/// +/// \param[in] name the node name to check +/// \return RCUTILS_RET_OK if the node name is valid, or +/// \return RCUTILS_RET_INVALID_ARGUMENT if node name is not valid, or +/// \return RCUTILS_RET_ERROR if an unspecified error occurred. +RCL_YAML_PARAM_PARSER_LOCAL +rcutils_ret_t +__validate_nodename(const char * node_name); + +/// +/// Check a name (namespace/node_name) whether it is valid +/// +/// \param name the name to check +/// \param allocator an allocator to use +/// \return RCUTILS_RET_OK if name is valid, or +/// \return RCUTILS_RET_INVALID_ARGUMENT if name is not valid, or +/// \return RCL_RET_BAD_ALLOC if an allocation failed, or +/// \return RCUTILS_RET_ERROR if an unspecified error occurred. +RCL_YAML_PARAM_PARSER_LOCAL +rcutils_ret_t +__validate_name(const char * name, rcutils_allocator_t allocator); + /// /// Determine the type of the value and return the converted value /// NOTE: Only canonical forms supported as of now @@ -409,10 +444,7 @@ rcutils_ret_t parse_value( return ret; } -/// -/// Check a name space whether it is valid -/// -static rcutils_ret_t +rcutils_ret_t __validate_namespace(const char * namespace) { int validation_result = 0; @@ -430,31 +462,25 @@ __validate_namespace(const char * namespace) return RCUTILS_RET_OK; } -/// -/// Check a node name whether it is valid -/// -static rcutils_ret_t -__validate_nodename(const char * name) +rcutils_ret_t +__validate_nodename(const char * node_name) { int validation_result = 0; rmw_ret_t ret; - ret = rmw_validate_node_name(name, &validation_result, NULL); + ret = rmw_validate_node_name(node_name, &validation_result, NULL); if (RMW_RET_OK != ret) { RCUTILS_SET_ERROR_MSG(rmw_get_error_string().str); return RCUTILS_RET_ERROR; } if (RMW_NODE_NAME_VALID != validation_result) { RCUTILS_SET_ERROR_MSG(rmw_node_name_validation_result_string(validation_result)); - return RCUTILS_RET_ERROR; + return RCUTILS_RET_INVALID_ARGUMENT; } return RCUTILS_RET_OK; } -/// -/// Check a name (namespace/node_name) whether it is valid -/// -static rcutils_ret_t +rcutils_ret_t __validate_name(const char * name, rcutils_allocator_t allocator) { // special rules @@ -463,7 +489,7 @@ __validate_name(const char * name, rcutils_allocator_t allocator) } rcutils_ret_t ret = RCUTILS_RET_OK; - char * separator_pos = rindex(name, '/'); + char * separator_pos = strrchr(name, '/'); char * node_name = NULL; char * absolute_namespace = NULL; if (NULL == separator_pos) { @@ -499,7 +525,7 @@ __validate_name(const char * name, rcutils_allocator_t allocator) if (absolute_namespace) { size_t i = 0; - separator_pos = index(absolute_namespace + i + 1, '/'); + separator_pos = strchr(absolute_namespace + i + 1, '/'); if (NULL == separator_pos) { ret = __validate_namespace(absolute_namespace); if (RCUTILS_RET_OK != ret) { From dda7f86666afceeecf84b3a1a07c97a1fd7e6065 Mon Sep 17 00:00:00 2001 From: Chen Lihui Date: Sat, 17 Oct 2020 12:44:19 +0800 Subject: [PATCH 14/14] Fix for ci on macOS and windows platform Co-authored-by: Jacob Perron Signed-off-by: Chen Lihui --- rcl_yaml_param_parser/src/parse.c | 51 ++++++++++++++++--------------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/rcl_yaml_param_parser/src/parse.c b/rcl_yaml_param_parser/src/parse.c index c04fa8509..c7cd69c7f 100644 --- a/rcl_yaml_param_parser/src/parse.c +++ b/rcl_yaml_param_parser/src/parse.c @@ -30,6 +30,7 @@ #include "./impl/namespace.h" #include "./impl/node_params.h" #include "rcl_yaml_param_parser/parser.h" +#include "rcl_yaml_param_parser/visibility_control.h" /// /// Check a name space whether it is valid @@ -40,7 +41,7 @@ /// \return RCUTILS_RET_ERROR if an unspecified error occurred. RCL_YAML_PARAM_PARSER_LOCAL rcutils_ret_t -__validate_namespace(const char * namespace); +_validate_namespace(const char * namespace_); /// /// Check a node name whether it is valid @@ -51,7 +52,7 @@ __validate_namespace(const char * namespace); /// \return RCUTILS_RET_ERROR if an unspecified error occurred. RCL_YAML_PARAM_PARSER_LOCAL rcutils_ret_t -__validate_nodename(const char * node_name); +_validate_nodename(const char * node_name); /// /// Check a name (namespace/node_name) whether it is valid @@ -64,7 +65,7 @@ __validate_nodename(const char * node_name); /// \return RCUTILS_RET_ERROR if an unspecified error occurred. RCL_YAML_PARAM_PARSER_LOCAL rcutils_ret_t -__validate_name(const char * name, rcutils_allocator_t allocator); +_validate_name(const char * name, rcutils_allocator_t allocator); /// /// Determine the type of the value and return the converted value @@ -445,11 +446,11 @@ rcutils_ret_t parse_value( } rcutils_ret_t -__validate_namespace(const char * namespace) +_validate_namespace(const char * namespace_) { int validation_result = 0; rmw_ret_t ret; - ret = rmw_validate_namespace(namespace, &validation_result, NULL); + ret = rmw_validate_namespace(namespace_, &validation_result, NULL); if (RMW_RET_OK != ret) { RCUTILS_SET_ERROR_MSG(rmw_get_error_string().str); return RCUTILS_RET_ERROR; @@ -463,7 +464,7 @@ __validate_namespace(const char * namespace) } rcutils_ret_t -__validate_nodename(const char * node_name) +_validate_nodename(const char * node_name) { int validation_result = 0; rmw_ret_t ret; @@ -481,7 +482,7 @@ __validate_nodename(const char * node_name) } rcutils_ret_t -__validate_name(const char * name, rcutils_allocator_t allocator) +_validate_name(const char * name, rcutils_allocator_t allocator) { // special rules if (0 == strcmp(name, "/**") || 0 == strcmp(name, "/*")) { @@ -500,20 +501,20 @@ __validate_name(const char * name, rcutils_allocator_t allocator) } } else { // substring namespace including the last '/' - char * namespace = rcutils_strndup(name, separator_pos - name + 1, allocator); - if (NULL == namespace) { + char * namespace_ = rcutils_strndup(name, separator_pos - name + 1, allocator); + if (NULL == namespace_) { ret = RCUTILS_RET_BAD_ALLOC; goto clean; } - if (namespace[0] != '/') { - absolute_namespace = rcutils_format_string(allocator, "/%s", namespace); - allocator.deallocate(namespace, allocator.state); + if (namespace_[0] != '/') { + absolute_namespace = rcutils_format_string(allocator, "/%s", namespace_); + allocator.deallocate(namespace_, allocator.state); if (NULL == absolute_namespace) { ret = RCUTILS_RET_BAD_ALLOC; goto clean; } } else { - absolute_namespace = namespace; + absolute_namespace = namespace_; } node_name = rcutils_strdup(separator_pos + 1, allocator); @@ -527,40 +528,40 @@ __validate_name(const char * name, rcutils_allocator_t allocator) size_t i = 0; separator_pos = strchr(absolute_namespace + i + 1, '/'); if (NULL == separator_pos) { - ret = __validate_namespace(absolute_namespace); + ret = _validate_namespace(absolute_namespace); if (RCUTILS_RET_OK != ret) { goto clean; } } else { do { size_t len = separator_pos - absolute_namespace - i; - char * namespace = rcutils_strndup(absolute_namespace + i, len, allocator); - if (NULL == namespace) { + char * namespace_ = rcutils_strndup(absolute_namespace + i, len, allocator); + if (NULL == namespace_) { ret = RCUTILS_RET_BAD_ALLOC; goto clean; } - if (0 == strcmp(namespace, "/")) { + if (0 == strcmp(namespace_, "/")) { RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( "%s contains repeated forward slash", absolute_namespace); - allocator.deallocate(namespace, allocator.state); + allocator.deallocate(namespace_, allocator.state); ret = RCUTILS_RET_INVALID_ARGUMENT; goto clean; } - if (0 != strcmp(namespace, "/**") && 0 != strcmp(namespace, "/*")) { - ret = __validate_namespace(namespace); + if (0 != strcmp(namespace_, "/**") && 0 != strcmp(namespace_, "/*")) { + ret = _validate_namespace(namespace_); if (RCUTILS_RET_OK != ret) { - allocator.deallocate(namespace, allocator.state); + allocator.deallocate(namespace_, allocator.state); goto clean; } } - allocator.deallocate(namespace, allocator.state); + allocator.deallocate(namespace_, allocator.state); i += len; - } while (NULL != (separator_pos = index(absolute_namespace + i + 1, '/'))); + } while (NULL != (separator_pos = strchr(absolute_namespace + i + 1, '/'))); } } if (0 != strcmp(node_name, "*") && 0 != strcmp(node_name, "**")) { - ret = __validate_nodename(node_name); + ret = _validate_nodename(node_name); if (RCUTILS_RET_OK != ret) { goto clean; } @@ -638,7 +639,7 @@ rcutils_ret_t parse_key( break; } - ret = __validate_name(node_name_ns, allocator); + ret = _validate_name(node_name_ns, allocator); if (RCUTILS_RET_OK != ret) { allocator.deallocate(node_name_ns, allocator.state); break;