Delete rcl_impl_getenv, replaced by rcutils_get_env (#502)

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

rcl/src/rcl/common.c

@@ -24,35 +24,6 @@ extern "C"
 #include "rcl/allocator.h"
 #include "rcl/error_handling.h"
 
-#if defined(_WIN32)
-# define WINDOWS_ENV_BUFFER_SIZE 2048
-static char __env_buffer[WINDOWS_ENV_BUFFER_SIZE];
-#endif  // defined(_WIN32)
-
-rcl_ret_t
-rcl_impl_getenv(const char * env_name, const char ** env_value)
-{
-  RCL_CHECK_ARGUMENT_FOR_NULL(env_name, RCL_RET_INVALID_ARGUMENT);
-  RCL_CHECK_ARGUMENT_FOR_NULL(env_value, RCL_RET_INVALID_ARGUMENT);
-  *env_value = NULL;
-#if !defined(_WIN32)
-  *env_value = getenv(env_name);
-  if (*env_value == NULL) {
-    *env_value = "";
-  }
-#else  // !defined(_WIN32)
-  size_t required_size;
-  errno_t ret = getenv_s(&required_size, __env_buffer, sizeof(__env_buffer), env_name);
-  if (ret != 0) {
-    RCL_SET_ERROR_MSG("value in env variable too large to read in");
-    return RCL_RET_ERROR;
-  }
-  __env_buffer[WINDOWS_ENV_BUFFER_SIZE - 1] = '\0';
-  *env_value = __env_buffer;
-#endif  // !defined(_WIN32)
-  return RCL_RET_OK;
-}
-
 rcl_ret_t
 rcl_convert_rmw_ret_to_rcl_ret(rmw_ret_t rmw_ret)
 {

rcl/src/rcl/node.c

@@ -45,11 +45,12 @@ extern "C"
 #include "rmw/validate_namespace.h"
 #include "rmw/validate_node_name.h"
 
-#include "./common.h"
 #include "./context_impl.h"
 
 #define ROS_SECURITY_STRATEGY_VAR_NAME "ROS_SECURITY_STRATEGY"
 #define ROS_SECURITY_ENABLE_VAR_NAME "ROS_SECURITY_ENABLE"
+#define ROS_DOMAIN_ID_VAR_NAME "ROS_DOMAIN_ID"
+
 
 typedef struct rcl_node_impl_t
 {
@@ -120,6 +121,7 @@ rcl_node_init(
 {
   size_t domain_id = 0;
   const char * ros_domain_id;
+  const char * get_env_error_str;
   const rmw_guard_condition_t * rmw_graph_guard_condition = NULL;
   rcl_guard_condition_options_t graph_guard_condition_options =
     rcl_guard_condition_get_default_options();
@@ -256,8 +258,11 @@ rcl_node_init(
   // node rmw_node_handle
   if (node->impl->options.domain_id == RCL_NODE_OPTIONS_DEFAULT_DOMAIN_ID) {
     // Find the domain ID set by the environment.
-    ret = rcl_impl_getenv("ROS_DOMAIN_ID", &ros_domain_id);
+    get_env_error_str = rcutils_get_env(ROS_DOMAIN_ID_VAR_NAME, &ros_domain_id);
-    if (ret != RCL_RET_OK) {
+    if (NULL != get_env_error_str) {
+      RCL_SET_ERROR_MSG_WITH_FORMAT_STRING(
+        "Error getting env var '" RCUTILS_STRINGIFY(ROS_DOMAIN_ID_VAR_NAME) "': %s\n",
+        get_env_error_str);
       goto fail;
     }
     if (ros_domain_id) {
@@ -278,10 +283,11 @@ rcl_node_init(
   const char * ros_security_enable = NULL;
   const char * ros_enforce_security = NULL;
 
-  if (rcutils_get_env(ROS_SECURITY_ENABLE_VAR_NAME, &ros_security_enable)) {
+  get_env_error_str = rcutils_get_env(ROS_SECURITY_ENABLE_VAR_NAME, &ros_security_enable);
-    RCL_SET_ERROR_MSG(
+  if (NULL != get_env_error_str) {
-      "Environment variable " RCUTILS_STRINGIFY(ROS_SECURITY_ENABLE_VAR_NAME)
+    RCL_SET_ERROR_MSG_WITH_FORMAT_STRING(
-      " could not be read");
+      "Error getting env var '" RCUTILS_STRINGIFY(ROS_SECURITY_ENABLE_VAR_NAME) "': %s\n",
+      get_env_error_str);
     ret = RCL_RET_ERROR;
     goto fail;
   }
@@ -290,10 +296,11 @@ rcl_node_init(
   RCUTILS_LOG_DEBUG_NAMED(
     ROS_PACKAGE_NAME, "Using security: %s", use_security ? "true" : "false");
 
-  if (rcutils_get_env(ROS_SECURITY_STRATEGY_VAR_NAME, &ros_enforce_security)) {
+  get_env_error_str = rcutils_get_env(ROS_SECURITY_STRATEGY_VAR_NAME, &ros_enforce_security);
-    RCL_SET_ERROR_MSG(
+  if (NULL != get_env_error_str) {
-      "Environment variable " RCUTILS_STRINGIFY(ROS_SECURITY_STRATEGY_VAR_NAME)
+    RCL_SET_ERROR_MSG_WITH_FORMAT_STRING(
-      " could not be read");
+      "Error getting env var '" RCUTILS_STRINGIFY(ROS_SECURITY_STRATEGY_VAR_NAME) "': %s\n",
+      get_env_error_str);
     ret = RCL_RET_ERROR;
     goto fail;
   }

rcl/src/rcl/rmw_implementation_identifier_check.c

@@ -23,12 +23,15 @@ extern "C"
 
 #include "rcl/allocator.h"
 #include "rcl/error_handling.h"
+#include "rcutils/get_env.h"
 #include "rcutils/logging_macros.h"
 #include "rcutils/strdup.h"
 #include "rmw/rmw.h"
 
 #include "rcl/types.h"
-#include "./common.h"
+
+#define RMW_IMPLEMENTATION_ENV_VAR_NAME "RMW_IMPLEMENTATION"
+#define RCL_ASSERT_RMW_ID_MATCHES_ENV_VAR_NAME "RCL_ASSERT_RMW_ID_MATCHES"
 
 // Extracted this portable method of doing a "shared library constructor" from SO:
 //   http://stackoverflow.com/a/2390626/671658
@@ -59,14 +62,15 @@ INITIALIZER(initialize) {
   rcl_allocator_t allocator = rcl_get_default_allocator();
   char * expected_rmw_impl = NULL;
   const char * expected_rmw_impl_env = NULL;
-  rcl_ret_t ret = rcl_impl_getenv("RMW_IMPLEMENTATION", &expected_rmw_impl_env);
+  const char * get_env_error_str = rcutils_get_env(
-  if (ret != RCL_RET_OK) {
+    RMW_IMPLEMENTATION_ENV_VAR_NAME,
+    &expected_rmw_impl_env);
+  if (NULL != get_env_error_str) {
     RCUTILS_LOG_ERROR_NAMED(
       ROS_PACKAGE_NAME,
-      "Error getting environment variable 'RMW_IMPLEMENTATION': %s",
+      "Error getting env var '" RCUTILS_STRINGIFY(RMW_IMPLEMENTATION_ENV_VAR_NAME) "': %s\n",
-      rcl_get_error_string().str
+      get_env_error_str);
-    );
+    exit(RCL_RET_ERROR);
-    exit(ret);
   }
   if (strlen(expected_rmw_impl_env) > 0) {
     // Copy the environment variable so it doesn't get over-written by the next getenv call.
@@ -79,14 +83,15 @@ INITIALIZER(initialize) {
 
   char * asserted_rmw_impl = NULL;
   const char * asserted_rmw_impl_env = NULL;
-  ret = rcl_impl_getenv("RCL_ASSERT_RMW_ID_MATCHES", &asserted_rmw_impl_env);
+  get_env_error_str = rcutils_get_env(
-  if (ret != RCL_RET_OK) {
+    RCL_ASSERT_RMW_ID_MATCHES_ENV_VAR_NAME, &asserted_rmw_impl_env);
+  if (NULL != get_env_error_str) {
     RCUTILS_LOG_ERROR_NAMED(
       ROS_PACKAGE_NAME,
-      "Error getting environment variable 'RCL_ASSERT_RMW_ID_MATCHES': %s",
+      "Error getting env var '"
-      rcl_get_error_string().str
+      RCUTILS_STRINGIFY(RCL_ASSERT_RMW_ID_MATCHES_ENV_VAR_NAME) "': %s\n",
-    );
+      get_env_error_str);
-    exit(ret);
+    exit(RCL_RET_ERROR);
   }
   if (strlen(asserted_rmw_impl_env) > 0) {
     // Copy the environment variable so it doesn't get over-written by the next getenv call.

rcl/test/CMakeLists.txt

@@ -50,17 +50,6 @@ function(test_target_function)
     AMENT_DEPENDENCIES ${rmw_implementation}
   )
 
-  rcl_add_custom_gtest(test_common${target_suffix}
-    SRCS rcl/test_common.cpp
-    ENV
-      ${rmw_implementation_env_var}
-      EMPTY_TEST=
-      NORMAL_TEST=foo
-    APPEND_LIBRARY_DIRS ${extra_lib_dirs}
-    LIBRARIES ${PROJECT_NAME}
-    AMENT_DEPENDENCIES ${rmw_implementation}
-  )
-
   rcl_add_custom_gtest(test_context${target_suffix}
     SRCS rcl/test_context.cpp
     ENV ${rmw_implementation_env_var} ${memory_tools_ld_preload_env_var}

rcl/test/rcl/test_common.cpp

@@ -1,59 +0,0 @@
-// Copyright 2015 Open Source Robotics Foundation, Inc.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-#include <gtest/gtest.h>
-
-#include <string>
-
-#include "../../src/rcl/common.h"
-#include "../../src/rcl/common.c"
-
-#ifdef RMW_IMPLEMENTATION
-# define CLASSNAME_(NAME, SUFFIX) NAME ## __ ## SUFFIX
-# define CLASSNAME(NAME, SUFFIX) CLASSNAME_(NAME, SUFFIX)
-#else
-# define CLASSNAME(NAME, SUFFIX) NAME
-#endif
-
-/* Tests the default allocator.
- *
- * Expected environment variables must be set by the calling code:
- *
- *   - EMPTY_TEST=
- *   - NORMAL_TEST=foo
- *
- * These are set in the call to `ament_add_gtest()` in the `CMakeLists.txt`.
- */
-TEST(CLASSNAME(TestCommon, RMW_IMPLEMENTATION), test_getenv) {
-  const char * env;
-  rcl_ret_t ret;
-  ret = rcl_impl_getenv("NORMAL_TEST", NULL);
-  EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, ret);
-  rcl_reset_error();
-  ret = rcl_impl_getenv(NULL, &env);
-  EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, ret);
-  rcl_reset_error();
-  ret = rcl_impl_getenv("SHOULD_NOT_EXIST_TEST", &env);
-  EXPECT_EQ(RCL_RET_OK, ret);
-  EXPECT_EQ("", std::string(env)) << std::string(env);
-  rcl_reset_error();
-  ret = rcl_impl_getenv("NORMAL_TEST", &env);
-  EXPECT_EQ(RCL_RET_OK, ret);
-  ASSERT_NE(nullptr, env);
-  EXPECT_EQ("foo", std::string(env));
-  ret = rcl_impl_getenv("EMPTY_TEST", &env);
-  EXPECT_EQ(RCL_RET_OK, ret);
-  ASSERT_NE(nullptr, env);
-  EXPECT_EQ("", std::string(env));
-}