Keep domain id if ROS_DOMAIN_ID is invalid. (#689)

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
ros2 · Jun 22, 2020 · dd5e4ae · dd5e4ae
1 parent b3ab619
commit dd5e4ae
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 7 deletions.
diff --git a/rcl/src/rcl/domain_id.c b/rcl/src/rcl/domain_id.c
@@ -14,6 +14,7 @@
 
 #include "rcl/domain_id.h"
 
+#include <errno.h>
 #include <limits.h>
 
 #include "rcutils/get_env.h"
@@ -40,10 +41,15 @@ rcl_get_default_domain_id(size_t * domain_id)
       get_env_error_str);
     return RCL_RET_ERROR;
   }
-  if (ros_domain_id) {
-    unsigned long number = strtoul(ros_domain_id, NULL, 0);  // NOLINT(runtime/int)
-    if (number == ULONG_MAX) {
-      RCL_SET_ERROR_MSG("failed to interpret ROS_DOMAIN_ID as integral number");
+  if (ros_domain_id && strcmp(ros_domain_id, "") != 0) {
+    char * end = NULL;
+    unsigned long number = strtoul(ros_domain_id, &end, 0);  // NOLINT(runtime/int)
+    if (number == 0UL && *end != '\0') {
+      RCL_SET_ERROR_MSG("ROS_DOMAIN_ID is not an integral number");
+      return RCL_RET_ERROR;
+    }
+    if ((number == ULONG_MAX && errno == ERANGE) || number > SIZE_MAX) {
+      RCL_SET_ERROR_MSG("ROS_DOMAIN_ID is out of range");
       return RCL_RET_ERROR;
     }
     *domain_id = (size_t)number;

diff --git a/rcl/test/rcl/test_domain_id.cpp b/rcl/test/rcl/test_domain_id.cpp
@@ -22,15 +22,31 @@
 
 TEST(TestGetDomainId, test_nominal) {
   ASSERT_TRUE(rcutils_set_env("ROS_DOMAIN_ID", "42"));
-  size_t domain_id = 0u;
+  size_t domain_id = RCL_DEFAULT_DOMAIN_ID;
   EXPECT_EQ(RCL_RET_OK, rcl_get_default_domain_id(&domain_id));
   EXPECT_EQ(42u, domain_id);
 
+  ASSERT_TRUE(rcutils_set_env("ROS_DOMAIN_ID", ""));
+  domain_id = RCL_DEFAULT_DOMAIN_ID;
+  EXPECT_EQ(RCL_RET_OK, rcl_get_default_domain_id(&domain_id));
+  EXPECT_EQ(RCL_DEFAULT_DOMAIN_ID, domain_id);
+
+  ASSERT_TRUE(rcutils_set_env("ROS_DOMAIN_ID", "0000"));
+  domain_id = RCL_DEFAULT_DOMAIN_ID;
+  EXPECT_EQ(RCL_RET_OK, rcl_get_default_domain_id(&domain_id));
+  EXPECT_EQ(0u, domain_id);
+
+  ASSERT_TRUE(rcutils_set_env("ROS_DOMAIN_ID", "0   not really"));
+  domain_id = RCL_DEFAULT_DOMAIN_ID;
+  EXPECT_EQ(RCL_RET_ERROR, rcl_get_default_domain_id(&domain_id));
+  rcl_reset_error();
+  EXPECT_EQ(RCL_DEFAULT_DOMAIN_ID, domain_id);
+
   ASSERT_TRUE(rcutils_set_env("ROS_DOMAIN_ID", "998446744073709551615"));
-  domain_id = 0u;
+  domain_id = RCL_DEFAULT_DOMAIN_ID;
   EXPECT_EQ(RCL_RET_ERROR, rcl_get_default_domain_id(&domain_id));
   rcl_reset_error();
-  EXPECT_EQ(0u, domain_id);
+  EXPECT_EQ(RCL_DEFAULT_DOMAIN_ID, domain_id);
 
   EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_get_default_domain_id(nullptr));
 }