Skip to content

Commit

Permalink
Optimize namespace node and topic validation (#130)
Browse files Browse the repository at this point in the history 
* Optimize namespace node and topic validation
make the valid result more accurate, which should be
different from the default NULL
use rmw_namespace_validation_result_string instead
for namespace validation

Signed-off-by: Ethan Gao <ethan.gao@linux.intel.com>

* no NULL returned from validation string
and twaek the tests accordingly

Signed-off-by: Ethan Gao <ethan.gao@linux.intel.com>

* keep NULL for the valid case and adapt tests

Signed-off-by: Ethan Gao <ethan.gao@linux.intel.com>

* tweak the default error string returned

Signed-off-by: Ethan Gao <ethan.gao@linux.intel.com>
  • Loading branch information
@gaoethan @wjwwood
gaoethan authored and wjwwood committed Jan 8, 2018
1 parent 4c39335 commit 589c69e
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 24 deletions.
2 changes: 1 addition & 1 deletion rmw/src/validate_full_topic_name.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,6 @@ rmw_full_topic_name_validation_result_string(int validation_result)
case RMW_TOPIC_INVALID_TOO_LONG:
return "topic length should not exceed '" RMW_STRINGIFY(RMW_TOPIC_MAX_NAME_LENGTH) "'";
default:
return NULL;
return "unknown result code for rwm topic name validation";
}
}
2 changes: 1 addition & 1 deletion rmw/src/validate_namespace.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,6 @@ rmw_namespace_validation_result_string(int validation_result)
case RMW_NAMESPACE_INVALID_TOO_LONG:
return "namespace should not exceed '" RMW_STRINGIFY(RMW_NAMESPACE_MAX_NAME_LENGTH) "'";
default:
return NULL;
return "unknown result code for rmw namespace validation";
}
}
2 changes: 1 addition & 1 deletion rmw/src/validate_node_name.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,6 @@ rmw_node_name_validation_result_string(int validation_result)
return
"node name length should not exceed '" RMW_STRINGIFY(RMW_NODE_NAME_MAX_NAME_LENGTH) "'";
default:
return NULL;
return "unknown result code for rmw node name validation";
}
}
16 changes: 8 additions & 8 deletions rmw/test/test_validate_full_topic_name.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ TEST(test_validate_topic_name, valid_topic) {
ASSERT_EQ(RMW_RET_OK, ret);
ASSERT_EQ(RMW_TOPIC_VALID, validation_result);

ASSERT_EQ((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result));
ASSERT_EQ((char *)nullptr, rmw_full_topic_name_validation_result_string(validation_result));
}

