diff --git a/rcl/src/rcl/common.c b/rcl/src/rcl/common.c index 50307aea0..5e1a2997d 100644 --- 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 index 2a31abe33..d6ad72098 100644 --- 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 index 47c53d289..e66ee2168 100644 --- 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 index f000ce509..fe0642225 100644 --- 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 deleted file mode 100644 index 2d09381f7..000000000 --- a/rcl/test/rcl/test_common.cpp +++ /dev/null @@ -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 - -#include - -#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)); -}