From 6985fc04df082e40687dbe3ae81009ba0b16a497 Mon Sep 17 00:00:00 2001 From: William Woodall Date: Fri, 11 Dec 2015 11:25:46 -0800 Subject: [PATCH] add tests for, and clean up, common --- rcl/CMakeLists.txt | 16 +++++++++++ rcl/src/rcl/common.c | 10 ++++--- rcl/src/rcl/common.h | 20 ++++++++++++-- rcl/src/rcl/node.c | 2 +- rcl/test/rcl/test_common.cpp | 51 ++++++++++++++++++++++++++++++++++++ 5 files changed, 93 insertions(+), 6 deletions(-) create mode 100644 rcl/test/rcl/test_common.cpp diff --git a/rcl/CMakeLists.txt b/rcl/CMakeLists.txt index c60faf3..b600c5b 100644 --- a/rcl/CMakeLists.txt +++ b/rcl/CMakeLists.txt @@ -89,6 +89,22 @@ if(AMENT_ENABLE_TESTING) set_target_properties(test_time PROPERTIES COMPILE_FLAGS "-std=c++11") target_link_libraries(test_time ${PROJECT_NAME} ${extra_test_libraries}) endif() + + ament_add_gtest(test_common + test/rcl/test_common.cpp + ENV + DYLD_INSERT_LIBRARIES=$ + EMPTY_TEST= + NORMAL_TEST=foo + ) + if(TARGET test_common) + target_include_directories(test_common PUBLIC + ${rcl_interfaces_INCLUDE_DIRS} + ${rmw_INCLUDE_DIRS} + ) + set_target_properties(test_common PROPERTIES COMPILE_FLAGS "-std=c++11") + target_link_libraries(test_common ${PROJECT_NAME} ${extra_test_libraries}) + endif() endif() ament_package() diff --git a/rcl/src/rcl/common.c b/rcl/src/rcl/common.c index 8a22c43..cae778c 100644 --- a/rcl/src/rcl/common.c +++ b/rcl/src/rcl/common.c @@ -22,12 +22,15 @@ extern "C" #include #if defined(WIN32) -static char __env_buffer[1024]; +#define WINDOWS_ENV_BUFFER_SIZE 2048 +static char __env_buffer[WINDOWS_ENV_BUFFER_SIZE]; #endif rcl_ret_t -rcl_impl_getenv(const char * env_name, char ** env_value) +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); @@ -38,7 +41,8 @@ rcl_impl_getenv(const char * env_name, char ** env_value) RCL_SET_ERROR_MSG("value in env variable too large to read in"); return RCL_RET_ERROR; } - env_value = __env_buffer; + __env_buffer[WINDOWS_ENV_BUFFER_SIZE - 1] = '\0'; + *env_value = __env_buffer; #endif return RCL_RET_OK; } diff --git a/rcl/src/rcl/common.h b/rcl/src/rcl/common.h index 03bf242..b27232f 100644 --- a/rcl/src/rcl/common.h +++ b/rcl/src/rcl/common.h @@ -31,9 +31,25 @@ extern "C" error_statement; \ } -// This value put in env_value is only valid until the next call to rcl_impl_getenv (on Windows). +/// Retrieve the value of the given environment variable if it exists, or NULL. +/* The returned char is only valid until the next time this function is called, + * because the returned char * is a direct pointer to the static storage. + * The returned value char * variable should never have free() called on it. + * + * Environment variable values will be truncated at 2048 characters on Windows. + * + * This function does not allocate heap memory, but the system calls might. + * This function is not thread-safe. + * This function is not lock-free. + * + * \param[in] env_name the name of the environment variable + * \param[out] env_value pointer to the value cstring + * \return RCL_RET_OK if the value is retrieved successfully, or + * RCL_RET_INVALID_ARGUMENT if any arguments are invalid, or + * RCL_RET_ERROR an unspecified error occur. + */ rcl_ret_t -rcl_impl_getenv(const char * env_name, char ** env_value); +rcl_impl_getenv(const char * env_name, const char ** env_value); #if __cplusplus } diff --git a/rcl/src/rcl/node.c b/rcl/src/rcl/node.c index 57dab8e..5ef8a87 100644 --- a/rcl/src/rcl/node.c +++ b/rcl/src/rcl/node.c @@ -46,7 +46,7 @@ rcl_ret_t rcl_node_init(rcl_node_t * node, const char * name, const rcl_node_options_t * options) { size_t domain_id = 0; - char * ros_domain_id; + const char * ros_domain_id; rcl_ret_t ret; rcl_ret_t fail_ret = RCL_RET_ERROR; RCL_CHECK_ARGUMENT_FOR_NULL(name, RCL_RET_INVALID_ARGUMENT); diff --git a/rcl/test/rcl/test_common.cpp b/rcl/test/rcl/test_common.cpp new file mode 100644 index 0000000..b4732eb --- /dev/null +++ b/rcl/test/rcl/test_common.cpp @@ -0,0 +1,51 @@ +// 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" + +/* 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(TestCommon, test_getenv) { + const char * env; + rcl_ret_t ret; + ret = rcl_impl_getenv("NORMAL_TEST", NULL); + EXPECT_EQ(ret, RCL_RET_INVALID_ARGUMENT); + rcl_reset_error(); + ret = rcl_impl_getenv(NULL, &env); + EXPECT_EQ(ret, RCL_RET_INVALID_ARGUMENT); + rcl_reset_error(); + ret = rcl_impl_getenv("SHOULD_NOT_EXIST_TEST", &env); + EXPECT_EQ(ret, RCL_RET_OK); + EXPECT_EQ(env, nullptr) << std::string(env); + rcl_reset_error(); + ret = rcl_impl_getenv("NORMAL_TEST", &env); + EXPECT_EQ(ret, RCL_RET_OK); + ASSERT_NE(env, nullptr); + EXPECT_EQ(std::string(env), "foo"); + ret = rcl_impl_getenv("EMPTY_TEST", &env); + EXPECT_EQ(ret, RCL_RET_OK); + ASSERT_NE(env, nullptr); + EXPECT_EQ(std::string(env), ""); +}