Delete rcl_impl_getenv, replaced by rcutils_get_env (#502)

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>
ros2 · Sep 25, 2019 · 3e0efce · 3e0efce
1 parent 9196cb8
commit 3e0efce
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 123 deletions.
diff --git a/rcl/src/rcl/common.c b/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)
 {

diff --git a/rcl/src/rcl/node.c b/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);
- if (ret != RCL_RET_OK) {
+ get_env_error_str = rcutils_get_env(ROS_DOMAIN_ID_VAR_NAME, &ros_domain_id);
+ 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)) {
- RCL_SET_ERROR_MSG(
- "Environment variable " RCUTILS_STRINGIFY(ROS_SECURITY_ENABLE_VAR_NAME)
- " could not be read");
+ get_env_error_str = rcutils_get_env(ROS_SECURITY_ENABLE_VAR_NAME, &ros_security_enable);
+ if (NULL != get_env_error_str) {
+ RCL_SET_ERROR_MSG_WITH_FORMAT_STRING(
+ "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)) {
- RCL_SET_ERROR_MSG(
- "Environment variable " RCUTILS_STRINGIFY(ROS_SECURITY_STRATEGY_VAR_NAME)
- " could not be read");
+ get_env_error_str = rcutils_get_env(ROS_SECURITY_STRATEGY_VAR_NAME, &ros_enforce_security);
+ if (NULL != get_env_error_str) {
+ RCL_SET_ERROR_MSG_WITH_FORMAT_STRING(
+ "Error getting env var '" RCUTILS_STRINGIFY(ROS_SECURITY_STRATEGY_VAR_NAME) "': %s\n",
+ get_env_error_str);
  ret = RCL_RET_ERROR;
  goto fail;
  }

diff --git a/rcl/src/rcl/rmw_implementation_identifier_check.c b/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);
- if (ret != RCL_RET_OK) {
+ const char * get_env_error_str = rcutils_get_env(
+ 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",
- rcl_get_error_string().str
- );
- exit(ret);
+ "Error getting env var '" RCUTILS_STRINGIFY(RMW_IMPLEMENTATION_ENV_VAR_NAME) "': %s\n",
+ get_env_error_str);
+ exit(RCL_RET_ERROR);
  }
  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);
- if (ret != RCL_RET_OK) {
+ get_env_error_str = rcutils_get_env(
+ 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",
- rcl_get_error_string().str
- );
- exit(ret);
+ "Error getting env var '"
+ RCUTILS_STRINGIFY(RCL_ASSERT_RMW_ID_MATCHES_ENV_VAR_NAME) "': %s\n",
+  get_env_error_str);
+ 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.

diff --git a/rcl/test/CMakeLists.txt b/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}

diff --git a/rcl/test/rcl/test_common.cpp b/rcl/test/rcl/test_common.cpp