TEST(test_validate_topic_name, empty_topic_name) {
Expand All @@ -66,7 +66,7 @@ TEST(test_validate_topic_name, empty_topic_name) {
ASSERT_EQ(RMW_TOPIC_INVALID_IS_EMPTY_STRING, validation_result);
ASSERT_EQ(0ul, invalid_index);

ASSERT_NE((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result));
ASSERT_NE((char *)nullptr, rmw_full_topic_name_validation_result_string(validation_result));
}

TEST(test_validate_topic_name, not_absolute) {
Expand All @@ -90,7 +90,7 @@ TEST(test_validate_topic_name, not_absolute) {
ASSERT_EQ(RMW_TOPIC_INVALID_NOT_ABSOLUTE, validation_result);
ASSERT_EQ(0ul, invalid_index);

ASSERT_NE((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result));
ASSERT_NE((char *)nullptr, rmw_full_topic_name_validation_result_string(validation_result));
}

TEST(test_validate_topic_name, ends_with_forward_slash) {
Expand All @@ -114,7 +114,7 @@ TEST(test_validate_topic_name, ends_with_forward_slash) {
ASSERT_EQ(RMW_TOPIC_INVALID_ENDS_WITH_FORWARD_SLASH, validation_result);
ASSERT_EQ(0ul, invalid_index);

ASSERT_NE((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result));
ASSERT_NE((char *)nullptr, rmw_full_topic_name_validation_result_string(validation_result));
}

TEST(test_validate_topic_name, unallowed_characters) {
Expand Down Expand Up @@ -148,7 +148,7 @@ TEST(test_validate_topic_name, unallowed_characters) {
ASSERT_EQ(RMW_TOPIC_INVALID_CONTAINS_UNALLOWED_CHARACTERS, validation_result);
ASSERT_EQ(5ul, invalid_index);

ASSERT_NE((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result));
ASSERT_NE((char *)nullptr, rmw_full_topic_name_validation_result_string(validation_result));
}

TEST(test_validate_topic_name, repeated_forward_slashes) {
Expand All @@ -167,7 +167,7 @@ TEST(test_validate_topic_name, repeated_forward_slashes) {
ASSERT_EQ(RMW_TOPIC_INVALID_CONTAINS_REPEATED_FORWARD_SLASH, validation_result);
ASSERT_EQ(10ul, invalid_index);

ASSERT_NE((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result));
ASSERT_NE((char *)nullptr, rmw_full_topic_name_validation_result_string(validation_result));
}

TEST(test_validate_topic_name, starts_with_number) {
Expand All @@ -191,7 +191,7 @@ TEST(test_validate_topic_name, starts_with_number) {
ASSERT_EQ(RMW_TOPIC_INVALID_NAME_TOKEN_STARTS_WITH_NUMBER, validation_result);
ASSERT_EQ(8ul, invalid_index);

ASSERT_NE((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result));
ASSERT_NE((char *)nullptr, rmw_full_topic_name_validation_result_string(validation_result));
}

TEST(test_validate_topic_name, topic_too_long) {
Expand Down Expand Up @@ -228,5 +228,5 @@ TEST(test_validate_topic_name, topic_too_long) {
EXPECT_EQ(RMW_TOPIC_INVALID_TOO_LONG, validation_result);
EXPECT_EQ(RMW_TOPIC_MAX_NAME_LENGTH - 1, invalid_index);

ASSERT_NE((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result));
ASSERT_NE((char *)nullptr, rmw_full_topic_name_validation_result_string(validation_result));
}
16 changes: 8 additions & 8 deletions rmw/test/test_validate_namespace.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ TEST(test_validate_namespace, valid_namespace) {
ASSERT_EQ(RMW_RET_OK, ret);
ASSERT_EQ(RMW_NAMESPACE_VALID, validation_result);

ASSERT_EQ((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result));
ASSERT_EQ((char *)nullptr, rmw_namespace_validation_result_string(validation_result));
}

TEST(test_validate_namespace, empty_namespace) {
Expand All @@ -71,7 +71,7 @@ TEST(test_validate_namespace, empty_namespace) {
ASSERT_EQ(RMW_NAMESPACE_INVALID_IS_EMPTY_STRING, validation_result);
ASSERT_EQ(0ul, invalid_index);

ASSERT_NE((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result));
ASSERT_NE((char *)nullptr, rmw_namespace_validation_result_string(validation_result));
}

TEST(test_validate_namespace, not_absolute) {
Expand All @@ -95,7 +95,7 @@ TEST(test_validate_namespace, not_absolute) {
ASSERT_EQ(RMW_NAMESPACE_INVALID_NOT_ABSOLUTE, validation_result);
ASSERT_EQ(0ul, invalid_index);

ASSERT_NE((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result));
ASSERT_NE((char *)nullptr, rmw_namespace_validation_result_string(validation_result));
}

TEST(test_validate_namespace, ends_with_forward_slash) {
Expand All @@ -114,7 +114,7 @@ TEST(test_validate_namespace, ends_with_forward_slash) {
ASSERT_EQ(RMW_NAMESPACE_INVALID_ENDS_WITH_FORWARD_SLASH, validation_result);
ASSERT_EQ(10ul, invalid_index);

ASSERT_NE((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result));
ASSERT_NE((char *)nullptr, rmw_namespace_validation_result_string(validation_result));
}

TEST(test_validate_namespace, unallowed_characters) {
Expand Down Expand Up @@ -148,7 +148,7 @@ TEST(test_validate_namespace, unallowed_characters) {
ASSERT_EQ(RMW_NAMESPACE_INVALID_CONTAINS_UNALLOWED_CHARACTERS, validation_result);
ASSERT_EQ(5ul, invalid_index);

ASSERT_NE((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result));
ASSERT_NE((char *)nullptr, rmw_namespace_validation_result_string(validation_result));
}

TEST(test_validate_namespace, repeated_forward_slashes) {
Expand All @@ -167,7 +167,7 @@ TEST(test_validate_namespace, repeated_forward_slashes) {
ASSERT_EQ(RMW_NAMESPACE_INVALID_CONTAINS_REPEATED_FORWARD_SLASH, validation_result);
ASSERT_EQ(10ul, invalid_index);

ASSERT_NE((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result));
ASSERT_NE((char *)nullptr, rmw_namespace_validation_result_string(validation_result));
}

TEST(test_validate_namespace, starts_with_number) {
Expand All @@ -191,7 +191,7 @@ TEST(test_validate_namespace, starts_with_number) {
ASSERT_EQ(RMW_NAMESPACE_INVALID_NAME_TOKEN_STARTS_WITH_NUMBER, validation_result);
ASSERT_EQ(8ul, invalid_index);

ASSERT_NE((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result));
ASSERT_NE((char *)nullptr, rmw_namespace_validation_result_string(validation_result));
}

TEST(test_validate_namespace, topic_too_long) {
Expand Down Expand Up @@ -228,5 +228,5 @@ TEST(test_validate_namespace, topic_too_long) {
EXPECT_EQ(RMW_NAMESPACE_INVALID_TOO_LONG, validation_result);
EXPECT_EQ(RMW_NAMESPACE_MAX_LENGTH - 1, invalid_index);

ASSERT_NE((char *)NULL, rmw_full_topic_name_validation_result_string(validation_result));
ASSERT_NE((char *)nullptr, rmw_namespace_validation_result_string(validation_result));
}
10 changes: 5 additions & 5 deletions rmw/test/test_validate_node_name.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ TEST(test_validate_node_name, valid_node_name) {
ASSERT_EQ(RMW_RET_OK, ret);
ASSERT_EQ(RMW_NODE_NAME_VALID, validation_result);

ASSERT_EQ((char *)NULL, rmw_node_name_validation_result_string(validation_result));
ASSERT_EQ((char *)nullptr, rmw_node_name_validation_result_string(validation_result));
}

TEST(test_validate_node_name, empty_node_name) {
Expand All @@ -66,7 +66,7 @@ TEST(test_validate_node_name, empty_node_name) {
ASSERT_EQ(RMW_NODE_NAME_INVALID_IS_EMPTY_STRING, validation_result);
ASSERT_EQ(0ul, invalid_index);

ASSERT_NE((char *)NULL, rmw_node_name_validation_result_string(validation_result));
ASSERT_NE((char *)nullptr, rmw_node_name_validation_result_string(validation_result));
}

TEST(test_validate_node_name, unallowed_characters) {
Expand Down Expand Up @@ -105,7 +105,7 @@ TEST(test_validate_node_name, unallowed_characters) {
ASSERT_EQ(RMW_NODE_NAME_INVALID_CONTAINS_UNALLOWED_CHARACTERS, validation_result);
ASSERT_EQ(4ul, invalid_index);

ASSERT_NE((char *)NULL, rmw_node_name_validation_result_string(validation_result));
ASSERT_NE((char *)nullptr, rmw_node_name_validation_result_string(validation_result));
}

TEST(test_validate_node_name, starts_with_number) {
Expand All @@ -124,7 +124,7 @@ TEST(test_validate_node_name, starts_with_number) {
ASSERT_EQ(RMW_NODE_NAME_INVALID_STARTS_WITH_NUMBER, validation_result);
ASSERT_EQ(0ul, invalid_index);

ASSERT_NE((char *)NULL, rmw_node_name_validation_result_string(validation_result));
ASSERT_NE((char *)nullptr, rmw_node_name_validation_result_string(validation_result));
}

TEST(test_validate_node_name, node_name_too_long) {
Expand Down Expand Up @@ -162,5 +162,5 @@ TEST(test_validate_node_name, node_name_too_long) {
EXPECT_EQ(RMW_NODE_NAME_INVALID_TOO_LONG, validation_result);
EXPECT_EQ(RMW_NODE_NAME_MAX_NAME_LENGTH - 1U, invalid_index);

ASSERT_NE((char *)NULL, rmw_node_name_validation_result_string(validation_result));
ASSERT_NE((char *)nullptr, rmw_node_name_validation_result_string(validation_result));
}

0 comments on commit 589c69e

Please sign in to comment.