From f12437ac87c5d10132dcd9a96c548185b48dae98 Mon Sep 17 00:00:00 2001 From: Ethan Gao Date: Tue, 28 Nov 2017 22:51:35 +0800 Subject: [PATCH 1/5] adapt to NULL removal from rmw result valiation string Signed-off-by: Ethan Gao --- rcl/src/rcl/node.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/rcl/src/rcl/node.c b/rcl/src/rcl/node.c index e87058679..55dfb2c0b 100644 --- a/rcl/src/rcl/node.c +++ b/rcl/src/rcl/node.c @@ -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; } @@ -169,15 +166,11 @@ 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); - } + char fixed_msg[256]; + rcutils_snprintf( + fixed_msg, sizeof(fixed_msg), + "%s: '%d'", msg, validation_result); + RCL_SET_ERROR_MSG(fixed_msg, *allocator); if (should_free_local_namespace_) { allocator->deallocate((char *)local_namespace_, allocator->state); From e15bd7bb520ee9afce1472c88ae93f5442ea268c Mon Sep 17 00:00:00 2001 From: Ethan Gao Date: Wed, 29 Nov 2017 23:32:53 +0800 Subject: [PATCH 2/5] adapt to rmw validation change and update rcl topic validation Signed-off-by: Ethan Gao --- rcl/src/rcl/node.c | 4 +--- rcl/src/rcl/validate_topic_name.c | 4 +++- rcl/test/rcl/test_validate_topic_name.cpp | 10 +++++----- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/rcl/src/rcl/node.c b/rcl/src/rcl/node.c index 55dfb2c0b..f07b7b1f7 100644 --- a/rcl/src/rcl/node.c +++ b/rcl/src/rcl/node.c @@ -167,9 +167,7 @@ rcl_node_init( if (validation_result != RMW_NAMESPACE_VALID) { const char * msg = rmw_namespace_validation_result_string(validation_result); char fixed_msg[256]; - rcutils_snprintf( - fixed_msg, sizeof(fixed_msg), - "%s: '%d'", msg, validation_result); + rcutils_snprintf(fixed_msg, sizeof(fixed_msg), "%s, result '%d'", msg, validation_result); RCL_SET_ERROR_MSG(fixed_msg, *allocator); if (should_free_local_namespace_) { diff --git a/rcl/src/rcl/validate_topic_name.c b/rcl/src/rcl/validate_topic_name.c index e7a987881..8c756b9c8 100644 --- a/rcl/src/rcl/validate_topic_name.c +++ b/rcl/src/rcl/validate_topic_name.c @@ -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: @@ -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 "undefined topic name type"; } } diff --git a/rcl/test/rcl/test_validate_topic_name.cpp b/rcl/test/rcl/test_validate_topic_name.cpp index 31b11f48d..017568ba5 100644 --- a/rcl/test/rcl/test_validate_topic_name.cpp +++ b/rcl/test/rcl/test_validate_topic_name.cpp @@ -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 @@ -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 @@ -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(); } From 518a6767d3b88be0c588bd7f1150baca39c9d7f7 Mon Sep 17 00:00:00 2001 From: Ethan Gao Date: Thu, 30 Nov 2017 18:23:01 +0800 Subject: [PATCH 3/5] tweak the default error string returned Signed-off-by: Ethan Gao --- rcl/src/rcl/validate_topic_name.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rcl/src/rcl/validate_topic_name.c b/rcl/src/rcl/validate_topic_name.c index 8c756b9c8..a1b1346ff 100644 --- a/rcl/src/rcl/validate_topic_name.c +++ b/rcl/src/rcl/validate_topic_name.c @@ -215,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 "undefined topic name type"; + return "unknown result code for rcl topic name validation"; } } From b3bcfbae8eab24c37a134c73627faec1ce297245 Mon Sep 17 00:00:00 2001 From: Ethan Gao Date: Mon, 4 Dec 2017 22:33:25 +0800 Subject: [PATCH 4/5] tweak error output format Signed-off-by: Ethan Gao --- rcl/src/rcl/node.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/rcl/src/rcl/node.c b/rcl/src/rcl/node.c index f07b7b1f7..6d9cfddfd 100644 --- a/rcl/src/rcl/node.c +++ b/rcl/src/rcl/node.c @@ -166,9 +166,7 @@ rcl_node_init( } if (validation_result != RMW_NAMESPACE_VALID) { const char * msg = rmw_namespace_validation_result_string(validation_result); - char fixed_msg[256]; - rcutils_snprintf(fixed_msg, sizeof(fixed_msg), "%s, result '%d'", msg, validation_result); - RCL_SET_ERROR_MSG(fixed_msg, *allocator); + RCL_SET_ERROR_MSG_WITH_FORMAT_STRING(*allocator, "%s, result '%d'", msg, validation_result); if (should_free_local_namespace_) { allocator->deallocate((char *)local_namespace_, allocator->state); From 5011aed9889a16c3c71dd817e85771a2535b69dd Mon Sep 17 00:00:00 2001 From: Ethan Gao Date: Thu, 21 Dec 2017 18:32:40 +0800 Subject: [PATCH 5/5] fix build error Signed-off-by: Ethan Gao --- rcl/src/rcl/node.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rcl/src/rcl/node.c b/rcl/src/rcl/node.c index 6d9cfddfd..981973f9f 100644 --- a/rcl/src/rcl/node.c +++ b/rcl/src/rcl/node.c @@ -27,6 +27,7 @@ extern "C" #include "rcl/error_handling.h" #include "rcl/rcl.h" #include "rcutils/filesystem.h" +#include "rcutils/format_string.h" #include "rcutils/get_env.h" #include "rcutils/logging_macros.h" #include "rcutils/macros.h" @@ -166,7 +167,7 @@ rcl_node_init( } if (validation_result != RMW_NAMESPACE_VALID) { const char * msg = rmw_namespace_validation_result_string(validation_result); - RCL_SET_ERROR_MSG_WITH_FORMAT_STRING(*allocator, "%s, result '%d'", msg, validation_result); + RCL_SET_ERROR_MSG_WITH_FORMAT_STRING((*allocator), "%s, result: %d", msg, validation_result); if (should_free_local_namespace_) { allocator->deallocate((char *)local_namespace_, allocator->state);