From 409f636fe50466dd093257a537fbb61ad2fbd409 Mon Sep 17 00:00:00 2001 From: Ernestas Date: Wed, 7 Oct 2020 11:38:16 +0300 Subject: [PATCH] backport of improved error message for malformed yaml Signed-off-by: Ernestas --- rcl/src/rcl/arguments.c | 24 ++++++++++++++----- rcl/test/rcl/test_arguments.cpp | 18 ++++++++++++++ .../test_malformed_parameters.1.yaml | 5 ++++ 3 files changed, 41 insertions(+), 6 deletions(-) create mode 100644 rcl/test/resources/test_arguments/test_malformed_parameters.1.yaml diff --git a/rcl/src/rcl/arguments.c b/rcl/src/rcl/arguments.c index cc2b46fc5..20e9b1ec2 100644 --- a/rcl/src/rcl/arguments.c +++ b/rcl/src/rcl/arguments.c @@ -372,11 +372,10 @@ rcl_parse_arguments( if (i + 1 < argc) { // Attempt to parse next argument as parameter file rule args_impl->parameter_files[args_impl->num_param_files_args] = NULL; - if ( - RCL_RET_OK == _rcl_parse_param_file( - argv[i + 1], allocator, args_impl->parameter_overrides, - &args_impl->parameter_files[args_impl->num_param_files_args])) - { + ret = _rcl_parse_param_file( + argv[i + 1], allocator, args_impl->parameter_overrides, + &args_impl->parameter_files[args_impl->num_param_files_args]); + if (RCL_RET_OK == ret) { ++(args_impl->num_param_files_args); RCUTILS_LOG_DEBUG_NAMED( ROS_PACKAGE_NAME, @@ -386,6 +385,19 @@ rcl_parse_arguments( ++i; // Skip flag here, for loop will skip rule. continue; } + if (RCL_RET_ERROR == ret) { + // If _rcl_parse_param_file() returned + // RCL_RET_ERROR then the argument contained the + // '__params:=' prefix, but parsing the parameter + // file failed without any notice. + rcl_error_string_t prev_error_string = rcl_get_error_string(); + rcl_reset_error(); + RCL_SET_ERROR_MSG_WITH_FORMAT_STRING( + "Couldn't parse params file: '%s'. Error: %s", + args_impl->parameter_files[args_impl->num_param_files_args], + prev_error_string.str); + goto fail; + } rcl_error_string_t prev_error_string = rcl_get_error_string(); rcl_reset_error(); RCL_SET_ERROR_MSG_WITH_FORMAT_STRING( @@ -606,7 +618,7 @@ rcl_parse_arguments( } // Drop parameter overrides if none was found. - if (0U == args_impl->parameter_overrides->num_nodes) { + if (NULL != args_impl->parameter_overrides && 0U == args_impl->parameter_overrides->num_nodes) { rcl_yaml_node_struct_fini(args_impl->parameter_overrides); args_impl->parameter_overrides = NULL; } diff --git a/rcl/test/rcl/test_arguments.cpp b/rcl/test/rcl/test_arguments.cpp index 5636a5486..2bc8e479a 100644 --- a/rcl/test/rcl/test_arguments.cpp +++ b/rcl/test/rcl/test_arguments.cpp @@ -935,3 +935,21 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_param_overrides ASSERT_TRUE(NULL != param_value->string_value); EXPECT_STREQ("foo", param_value->string_value); } + +// Regression test for https://github.com/ros2/rcl/issues/815 +// Testing behaviour that was broken in Foxy +TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_malformed_param_file) { + const std::string parameters_filepath = (test_path / "test_malformed_parameters.1.yaml").string(); + const std::string parameter_rule = "__params:=" + parameters_filepath; + const char * argv[] = { + "process_name", "--ros-args", "--params-file", parameter_rule.c_str() + }; + int argc = sizeof(argv) / sizeof(const char *); + rcl_ret_t ret; + + rcl_allocator_t alloc = rcl_get_default_allocator(); + rcl_arguments_t parsed_args = rcl_get_zero_initialized_arguments(); + + ret = rcl_parse_arguments(argc, argv, alloc, &parsed_args); + ASSERT_EQ(RCL_RET_ERROR, ret) << rcl_get_error_string().str; +} diff --git a/rcl/test/resources/test_arguments/test_malformed_parameters.1.yaml b/rcl/test/resources/test_arguments/test_malformed_parameters.1.yaml new file mode 100644 index 000000000..5871824ad --- /dev/null +++ b/rcl/test/resources/test_arguments/test_malformed_parameters.1.yaml @@ -0,0 +1,5 @@ +some_node: + ros_parameters: # must have double _ + int_param: 1 + param_group: + string_param: foo \ No newline at end of file