From 08b8b2f735e41ccdfe5cd8f9f10bb333e94d9449 Mon Sep 17 00:00:00 2001 From: Lei Liu Date: Thu, 12 Jan 2023 10:35:49 +0800 Subject: [PATCH 1/4] Add yaml tag full support. Signed-off-by: Lei Liu --- rcl_yaml_param_parser/src/parse.c | 380 +++++++++++++----- .../test/correct_config.yaml | 26 ++ .../test/test_parse_yaml.cpp | 76 ++++ 3 files changed, 386 insertions(+), 96 deletions(-) diff --git a/rcl_yaml_param_parser/src/parse.c b/rcl_yaml_param_parser/src/parse.c index 967d437d2..3eb2c5da8 100644 --- a/rcl_yaml_param_parser/src/parse.c +++ b/rcl_yaml_param_parser/src/parse.c @@ -67,6 +67,65 @@ RCL_YAML_PARAM_PARSER_LOCAL rcutils_ret_t _validate_name(const char * name, rcutils_allocator_t allocator); +/// +/// Check a tag whether it is valid +/// +/// \param tag the tag to check +/// \param line_num the line number error happened +/// \return RCUTILS_RET_OK if tag is valid, or +/// \return RCUTILS_RET_ERROR if tag is not valid +RCL_YAML_PARAM_PARSER_LOCAL +rcutils_ret_t +_validate_tag(const char * tag, uint32_t line_num); + +/// +/// Check a bool value whether it is valid +/// +/// \param value the bool value to check +/// \param val_type the value type +/// \param ret_val the converted value when value is valid +/// \return RCUTILS_RET_OK if value is valid, or +/// \return RCUTILS_RET_ERROR if value is not valid +RCL_YAML_PARAM_PARSER_LOCAL +rcutils_ret_t +_validate_bool_value( + const char * const value, + data_types_t * val_type, + void ** ret_val, + const rcutils_allocator_t allocator); + +/// +/// Check a int value whether it is valid +/// +/// \param value the int value to check +/// \param val_type the value type +/// \param ret_val the converted value when value is valid +/// \return RCUTILS_RET_OK if value is valid, or +/// \return RCUTILS_RET_ERROR if value is not valid +RCL_YAML_PARAM_PARSER_LOCAL +rcutils_ret_t +_validate_int_value( + const char * const value, + data_types_t * val_type, + void ** ret_val, + const rcutils_allocator_t allocator); + +/// +/// Check a float value whether it is valid +/// +/// \param value the float value to check +/// \param val_type the value type +/// \param ret_val the converted value when value is valid +/// \return RCUTILS_RET_OK if value is valid, or +/// \return RCUTILS_RET_ERROR if value is not valid +RCL_YAML_PARAM_PARSER_LOCAL +rcutils_ret_t +_validate_float_value( + const char * const value, + data_types_t * val_type, + void ** ret_val, + const rcutils_allocator_t allocator); + /// /// Determine the type of the value and return the converted value /// NOTE: Only canonical forms supported as of now @@ -78,10 +137,7 @@ void * get_value( data_types_t * val_type, const rcutils_allocator_t allocator) { - void * ret_val; - int64_t ival; - double dval; - char * endptr = NULL; + void * ret_val = NULL; RCUTILS_CHECK_ARGUMENT_FOR_NULL(value, NULL); RCUTILS_CHECK_ARGUMENT_FOR_NULL(val_type, NULL); @@ -94,49 +150,51 @@ void * get_value( return rcutils_strdup(value, allocator); } - /// Check if it is bool - if (style != YAML_SINGLE_QUOTED_SCALAR_STYLE && - style != YAML_DOUBLE_QUOTED_SCALAR_STYLE) - { - if ((0 == strcmp(value, "Y")) || - (0 == strcmp(value, "y")) || - (0 == strcmp(value, "yes")) || - (0 == strcmp(value, "Yes")) || - (0 == strcmp(value, "YES")) || - (0 == strcmp(value, "true")) || - (0 == strcmp(value, "True")) || - (0 == strcmp(value, "TRUE")) || - (0 == strcmp(value, "on")) || - (0 == strcmp(value, "On")) || - (0 == strcmp(value, "ON"))) + /// Check for yaml null tag + if (tag != NULL && strcmp(YAML_NULL_TAG, (char *)tag) == 0) { + *val_type = DATA_TYPE_STRING; + if ((0 == strcmp(value, "null")) || + (0 == strcmp(value, "Null")) || + (0 == strcmp(value, "none")) || + (0 == strcmp(value, "None"))) { - *val_type = DATA_TYPE_BOOL; - ret_val = allocator.zero_allocate(1U, sizeof(bool), allocator.state); - if (NULL == ret_val) { - return NULL; - } - *((bool *)ret_val) = true; + return rcutils_strdup("null", allocator); + } + return NULL; + } + + /// Check for yaml bool tag + if (tag != NULL && strcmp(YAML_BOOL_TAG, (char *)tag) == 0) { + if (_validate_bool_value(value, val_type, &ret_val, allocator) == RCUTILS_RET_OK) { return ret_val; + } else { + return NULL; } + } - if ((0 == strcmp(value, "N")) || - (0 == strcmp(value, "n")) || - (0 == strcmp(value, "no")) || - (0 == strcmp(value, "No")) || - (0 == strcmp(value, "NO")) || - (0 == strcmp(value, "false")) || - (0 == strcmp(value, "False")) || - (0 == strcmp(value, "FALSE")) || - (0 == strcmp(value, "off")) || - (0 == strcmp(value, "Off")) || - (0 == strcmp(value, "OFF"))) - { - *val_type = DATA_TYPE_BOOL; - ret_val = allocator.zero_allocate(1U, sizeof(bool), allocator.state); - if (NULL == ret_val) { - return NULL; - } - *((bool *)ret_val) = false; + /// Check for yaml int tag + if (tag != NULL && strcmp(YAML_INT_TAG, (char *)tag) == 0) { + if (_validate_int_value(value, val_type, &ret_val, allocator) == RCUTILS_RET_OK) { + return ret_val; + } else { + return NULL; + } + } + + /// Check for yaml float tag + if (tag != NULL && strcmp(YAML_FLOAT_TAG, (char *)tag) == 0) { + if (_validate_float_value(value, val_type, &ret_val, allocator) == RCUTILS_RET_OK) { + return ret_val; + } else { + return NULL; + } + } + + /// Check if it is bool + if (style != YAML_SINGLE_QUOTED_SCALAR_STYLE && + style != YAML_DOUBLE_QUOTED_SCALAR_STYLE) + { + if (_validate_bool_value(value, val_type, &ret_val, allocator) == RCUTILS_RET_OK) { return ret_val; } } @@ -145,20 +203,8 @@ void * get_value( if (style != YAML_SINGLE_QUOTED_SCALAR_STYLE && style != YAML_DOUBLE_QUOTED_SCALAR_STYLE) { - errno = 0; - ival = strtol(value, &endptr, 0); - if ((0 == errno) && (NULL != endptr)) { - if ((NULL != endptr) && (endptr != value)) { - if (('\0' != *value) && ('\0' == *endptr)) { - *val_type = DATA_TYPE_INT64; - ret_val = allocator.zero_allocate(1U, sizeof(int64_t), allocator.state); - if (NULL == ret_val) { - return NULL; - } - *((int64_t *)ret_val) = ival; - return ret_val; - } - } + if (_validate_int_value(value, val_type, &ret_val, allocator) == RCUTILS_RET_OK) { + return ret_val; } } @@ -166,46 +212,9 @@ void * get_value( if (style != YAML_SINGLE_QUOTED_SCALAR_STYLE && style != YAML_DOUBLE_QUOTED_SCALAR_STYLE) { - errno = 0; - endptr = NULL; - const char * iter_ptr = NULL; - if ((0 == strcmp(value, ".nan")) || - (0 == strcmp(value, ".NaN")) || - (0 == strcmp(value, ".NAN")) || - (0 == strcmp(value, ".inf")) || - (0 == strcmp(value, ".Inf")) || - (0 == strcmp(value, ".INF")) || - (0 == strcmp(value, "+.inf")) || - (0 == strcmp(value, "+.Inf")) || - (0 == strcmp(value, "+.INF")) || - (0 == strcmp(value, "-.inf")) || - (0 == strcmp(value, "-.Inf")) || - (0 == strcmp(value, "-.INF"))) - { - for (iter_ptr = value; !isalpha(*iter_ptr); ) { - iter_ptr += 1; - } - dval = strtod(iter_ptr, &endptr); - if (*value == '-') { - dval = -dval; - } - } else { - dval = strtod(value, &endptr); - } - if ((0 == errno) && (NULL != endptr)) { - if ((NULL != endptr) && (endptr != value)) { - if (('\0' != *value) && ('\0' == *endptr)) { - *val_type = DATA_TYPE_DOUBLE; - ret_val = allocator.zero_allocate(1U, sizeof(double), allocator.state); - if (NULL == ret_val) { - return NULL; - } - *((double *)ret_val) = dval; - return ret_val; - } - } + if (_validate_float_value(value, val_type, &ret_val, allocator) == RCUTILS_RET_OK) { + return ret_val; } - errno = 0; } /// It is a string @@ -585,6 +594,166 @@ _validate_name(const char * name, rcutils_allocator_t allocator) return ret; } +rcutils_ret_t +_validate_tag(const char * tag, uint32_t line_num) +{ + if (tag == NULL) { + return RCUTILS_RET_ERROR; + } + + if ((0 == strcmp(tag, YAML_NULL_TAG)) || + (0 == strcmp(tag, YAML_BOOL_TAG)) || + (0 == strcmp(tag, YAML_STR_TAG)) || + (0 == strcmp(tag, YAML_INT_TAG)) || + (0 == strcmp(tag, YAML_FLOAT_TAG)) || + (0 == strcmp(tag, YAML_TIMESTAMP_TAG)) || + (0 == strcmp(tag, YAML_SEQ_TAG)) || + (0 == strcmp(tag, YAML_MAP_TAG))) + { + return RCUTILS_RET_OK; + } + + RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( + "Unsupported tag %s at line %d", (char *)tag, line_num); + return RCUTILS_RET_ERROR; +} + +rcutils_ret_t +_validate_bool_value( + const char * const value, + data_types_t * val_type, + void ** ret_val, + const rcutils_allocator_t allocator) +{ + rcutils_ret_t ret = RCUTILS_RET_ERROR; + + if ((0 == strcmp(value, "Y")) || + (0 == strcmp(value, "y")) || + (0 == strcmp(value, "yes")) || + (0 == strcmp(value, "Yes")) || + (0 == strcmp(value, "YES")) || + (0 == strcmp(value, "true")) || + (0 == strcmp(value, "True")) || + (0 == strcmp(value, "TRUE")) || + (0 == strcmp(value, "on")) || + (0 == strcmp(value, "On")) || + (0 == strcmp(value, "ON"))) + { + *val_type = DATA_TYPE_BOOL; + *ret_val = allocator.zero_allocate(1U, sizeof(bool), allocator.state); + if (NULL != *ret_val) { + *((bool *)*ret_val) = true; + } + ret = RCUTILS_RET_OK; + goto final; + } + + if ((0 == strcmp(value, "N")) || + (0 == strcmp(value, "n")) || + (0 == strcmp(value, "no")) || + (0 == strcmp(value, "No")) || + (0 == strcmp(value, "NO")) || + (0 == strcmp(value, "false")) || + (0 == strcmp(value, "False")) || + (0 == strcmp(value, "FALSE")) || + (0 == strcmp(value, "off")) || + (0 == strcmp(value, "Off")) || + (0 == strcmp(value, "OFF"))) + { + *val_type = DATA_TYPE_BOOL; + *ret_val = allocator.zero_allocate(1U, sizeof(bool), allocator.state); + if (NULL != *ret_val) { + *((bool *)*ret_val) = false; + } + ret = RCUTILS_RET_OK; + goto final; + } + +final: + return ret; +} + +rcutils_ret_t +_validate_int_value( + const char * const value, + data_types_t * val_type, + void ** ret_val, + const rcutils_allocator_t allocator) +{ + errno = 0; + int64_t ival; + char * endptr = NULL; + rcutils_ret_t ret = RCUTILS_RET_ERROR; + + ival = strtol(value, &endptr, 0); + if ((0 == errno) && (NULL != endptr)) { + if ((NULL != endptr) && (endptr != value)) { + if (('\0' != *value) && ('\0' == *endptr)) { + *val_type = DATA_TYPE_INT64; + *ret_val = allocator.zero_allocate(1U, sizeof(int64_t), allocator.state); + if (NULL != *ret_val) { + *((int64_t *)*ret_val) = ival; + } + ret = RCUTILS_RET_OK; + } + } + } + + return ret; +} + +rcutils_ret_t +_validate_float_value( + const char * const value, + data_types_t * val_type, + void ** ret_val, + const rcutils_allocator_t allocator) +{ + errno = 0; + double dval; + char * endptr = NULL; + const char * iter_ptr = NULL; + rcutils_ret_t ret = RCUTILS_RET_ERROR; + + if ((0 == strcmp(value, ".nan")) || + (0 == strcmp(value, ".NaN")) || + (0 == strcmp(value, ".NAN")) || + (0 == strcmp(value, ".inf")) || + (0 == strcmp(value, ".Inf")) || + (0 == strcmp(value, ".INF")) || + (0 == strcmp(value, "+.inf")) || + (0 == strcmp(value, "+.Inf")) || + (0 == strcmp(value, "+.INF")) || + (0 == strcmp(value, "-.inf")) || + (0 == strcmp(value, "-.Inf")) || + (0 == strcmp(value, "-.INF"))) + { + for (iter_ptr = value; !isalpha(*iter_ptr); ) { + iter_ptr += 1; + } + dval = strtod(iter_ptr, &endptr); + if (*value == '-') { + dval = -dval; + } + } else { + dval = strtod(value, &endptr); + } + if ((0 == errno) && (NULL != endptr)) { + if ((NULL != endptr) && (endptr != value)) { + if (('\0' != *value) && ('\0' == *endptr)) { + *val_type = DATA_TYPE_DOUBLE; + *ret_val = allocator.zero_allocate(1U, sizeof(double), allocator.state); + if (NULL != *ret_val) { + *((double *)*ret_val) = dval; + } + ret = RCUTILS_RET_OK; + } + } + } + + return ret; +} + /// /// Parse the key part of the pair /// @@ -780,6 +949,13 @@ rcutils_ret_t parse_file_events( break; } line_num = ((uint32_t)(event.start_mark.line) + 1U); + const yaml_char_t * const tag = event.data.scalar.tag; + if (tag != NULL) { + ret = _validate_tag((char *)tag, line_num); + if (RCUTILS_RET_OK != ret) { + break; + } + } switch (event.type) { case YAML_STREAM_END_EVENT: done_parsing = 1; @@ -826,6 +1002,12 @@ rcutils_ret_t parse_file_events( } break; case YAML_SEQUENCE_START_EVENT: + if (tag != NULL && strcmp(YAML_SEQ_TAG, (char *)tag) != 0) { + RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( + "Sequences cannot be used with tag %s at line %d", (char *)tag, line_num); + ret = RCUTILS_RET_ERROR; + break; + } if (is_key) { RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( "Sequences cannot be key at line %d", line_num); @@ -846,6 +1028,12 @@ rcutils_ret_t parse_file_events( is_key = true; break; case YAML_MAPPING_START_EVENT: + if (tag != NULL && strcmp(YAML_MAP_TAG, (char *)tag) != 0) { + RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( + "Maps cannot be used with tag %s at line %d", (char *)tag, line_num); + ret = RCUTILS_RET_ERROR; + break; + } map_depth++; is_new_map = true; is_key = true; diff --git a/rcl_yaml_param_parser/test/correct_config.yaml b/rcl_yaml_param_parser/test/correct_config.yaml index 502b3e3d9..54661364d 100644 --- a/rcl_yaml_param_parser/test/correct_config.yaml +++ b/rcl_yaml_param_parser/test/correct_config.yaml @@ -54,3 +54,29 @@ string_tag: string_bool: !!str yes string_int: !!str 1234 string_double: !!str 12.34 +bool_tag: + ros__parameters: + bool_true: !!bool true + bool_yes: !!bool yes + bool_on: !!bool on + bool_false: !!bool false + bool_no: !!bool no + bool_off: !!bool off +null_tag: + ros__parameters: + null_null: !!null null + null_none: !!null none +int_tag: + ros__parameters: + int_pos: !!int 10 + int_zero: !!int 0 + int_neg: !!int -10 +float_tag: + ros__parameters: + float_pos: !!float 1.1 + float_zero: !!float 0.0 + float_neg: !!float -1.122 + float_nan: !!float .nan + float_inf: !!float .inf + float_infn: !!float +.inf + float_infp: !!float -.inf diff --git a/rcl_yaml_param_parser/test/test_parse_yaml.cpp b/rcl_yaml_param_parser/test/test_parse_yaml.cpp index 9a357c7c2..85ce3809c 100644 --- a/rcl_yaml_param_parser/test/test_parse_yaml.cpp +++ b/rcl_yaml_param_parser/test/test_parse_yaml.cpp @@ -207,6 +207,82 @@ TEST(test_parser, correct_syntax) { ASSERT_TRUE(NULL != param_value->string_value); EXPECT_STREQ("12.34", param_value->string_value); + param_value = rcl_yaml_node_struct_get("bool_tag", "bool_true", params); + ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; + ASSERT_TRUE(NULL != param_value->bool_value); + EXPECT_TRUE(*param_value->bool_value); + param_value = rcl_yaml_node_struct_get("bool_tag", "bool_yes", params); + ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; + ASSERT_TRUE(NULL != param_value->bool_value); + EXPECT_TRUE(*param_value->bool_value); + param_value = rcl_yaml_node_struct_get("bool_tag", "bool_on", params); + ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; + ASSERT_TRUE(NULL != param_value->bool_value); + EXPECT_TRUE(*param_value->bool_value); + param_value = rcl_yaml_node_struct_get("bool_tag", "bool_false", params); + ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; + ASSERT_TRUE(NULL != param_value->bool_value); + EXPECT_FALSE(*param_value->bool_value); + param_value = rcl_yaml_node_struct_get("bool_tag", "bool_no", params); + ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; + ASSERT_TRUE(NULL != param_value->bool_value); + EXPECT_FALSE(*param_value->bool_value); + param_value = rcl_yaml_node_struct_get("bool_tag", "bool_off", params); + ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; + ASSERT_TRUE(NULL != param_value->bool_value); + EXPECT_FALSE(*param_value->bool_value); + + param_value = rcl_yaml_node_struct_get("null_tag", "null_null", params); + ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; + ASSERT_TRUE(NULL != param_value->string_value); + EXPECT_STREQ("null", param_value->string_value); + param_value = rcl_yaml_node_struct_get("null_tag", "null_none", params); + ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; + ASSERT_TRUE(NULL != param_value->string_value); + EXPECT_STREQ("null", param_value->string_value); + + param_value = rcl_yaml_node_struct_get("int_tag", "int_pos", params); + ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; + ASSERT_TRUE(NULL != param_value->integer_value); + EXPECT_EQ(10, *param_value->integer_value); + param_value = rcl_yaml_node_struct_get("int_tag", "int_zero", params); + ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; + ASSERT_TRUE(NULL != param_value->integer_value); + EXPECT_EQ(0, *param_value->integer_value); + param_value = rcl_yaml_node_struct_get("int_tag", "int_neg", params); + ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; + ASSERT_TRUE(NULL != param_value->integer_value); + EXPECT_EQ(-10, *param_value->integer_value); + + param_value = rcl_yaml_node_struct_get("float_tag", "float_pos", params); + ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; + ASSERT_TRUE(NULL != param_value->double_value); + EXPECT_DOUBLE_EQ(1.1, *param_value->double_value); + param_value = rcl_yaml_node_struct_get("float_tag", "float_zero", params); + ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; + ASSERT_TRUE(NULL != param_value->double_value); + EXPECT_DOUBLE_EQ(0.0, *param_value->double_value); + param_value = rcl_yaml_node_struct_get("float_tag", "float_neg", params); + ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; + ASSERT_TRUE(NULL != param_value->double_value); + EXPECT_DOUBLE_EQ(-1.122, *param_value->double_value); + param_value = rcl_yaml_node_struct_get("float_tag", "float_nan", params); + ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; + ASSERT_TRUE(NULL != param_value->double_value); + EXPECT_TRUE(std::isnan(*param_value->double_value)); + param_value = rcl_yaml_node_struct_get("float_tag", "float_inf", params); + ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; + ASSERT_TRUE(NULL != param_value->double_value); + EXPECT_DOUBLE_EQ(INFINITY, *param_value->double_value); + param_value = rcl_yaml_node_struct_get("float_tag", "float_infn", params); + ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; + ASSERT_TRUE(NULL != param_value->double_value); + EXPECT_DOUBLE_EQ(INFINITY, *param_value->double_value); + param_value = rcl_yaml_node_struct_get("float_tag", "float_infp", params); + ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; + ASSERT_TRUE(NULL != param_value->double_value); + EXPECT_DOUBLE_EQ(-INFINITY, *param_value->double_value); + rcl_yaml_node_struct_print(params); } } From ba4046a94425c338264f245f1caef719db5585e9 Mon Sep 17 00:00:00 2001 From: Lei Liu Date: Fri, 3 Feb 2023 13:28:13 +0800 Subject: [PATCH 2/4] Update code. Signed-off-by: Lei Liu --- rcl_yaml_param_parser/src/parse.c | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/rcl_yaml_param_parser/src/parse.c b/rcl_yaml_param_parser/src/parse.c index 3eb2c5da8..c4935ea33 100644 --- a/rcl_yaml_param_parser/src/parse.c +++ b/rcl_yaml_param_parser/src/parse.c @@ -606,7 +606,6 @@ _validate_tag(const char * tag, uint32_t line_num) (0 == strcmp(tag, YAML_STR_TAG)) || (0 == strcmp(tag, YAML_INT_TAG)) || (0 == strcmp(tag, YAML_FLOAT_TAG)) || - (0 == strcmp(tag, YAML_TIMESTAMP_TAG)) || (0 == strcmp(tag, YAML_SEQ_TAG)) || (0 == strcmp(tag, YAML_MAP_TAG))) { @@ -625,8 +624,6 @@ _validate_bool_value( void ** ret_val, const rcutils_allocator_t allocator) { - rcutils_ret_t ret = RCUTILS_RET_ERROR; - if ((0 == strcmp(value, "Y")) || (0 == strcmp(value, "y")) || (0 == strcmp(value, "yes")) || @@ -644,8 +641,7 @@ _validate_bool_value( if (NULL != *ret_val) { *((bool *)*ret_val) = true; } - ret = RCUTILS_RET_OK; - goto final; + return RCUTILS_RET_OK; } if ((0 == strcmp(value, "N")) || @@ -665,12 +661,10 @@ _validate_bool_value( if (NULL != *ret_val) { *((bool *)*ret_val) = false; } - ret = RCUTILS_RET_OK; - goto final; + return RCUTILS_RET_OK; } -final: - return ret; + return RCUTILS_RET_ERROR; } rcutils_ret_t @@ -683,7 +677,6 @@ _validate_int_value( errno = 0; int64_t ival; char * endptr = NULL; - rcutils_ret_t ret = RCUTILS_RET_ERROR; ival = strtol(value, &endptr, 0); if ((0 == errno) && (NULL != endptr)) { @@ -694,12 +687,12 @@ _validate_int_value( if (NULL != *ret_val) { *((int64_t *)*ret_val) = ival; } - ret = RCUTILS_RET_OK; + return RCUTILS_RET_OK; } } } - return ret; + return RCUTILS_RET_ERROR; } rcutils_ret_t @@ -713,7 +706,6 @@ _validate_float_value( double dval; char * endptr = NULL; const char * iter_ptr = NULL; - rcutils_ret_t ret = RCUTILS_RET_ERROR; if ((0 == strcmp(value, ".nan")) || (0 == strcmp(value, ".NaN")) || @@ -746,12 +738,12 @@ _validate_float_value( if (NULL != *ret_val) { *((double *)*ret_val) = dval; } - ret = RCUTILS_RET_OK; + return RCUTILS_RET_OK; } } } - return ret; + return RCUTILS_RET_ERROR; } /// From 7920f008f8be0f0281db0559931e5669a4ceb07c Mon Sep 17 00:00:00 2001 From: Lei Liu Date: Mon, 6 Feb 2023 15:45:57 +0800 Subject: [PATCH 3/4] Update code again. Signed-off-by: Lei Liu --- rcl_yaml_param_parser/src/parse.c | 79 +++++++++---------- .../test/correct_config.yaml | 13 ++- rcl_yaml_param_parser/test/test_parse.cpp | 2 + .../test/test_parse_yaml.cpp | 66 +++++++++++++--- 4 files changed, 105 insertions(+), 55 deletions(-) diff --git a/rcl_yaml_param_parser/src/parse.c b/rcl_yaml_param_parser/src/parse.c index c4935ea33..74221dfd4 100644 --- a/rcl_yaml_param_parser/src/parse.c +++ b/rcl_yaml_param_parser/src/parse.c @@ -70,7 +70,12 @@ _validate_name(const char * name, rcutils_allocator_t allocator); /// /// Check a tag whether it is valid /// -/// \param tag the tag to check +/// \param tag the tag to check, include tags: +/// YAML_BOOL_TAG, YAML_STR_TAG, YAML_INT_TAG, +/// YAML_FLOAT_TAG, YAML_SEQ_TAG, YAML_MAP_TAG +/// NOTE: YAML_NULL_TAG & YAML_TIMESTAMP_TAG are +/// not supported by ROS2 Parameters, so they are +/// excluded. /// \param line_num the line number error happened /// \return RCUTILS_RET_OK if tag is valid, or /// \return RCUTILS_RET_ERROR if tag is not valid @@ -79,48 +84,48 @@ rcutils_ret_t _validate_tag(const char * tag, uint32_t line_num); /// -/// Check a bool value whether it is valid +/// Get a bool value when it is valid /// -/// \param value the bool value to check +/// \param value the bool value to get /// \param val_type the value type /// \param ret_val the converted value when value is valid /// \return RCUTILS_RET_OK if value is valid, or /// \return RCUTILS_RET_ERROR if value is not valid RCL_YAML_PARAM_PARSER_LOCAL rcutils_ret_t -_validate_bool_value( +_get_bool_value( const char * const value, data_types_t * val_type, void ** ret_val, const rcutils_allocator_t allocator); /// -/// Check a int value whether it is valid +/// Get a int value when it is valid /// -/// \param value the int value to check +/// \param value the int value to get /// \param val_type the value type /// \param ret_val the converted value when value is valid /// \return RCUTILS_RET_OK if value is valid, or /// \return RCUTILS_RET_ERROR if value is not valid RCL_YAML_PARAM_PARSER_LOCAL rcutils_ret_t -_validate_int_value( +_get_int_value( const char * const value, data_types_t * val_type, void ** ret_val, const rcutils_allocator_t allocator); /// -/// Check a float value whether it is valid +/// Get a float value when it is valid /// -/// \param value the float value to check +/// \param value the float value to get /// \param val_type the value type /// \param ret_val the converted value when value is valid /// \return RCUTILS_RET_OK if value is valid, or /// \return RCUTILS_RET_ERROR if value is not valid RCL_YAML_PARAM_PARSER_LOCAL rcutils_ret_t -_validate_float_value( +_get_float_value( const char * const value, data_types_t * val_type, void ** ret_val, @@ -150,22 +155,9 @@ void * get_value( return rcutils_strdup(value, allocator); } - /// Check for yaml null tag - if (tag != NULL && strcmp(YAML_NULL_TAG, (char *)tag) == 0) { - *val_type = DATA_TYPE_STRING; - if ((0 == strcmp(value, "null")) || - (0 == strcmp(value, "Null")) || - (0 == strcmp(value, "none")) || - (0 == strcmp(value, "None"))) - { - return rcutils_strdup("null", allocator); - } - return NULL; - } - /// Check for yaml bool tag if (tag != NULL && strcmp(YAML_BOOL_TAG, (char *)tag) == 0) { - if (_validate_bool_value(value, val_type, &ret_val, allocator) == RCUTILS_RET_OK) { + if (_get_bool_value(value, val_type, &ret_val, allocator) != RCUTILS_RET_ERROR) { return ret_val; } else { return NULL; @@ -174,7 +166,7 @@ void * get_value( /// Check for yaml int tag if (tag != NULL && strcmp(YAML_INT_TAG, (char *)tag) == 0) { - if (_validate_int_value(value, val_type, &ret_val, allocator) == RCUTILS_RET_OK) { + if (_get_int_value(value, val_type, &ret_val, allocator) != RCUTILS_RET_ERROR) { return ret_val; } else { return NULL; @@ -183,7 +175,7 @@ void * get_value( /// Check for yaml float tag if (tag != NULL && strcmp(YAML_FLOAT_TAG, (char *)tag) == 0) { - if (_validate_float_value(value, val_type, &ret_val, allocator) == RCUTILS_RET_OK) { + if (_get_float_value(value, val_type, &ret_val, allocator) != RCUTILS_RET_ERROR) { return ret_val; } else { return NULL; @@ -194,7 +186,7 @@ void * get_value( if (style != YAML_SINGLE_QUOTED_SCALAR_STYLE && style != YAML_DOUBLE_QUOTED_SCALAR_STYLE) { - if (_validate_bool_value(value, val_type, &ret_val, allocator) == RCUTILS_RET_OK) { + if (_get_bool_value(value, val_type, &ret_val, allocator) != RCUTILS_RET_ERROR) { return ret_val; } } @@ -203,7 +195,7 @@ void * get_value( if (style != YAML_SINGLE_QUOTED_SCALAR_STYLE && style != YAML_DOUBLE_QUOTED_SCALAR_STYLE) { - if (_validate_int_value(value, val_type, &ret_val, allocator) == RCUTILS_RET_OK) { + if (_get_int_value(value, val_type, &ret_val, allocator) != RCUTILS_RET_ERROR) { return ret_val; } } @@ -212,7 +204,7 @@ void * get_value( if (style != YAML_SINGLE_QUOTED_SCALAR_STYLE && style != YAML_DOUBLE_QUOTED_SCALAR_STYLE) { - if (_validate_float_value(value, val_type, &ret_val, allocator) == RCUTILS_RET_OK) { + if (_get_float_value(value, val_type, &ret_val, allocator) != RCUTILS_RET_ERROR) { return ret_val; } } @@ -601,8 +593,7 @@ _validate_tag(const char * tag, uint32_t line_num) return RCUTILS_RET_ERROR; } - if ((0 == strcmp(tag, YAML_NULL_TAG)) || - (0 == strcmp(tag, YAML_BOOL_TAG)) || + if ((0 == strcmp(tag, YAML_BOOL_TAG)) || (0 == strcmp(tag, YAML_STR_TAG)) || (0 == strcmp(tag, YAML_INT_TAG)) || (0 == strcmp(tag, YAML_FLOAT_TAG)) || @@ -618,7 +609,7 @@ _validate_tag(const char * tag, uint32_t line_num) } rcutils_ret_t -_validate_bool_value( +_get_bool_value( const char * const value, data_types_t * val_type, void ** ret_val, @@ -638,9 +629,10 @@ _validate_bool_value( { *val_type = DATA_TYPE_BOOL; *ret_val = allocator.zero_allocate(1U, sizeof(bool), allocator.state); - if (NULL != *ret_val) { - *((bool *)*ret_val) = true; + if (NULL == *ret_val) { + return RCUTILS_RET_BAD_ALLOC; } + *((bool *)*ret_val) = true; return RCUTILS_RET_OK; } @@ -658,9 +650,10 @@ _validate_bool_value( { *val_type = DATA_TYPE_BOOL; *ret_val = allocator.zero_allocate(1U, sizeof(bool), allocator.state); - if (NULL != *ret_val) { - *((bool *)*ret_val) = false; + if (NULL == *ret_val) { + return RCUTILS_RET_BAD_ALLOC; } + *((bool *)*ret_val) = false; return RCUTILS_RET_OK; } @@ -668,7 +661,7 @@ _validate_bool_value( } rcutils_ret_t -_validate_int_value( +_get_int_value( const char * const value, data_types_t * val_type, void ** ret_val, @@ -684,9 +677,10 @@ _validate_int_value( if (('\0' != *value) && ('\0' == *endptr)) { *val_type = DATA_TYPE_INT64; *ret_val = allocator.zero_allocate(1U, sizeof(int64_t), allocator.state); - if (NULL != *ret_val) { - *((int64_t *)*ret_val) = ival; + if (NULL == *ret_val) { + return RCUTILS_RET_BAD_ALLOC; } + *((int64_t *)*ret_val) = ival; return RCUTILS_RET_OK; } } @@ -696,7 +690,7 @@ _validate_int_value( } rcutils_ret_t -_validate_float_value( +_get_float_value( const char * const value, data_types_t * val_type, void ** ret_val, @@ -735,9 +729,10 @@ _validate_float_value( if (('\0' != *value) && ('\0' == *endptr)) { *val_type = DATA_TYPE_DOUBLE; *ret_val = allocator.zero_allocate(1U, sizeof(double), allocator.state); - if (NULL != *ret_val) { - *((double *)*ret_val) = dval; + if (NULL == *ret_val) { + return RCUTILS_RET_BAD_ALLOC; } + *((double *)*ret_val) = dval; return RCUTILS_RET_OK; } } diff --git a/rcl_yaml_param_parser/test/correct_config.yaml b/rcl_yaml_param_parser/test/correct_config.yaml index 54661364d..0f5ed8483 100644 --- a/rcl_yaml_param_parser/test/correct_config.yaml +++ b/rcl_yaml_param_parser/test/correct_config.yaml @@ -62,10 +62,6 @@ bool_tag: bool_false: !!bool false bool_no: !!bool no bool_off: !!bool off -null_tag: - ros__parameters: - null_null: !!null null - null_none: !!null none int_tag: ros__parameters: int_pos: !!int 10 @@ -80,3 +76,12 @@ float_tag: float_inf: !!float .inf float_infn: !!float +.inf float_infp: !!float -.inf +seq_tag: + ros__parameters: + seq_string: !!seq [ab, bc, cd, de] + seq_bool: !!seq [true, yes, on, false, no, off] + seq_int: !!seq [10, 0, -10] + seq_float: !!seq [1.1, 0.0, -1.122, .nan, .inf, +.inf, -.inf] +map_tag: + ros__parameters: + map_test: !!map {str: string, bool: true, int: 10, float: 1.1} diff --git a/rcl_yaml_param_parser/test/test_parse.cpp b/rcl_yaml_param_parser/test/test_parse.cpp index e53849cc0..7428159df 100644 --- a/rcl_yaml_param_parser/test/test_parse.cpp +++ b/rcl_yaml_param_parser/test/test_parse.cpp @@ -497,6 +497,7 @@ TEST(TestParse, parse_file_events_mock_yaml_parser_parse) { "lib:rcl_yaml_param_parser", yaml_parser_parse, [](yaml_parser_t *, yaml_event_t * event) { event->start_mark.line = 0u; event->type = YAML_NO_EVENT; + event->data.scalar.tag = NULL; return 1; }); EXPECT_EQ(RCUTILS_RET_ERROR, parse_file_events(&parser, &ns_tracker, params_hdl)); @@ -518,6 +519,7 @@ TEST(TestParse, parse_value_events_mock_yaml_parser_parse) { "lib:rcl_yaml_param_parser", yaml_parser_parse, [](yaml_parser_t *, yaml_event_t * event) { event->start_mark.line = 0u; event->type = YAML_NO_EVENT; + event->data.scalar.tag = NULL; return 1; }); EXPECT_FALSE(rcl_parse_yaml_value(node_name, param_name, yaml_value, params_st)); diff --git a/rcl_yaml_param_parser/test/test_parse_yaml.cpp b/rcl_yaml_param_parser/test/test_parse_yaml.cpp index 85ce3809c..3cbe12ae0 100644 --- a/rcl_yaml_param_parser/test/test_parse_yaml.cpp +++ b/rcl_yaml_param_parser/test/test_parse_yaml.cpp @@ -232,15 +232,6 @@ TEST(test_parser, correct_syntax) { ASSERT_TRUE(NULL != param_value->bool_value); EXPECT_FALSE(*param_value->bool_value); - param_value = rcl_yaml_node_struct_get("null_tag", "null_null", params); - ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; - ASSERT_TRUE(NULL != param_value->string_value); - EXPECT_STREQ("null", param_value->string_value); - param_value = rcl_yaml_node_struct_get("null_tag", "null_none", params); - ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; - ASSERT_TRUE(NULL != param_value->string_value); - EXPECT_STREQ("null", param_value->string_value); - param_value = rcl_yaml_node_struct_get("int_tag", "int_pos", params); ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; ASSERT_TRUE(NULL != param_value->integer_value); @@ -283,6 +274,63 @@ TEST(test_parser, correct_syntax) { ASSERT_TRUE(NULL != param_value->double_value); EXPECT_DOUBLE_EQ(-INFINITY, *param_value->double_value); + param_value = rcl_yaml_node_struct_get("seq_tag", "seq_string", params_hdl); + ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; + ASSERT_TRUE(NULL != param_value->string_array_value); + ASSERT_EQ(4U, param_value->string_array_value->size); + EXPECT_STREQ("ab", param_value->string_array_value->data[0]); + EXPECT_STREQ("bc", param_value->string_array_value->data[1]); + EXPECT_STREQ("cd", param_value->string_array_value->data[2]); + EXPECT_STREQ("de", param_value->string_array_value->data[3]); + + param_value = rcl_yaml_node_struct_get("seq_tag", "seq_bool", params_hdl); + ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; + ASSERT_TRUE(NULL != param_value->bool_array_value); + ASSERT_EQ(6U, param_value->bool_array_value->size); + EXPECT_TRUE(param_value->bool_array_value->values[0]); + EXPECT_TRUE(param_value->bool_array_value->values[1]); + EXPECT_TRUE(param_value->bool_array_value->values[2]); + EXPECT_FALSE(param_value->bool_array_value->values[3]); + EXPECT_FALSE(param_value->bool_array_value->values[4]); + EXPECT_FALSE(param_value->bool_array_value->values[5]); + + param_value = rcl_yaml_node_struct_get("seq_tag", "seq_int", params_hdl); + ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; + ASSERT_TRUE(NULL != param_value->integer_array_value); + ASSERT_EQ(3U, param_value->integer_array_value->size); + EXPECT_EQ(10, param_value->integer_array_value->values[0]); + EXPECT_EQ(0, param_value->integer_array_value->values[1]); + EXPECT_EQ(-10, param_value->integer_array_value->values[2]); + + param_value = rcl_yaml_node_struct_get("seq_tag", "seq_float", params_hdl); + ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; + ASSERT_TRUE(NULL != param_value->double_array_value); + ASSERT_EQ(7U, param_value->double_array_value->size); + EXPECT_DOUBLE_EQ(1.1, param_value->double_array_value->values[0]); + EXPECT_DOUBLE_EQ(0.0, param_value->double_array_value->values[1]); + EXPECT_DOUBLE_EQ(-1.122, param_value->double_array_value->values[2]); + EXPECT_TRUE(std::isnan(param_value->double_array_value->values[3])); + EXPECT_DOUBLE_EQ(INFINITY, param_value->double_array_value->values[4]); + EXPECT_DOUBLE_EQ(INFINITY, param_value->double_array_value->values[5]); + EXPECT_DOUBLE_EQ(-INFINITY, param_value->double_array_value->values[6]); + + param_value = rcl_yaml_node_struct_get("map_tag", "map_test.str", params); + ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; + ASSERT_TRUE(NULL != param_value->string_value); + EXPECT_STREQ("string", param_value->string_value); + param_value = rcl_yaml_node_struct_get("map_tag", "map_test.bool", params); + ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; + ASSERT_TRUE(NULL != param_value->bool_value); + EXPECT_TRUE(*param_value->bool_value); + param_value = rcl_yaml_node_struct_get("map_tag", "map_test.int", params); + ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; + ASSERT_TRUE(NULL != param_value->integer_value); + EXPECT_EQ(10, *param_value->integer_value); + param_value = rcl_yaml_node_struct_get("map_tag", "map_test.float", params); + ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; + ASSERT_TRUE(NULL != param_value->double_value); + EXPECT_DOUBLE_EQ(1.1, *param_value->double_value); + rcl_yaml_node_struct_print(params); } } From 4530a3fccb4592abf760e81bad5766b9a1e39e2e Mon Sep 17 00:00:00 2001 From: Lei Liu Date: Wed, 8 Feb 2023 10:09:58 +0800 Subject: [PATCH 4/4] Add testcases for incorrect config Signed-off-by: Lei Liu --- rcl_yaml_param_parser/src/parse.c | 4 -- .../test/tag_incorrect_config_01.yaml | 3 ++ .../test/tag_incorrect_config_02.yaml | 3 ++ .../test/tag_incorrect_config_03.yaml | 3 ++ .../test/tag_incorrect_config_04.yaml | 3 ++ .../test/tag_incorrect_config_05.yaml | 3 ++ .../test/test_parse_yaml.cpp | 37 +++++++++++++++++++ 7 files changed, 52 insertions(+), 4 deletions(-) create mode 100644 rcl_yaml_param_parser/test/tag_incorrect_config_01.yaml create mode 100644 rcl_yaml_param_parser/test/tag_incorrect_config_02.yaml create mode 100644 rcl_yaml_param_parser/test/tag_incorrect_config_03.yaml create mode 100644 rcl_yaml_param_parser/test/tag_incorrect_config_04.yaml create mode 100644 rcl_yaml_param_parser/test/tag_incorrect_config_05.yaml diff --git a/rcl_yaml_param_parser/src/parse.c b/rcl_yaml_param_parser/src/parse.c index 74221dfd4..c8b0374af 100644 --- a/rcl_yaml_param_parser/src/parse.c +++ b/rcl_yaml_param_parser/src/parse.c @@ -589,10 +589,6 @@ _validate_name(const char * name, rcutils_allocator_t allocator) rcutils_ret_t _validate_tag(const char * tag, uint32_t line_num) { - if (tag == NULL) { - return RCUTILS_RET_ERROR; - } - if ((0 == strcmp(tag, YAML_BOOL_TAG)) || (0 == strcmp(tag, YAML_STR_TAG)) || (0 == strcmp(tag, YAML_INT_TAG)) || diff --git a/rcl_yaml_param_parser/test/tag_incorrect_config_01.yaml b/rcl_yaml_param_parser/test/tag_incorrect_config_01.yaml new file mode 100644 index 000000000..e6b6ac459 --- /dev/null +++ b/rcl_yaml_param_parser/test/tag_incorrect_config_01.yaml @@ -0,0 +1,3 @@ +bool_tag: + ros__parameters: + test: !!bool eurt \ No newline at end of file diff --git a/rcl_yaml_param_parser/test/tag_incorrect_config_02.yaml b/rcl_yaml_param_parser/test/tag_incorrect_config_02.yaml new file mode 100644 index 000000000..fd6e0b9fc --- /dev/null +++ b/rcl_yaml_param_parser/test/tag_incorrect_config_02.yaml @@ -0,0 +1,3 @@ +int_tag: + ros__parameters: + test: !!int a10 \ No newline at end of file diff --git a/rcl_yaml_param_parser/test/tag_incorrect_config_03.yaml b/rcl_yaml_param_parser/test/tag_incorrect_config_03.yaml new file mode 100644 index 000000000..60469a5be --- /dev/null +++ b/rcl_yaml_param_parser/test/tag_incorrect_config_03.yaml @@ -0,0 +1,3 @@ +float_tag: + ros__parameters: + test: !!float a10 \ No newline at end of file diff --git a/rcl_yaml_param_parser/test/tag_incorrect_config_04.yaml b/rcl_yaml_param_parser/test/tag_incorrect_config_04.yaml new file mode 100644 index 000000000..fe3fb26aa --- /dev/null +++ b/rcl_yaml_param_parser/test/tag_incorrect_config_04.yaml @@ -0,0 +1,3 @@ +seq_tag: + ros__parameters: + test: !!seq {a: 1, b: 2} \ No newline at end of file diff --git a/rcl_yaml_param_parser/test/tag_incorrect_config_05.yaml b/rcl_yaml_param_parser/test/tag_incorrect_config_05.yaml new file mode 100644 index 000000000..f5a5e9c84 --- /dev/null +++ b/rcl_yaml_param_parser/test/tag_incorrect_config_05.yaml @@ -0,0 +1,3 @@ +map_tag: + ros__parameters: + test: !!map [1, 2] \ No newline at end of file diff --git a/rcl_yaml_param_parser/test/test_parse_yaml.cpp b/rcl_yaml_param_parser/test/test_parse_yaml.cpp index 3cbe12ae0..d1f97fa27 100644 --- a/rcl_yaml_param_parser/test/test_parse_yaml.cpp +++ b/rcl_yaml_param_parser/test/test_parse_yaml.cpp @@ -798,6 +798,43 @@ TEST(test_file_parser, wildcards_partial) { } } +TEST(test_file_parser, tag_incorrect_config) { + 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); + }); + const std::vector filenames = { + "tag_incorrect_config_01.yaml", + "tag_incorrect_config_02.yaml", + "tag_incorrect_config_03.yaml", + "tag_incorrect_config_04.yaml", + "tag_incorrect_config_05.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) { ::testing::InitGoogleTest(&argc, argv);