Skip to content

Commit

Permalink
Remove the maximum string size. (ros2#524)
Browse files Browse the repository at this point in the history
It wasn't preventing any allocations from happening, so it
doesn't seem to serve much purpose.  Also remove the tests
for the maximum string size.

Signed-off-by: Chris Lalancette <[email protected]>
  • Loading branch information
clalancette authored Oct 21, 2019
1 parent 7859398 commit cdf1b4d
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 63 deletions.
40 changes: 7 additions & 33 deletions rcl_yaml_param_parser/src/parser.c
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ typedef struct namespace_tracker_s
uint32_t num_parameter_ns;
} namespace_tracker_t;

#define MAX_STRING_SIZE 256U
#define PARAMS_KEY "ros__parameters"
#define NODE_NS_SEPERATOR "/"
#define PARAMETER_NS_SEPERATOR "."
Expand Down Expand Up @@ -207,10 +206,6 @@ static rcutils_ret_t add_name_to_ns(

tot_len = ns_len + sep_len + name_len + 1U;

if (tot_len > MAX_STRING_SIZE) {
RCUTILS_SET_ERROR_MSG("New namespace string is exceeding max string size");
return RCUTILS_RET_ERROR;
}
cur_ns = allocator.reallocate(cur_ns, tot_len, allocator.state);
if (NULL == cur_ns) {
return RCUTILS_RET_BAD_ALLOC;
Expand Down Expand Up @@ -1116,19 +1111,12 @@ static rcutils_ret_t parse_value(
RCUTILS_CHECK_FOR_NULL_WITH_MSG(
value, "event argument has no value", return RCUTILS_RET_INVALID_ARGUMENT);

if (val_size > MAX_STRING_SIZE) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Scalar value at line %u has %zu bytes which is bigger than the compile "
"time limit of %u bytes", line_num, val_size, MAX_STRING_SIZE);
if (style != YAML_SINGLE_QUOTED_SCALAR_STYLE &&
style != YAML_DOUBLE_QUOTED_SCALAR_STYLE &&
0U == val_size)
{
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("No value at line %d", line_num);
return RCUTILS_RET_ERROR;
} else {
if (style != YAML_SINGLE_QUOTED_SCALAR_STYLE &&
style != YAML_DOUBLE_QUOTED_SCALAR_STYLE &&
0U == val_size)
{
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("No value at line %d", line_num);
return RCUTILS_RET_ERROR;
}
}

if (NULL == params_st->params[node_idx].parameter_values) {
Expand Down Expand Up @@ -1315,16 +1303,9 @@ static rcutils_ret_t parse_key(
RCUTILS_CHECK_FOR_NULL_WITH_MSG(
value, "event argument has no value", return RCUTILS_RET_INVALID_ARGUMENT);

if (val_size > MAX_STRING_SIZE) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Scalar value at line %d is bigger than %d bytes",
line_num, MAX_STRING_SIZE);
if (0U == val_size) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("No key at line %d", line_num);
return RCUTILS_RET_ERROR;
} else {
if (0U == val_size) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("No key at line %d", line_num);
return RCUTILS_RET_ERROR;
}
}

rcutils_ret_t ret = RCUTILS_RET_OK;
Expand Down Expand Up @@ -1426,13 +1407,6 @@ static rcutils_ret_t parse_key(
const size_t param_name_len = strlen(value);
const size_t tot_len = (params_ns_len + param_name_len + 2U);

if (tot_len > MAX_STRING_SIZE) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"The name length exceeds the MAX size %d at line %d", MAX_STRING_SIZE, line_num);
ret = RCUTILS_RET_ERROR;
break;
}

param_name = allocator.zero_allocate(1U, tot_len, allocator.state);
if (NULL == param_name) {
ret = RCUTILS_RET_BAD_ALLOC;
Expand Down
8 changes: 0 additions & 8 deletions rcl_yaml_param_parser/test/max_string_sz.yaml

This file was deleted.

22 changes: 0 additions & 22 deletions rcl_yaml_param_parser/test/test_parse_yaml.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -363,28 +363,6 @@ TEST(test_file_parser, no_alias_support) {
EXPECT_FALSE(res);
}

TEST(test_file_parser, max_string_sz) {
rcutils_reset_error();
EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)) << rcutils_get_error_string().str;
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, "max_string_sz.yaml", allocator);
ASSERT_TRUE(NULL != path) << rcutils_get_error_string().str;
OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({
allocator.deallocate(path, allocator.state);
});
ASSERT_TRUE(rcutils_exists(path)) << "No test YAML file found at " << path;
rcl_params_t * params_hdl = rcl_yaml_node_struct_init(allocator);
ASSERT_TRUE(NULL != params_hdl) << rcutils_get_error_string().str;
bool res = rcl_parse_yaml_file(path, params_hdl);
fprintf(stderr, "%s\n", rcutils_get_error_string().str);
EXPECT_FALSE(res);
}

TEST(test_file_parser, empty_string) {
rcutils_reset_error();
EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)) << rcutils_get_error_string().str;
Expand Down

0 comments on commit cdf1b4d

Please sign in to comment.