From 7a29617747ed3c39a1b1edc9c808e100523e4fc7 Mon Sep 17 00:00:00 2001
From: Michel Hidalgo <michel@ekumenlabs.com>
Date: Mon, 22 Jun 2020 12:08:27 -0300
Subject: [PATCH] Keep domain id if ROS_DOMAIN_ID is invalid. (#689)

Including domain id tests.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
---
 rcl/src/rcl/domain_id.c         | 14 ++++++++++----
 rcl/test/CMakeLists.txt         |  6 ++++++
 rcl/test/rcl/test_domain_id.cpp | 22 +++++++++++++++++++---
 3 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/rcl/src/rcl/domain_id.c b/rcl/src/rcl/domain_id.c
index 84b7052817..07f586995f 100644
--- 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/CMakeLists.txt b/rcl/test/CMakeLists.txt
index 1c1a92ec6d..f8692e8079 100644
--- a/rcl/test/CMakeLists.txt
+++ b/rcl/test/CMakeLists.txt
@@ -344,6 +344,12 @@ rcl_add_custom_gtest(test_validate_topic_name
   LIBRARIES ${PROJECT_NAME}
 )
 
+rcl_add_custom_gtest(test_domain_id
+  SRCS rcl/test_domain_id.cpp
+  APPEND_LIBRARY_DIRS ${extra_lib_dirs}
+  LIBRARIES ${PROJECT_NAME}
+)
+
 rcl_add_custom_gtest(test_expand_topic_name
   SRCS rcl/test_expand_topic_name.cpp
   APPEND_LIBRARY_DIRS ${extra_lib_dirs}
diff --git a/rcl/test/rcl/test_domain_id.cpp b/rcl/test/rcl/test_domain_id.cpp
index fc964df211..b5d4616bc2 100644
--- 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));
 }