From 589c69efc50ccf9fb6e7b7b0be08f41d0d5ec6bc Mon Sep 17 00:00:00 2001 From: Ethan Gao <16472154+gaoethan@users.noreply.github.com> Date: Mon, 8 Jan 2018 23:31:29 +0800 Subject: [PATCH] Optimize namespace node and topic validation (#130) * 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 * no NULL returned from validation string and twaek the tests accordingly Signed-off-by: Ethan Gao * keep NULL for the valid case and adapt tests Signed-off-by: Ethan Gao * tweak the default error string returned Signed-off-by: Ethan Gao --- rmw/src/validate_full_topic_name.c | 2 +- rmw/src/validate_namespace.c | 2 +- rmw/src/validate_node_name.c | 2 +- rmw/test/test_validate_full_topic_name.cpp | 16 ++++++++-------- rmw/test/test_validate_namespace.cpp | 16 ++++++++-------- rmw/test/test_validate_node_name.cpp | 10 +++++----- 6 files changed, 24 insertions(+), 24 deletions(-) diff --git a/rmw/src/validate_full_topic_name.c b/rmw/src/validate_full_topic_name.c index 9a24a236..8b59825d 100644 --- a/rmw/src/validate_full_topic_name.c +++ b/rmw/src/validate_full_topic_name.c @@ -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"; } } diff --git a/rmw/src/validate_namespace.c b/rmw/src/validate_namespace.c index c708cb05..3a7a23f7 100644 --- a/rmw/src/validate_namespace.c +++ b/rmw/src/validate_namespace.c @@ -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"; } } diff --git a/rmw/src/validate_node_name.c b/rmw/src/validate_node_name.c index df223fc7..f86335c2 100644 --- a/rmw/src/validate_node_name.c +++ b/rmw/src/validate_node_name.c @@ -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"; } } diff --git a/rmw/test/test_validate_full_topic_name.cpp b/rmw/test/test_validate_full_topic_name.cpp index c3def82d..dfb1f77f 100644 --- a/rmw/test/test_validate_full_topic_name.cpp +++ b/rmw/test/test_validate_full_topic_name.cpp @@ -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) { @@ -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) { @@ -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) { @@ -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) { @@ -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) { @@ -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) { @@ -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) { @@ -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)); } diff --git a/rmw/test/test_validate_namespace.cpp b/rmw/test/test_validate_namespace.cpp index d17300fe..13c32a4a 100644 --- a/rmw/test/test_validate_namespace.cpp +++ b/rmw/test/test_validate_namespace.cpp @@ -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) { @@ -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) { @@ -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) { @@ -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) { @@ -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) { @@ -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) { @@ -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) { @@ -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)); } diff --git a/rmw/test/test_validate_node_name.cpp b/rmw/test/test_validate_node_name.cpp index 2d6f5837..da56fdbe 100644 --- a/rmw/test/test_validate_node_name.cpp +++ b/rmw/test/test_validate_node_name.cpp @@ -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) { @@ -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) { @@ -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) { @@ -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) { @@ -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)); }