Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

adapt to NULL removal from rmw result validation string #193

Merged
merged 5 commits into from
Jan 8, 2018
Merged

adapt to NULL removal from rmw result validation string #193

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f12437a
adapt to NULL removal from rmw result valiation string
gaoethan Nov 28, 2017
e15bd7b
adapt to rmw validation change and update rcl topic validation
gaoethan Nov 29, 2017
518a676
tweak the default error string returned
gaoethan Nov 30, 2017
b3bcfba
tweak error output format
gaoethan Dec 4, 2017
5011aed
fix build error
gaoethan Dec 21, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 1 addition & 12 deletions rcl/src/rcl/node.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,6 @@ rcl_node_init(
}
if (validation_result != RMW_NODE_NAME_VALID) {
const char * msg = rmw_node_name_validation_result_string(validation_result);
if (!msg) {
msg = "unknown validation_result, this should not happen";
}
RCL_SET_ERROR_MSG(msg, *allocator);
return RCL_RET_NODE_INVALID_NAME;
}
Expand Down Expand Up @@ -169,15 +166,7 @@ rcl_node_init(
}
if (validation_result != RMW_NAMESPACE_VALID) {
const char * msg = rmw_namespace_validation_result_string(validation_result);
if (!msg) {
char fixed_msg[256];
rcutils_snprintf(
fixed_msg, sizeof(fixed_msg),
"unknown validation_result '%d', this should not happen", validation_result);
RCL_SET_ERROR_MSG(fixed_msg, *allocator);
} else {
RCL_SET_ERROR_MSG(msg, *allocator);
}
RCL_SET_ERROR_MSG_WITH_FORMAT_STRING(*allocator, "%s, result '%d'", msg, validation_result);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change fails to compile: (http://ci.ros2.org/job/ci_linux-aarch64/935/consoleFull, http://ci.ros2.org/job/ci_windows/3885/). I didn't have time to investigate why yet

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed, thanks !


if (should_free_local_namespace_) {
allocator->deallocate((char *)local_namespace_, allocator->state);
Expand Down
4 changes: 3 additions & 1 deletion rcl/src/rcl/validate_topic_name.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,8 @@ const char *
rcl_topic_name_validation_result_string(int validation_result)
{
switch (validation_result) {
case RCL_TOPIC_NAME_VALID:
return NULL;
case RCL_TOPIC_NAME_INVALID_IS_EMPTY_STRING:
return "topic name must not be empty string";
case RCL_TOPIC_NAME_INVALID_ENDS_WITH_FORWARD_SLASH:
Expand All @@ -213,7 +215,7 @@ rcl_topic_name_validation_result_string(int validation_result)
case RCL_TOPIC_NAME_INVALID_SUBSTITUTION_STARTS_WITH_NUMBER:
return "substitution name must not start with a number";
default:
return NULL;
return "unknown result code for rcl topic name validation";
}
}

Expand Down
10 changes: 5 additions & 5 deletions rcl/test/rcl/test_validate_topic_name.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ TEST(test_validate_topic_name, normal) {
// passing without invalid_index
{
int validation_result;
ret = rcl_validate_topic_name("topic", &validation_result, NULL);
ret = rcl_validate_topic_name("topic", &validation_result, nullptr);
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string_safe();
EXPECT_EQ(RCL_TOPIC_NAME_VALID, validation_result);
EXPECT_EQ(NULL, rcl_topic_name_validation_result_string(validation_result));
EXPECT_EQ(nullptr, rcl_topic_name_validation_result_string(validation_result));
}

// passing with invalid_index
Expand All @@ -42,7 +42,7 @@ TEST(test_validate_topic_name, normal) {
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string_safe();
EXPECT_EQ(RCL_TOPIC_NAME_VALID, validation_result);
EXPECT_EQ(42u, invalid_index); // ensure invalid_index is not assigned on success
EXPECT_EQ(NULL, rcl_topic_name_validation_result_string(validation_result));
EXPECT_EQ(nullptr, rcl_topic_name_validation_result_string(validation_result));
}

// failing with invalid_index
Expand All @@ -63,14 +63,14 @@ TEST(test_validate_topic_name, invalid_arguments) {
// pass null for topic string
{
int validation_result;
ret = rcl_validate_topic_name(NULL, &validation_result, NULL);
ret = rcl_validate_topic_name(nullptr, &validation_result, nullptr);
EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, ret);
rcl_reset_error();
}

// pass null for validation_result
{
ret = rcl_validate_topic_name("topic", NULL, NULL);
ret = rcl_validate_topic_name("topic", nullptr, nullptr);
EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, ret);
rcl_reset_error();
}
Expand Down