From 268533b34aa9e5045705c4bdda281588468f4b55 Mon Sep 17 00:00:00 2001 From: ahcorde Date: Tue, 10 Mar 2020 10:39:58 +0100 Subject: [PATCH 01/35] Included utils to load, unload and get symbols from shared libraries Shared library working on Windows Signed-off-by: ahcorde --- CMakeLists.txt | 3 ++ include/rcutils/shared_library.h | 79 +++++++++++++++++++++++++++ src/shared_library.c | 93 ++++++++++++++++++++++++++++++++ 3 files changed, 175 insertions(+) create mode 100644 include/rcutils/shared_library.h create mode 100644 src/shared_library.c diff --git a/CMakeLists.txt b/CMakeLists.txt index 82172f32..9b2272fb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -42,6 +42,7 @@ set(rcutils_sources src/logging.c src/process.c src/repl_str.c + src/shared_library.c src/snprintf.c src/split.c src/strdup.c @@ -99,6 +100,8 @@ add_library( # which is appropriate when building the dll but not consuming it. target_compile_definitions(${PROJECT_NAME} PRIVATE "RCUTILS_BUILDING_DLL") +target_link_libraries(${PROJECT_NAME} ${CMAKE_DL_LIBS}) + # Needed if pthread is used for thread local storage. if(IOS AND IOS_SDK_VERSION LESS 10.0) ament_export_libraries(pthread) diff --git a/include/rcutils/shared_library.h b/include/rcutils/shared_library.h new file mode 100644 index 00000000..034e4a12 --- /dev/null +++ b/include/rcutils/shared_library.h @@ -0,0 +1,79 @@ +// Copyright 2020 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. + +#ifndef RCUTILS__SHARED_LIBRARY_H_ +#define RCUTILS__SHARED_LIBRARY_H_ + +#ifdef __cplusplus +extern "C" +{ +#endif + +#include + +#ifndef _WIN32 +#include +#else +#include +#endif // _WIN32 + +#include "rcutils/macros.h" +#include "rcutils/visibility_control.h" + +/// The structure +typedef struct rcutils_shared_library_t +{ + /// The pointer to the shared library. + #ifndef _WIN32 + void * lib_pointer; + #else + HINSTANCE lib_pointer; + #endif + /// The path of the shared_library + char * library_path; +} rcutils_shared_library_t; + +/// Return shared library pointer. +/** + * \param[in] library_path path to the library + * \return void* shared library pointer. + * nullptr if library doesn't exist + */ +RCUTILS_PUBLIC +rcutils_shared_library_t * +rcutils_get_shared_library(const char * library_path); + +/// Return shared library symbol pointer. +/** + * \param[in] symbol_name name of the symbol inside the shared library + * \return void* symbol pointer. + * nullptr if symbol doesn't exist + */ +RCUTILS_PUBLIC +void * +rcutils_get_symbol(rcutils_shared_library_t * lib, const char * symbol_name); + +/// Unload the shared library. +/** + * \param[in] lib struct to the shared library + */ +RCUTILS_PUBLIC +void +rcutils_unload_library(rcutils_shared_library_t * lib); + +#ifdef __cplusplus +} +#endif + +#endif // RCUTILS__SHARED_LIBRARY_H_ diff --git a/src/shared_library.c b/src/shared_library.c new file mode 100644 index 00000000..e1b5c551 --- /dev/null +++ b/src/shared_library.c @@ -0,0 +1,93 @@ +// Copyright 2020 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. + +#ifdef __cplusplus +extern "C" +{ +#endif +#include "rcutils/shared_library.h" + +#include +#include + +rcutils_shared_library_t * +rcutils_get_shared_library(const char * library_path) +{ + rcutils_shared_library_t * lib = (rcutils_shared_library_t *) + malloc(sizeof(rcutils_shared_library_t)); + +#ifndef _WIN32 + lib->lib_pointer = dlopen(library_path, RTLD_LAZY); +#else + lib->lib_pointer = LoadLibrary(library_path); +#endif // _WIN32 + if (!lib->lib_pointer) { + free(lib); + return NULL; + } + + // +1 to accomodate for the null terminator + lib->library_path = (char *) malloc((strlen(library_path) + 1) * sizeof(char)); + snprintf(lib->library_path, strlen(library_path), "%s", library_path); + + return lib; +} + +void * +rcutils_get_symbol(rcutils_shared_library_t * lib, const char * symbol_name) +{ +#ifndef _WIN32 + void * lib_symbol = dlsym(lib->lib_pointer, symbol_name); + const char * dlsym_error = dlerror(); + if (dlsym_error) { + fprintf( + stderr, "Cannot load symbol '%s' in shared library '%s'", + symbol_name, lib->library_path); + dlclose(lib->lib_pointer); + free(lib->library_path); + free(lib); + return NULL; + } +#else + void * lib_symbol = GetProcAddress(lib->lib_pointer, symbol_name); + if (!lib_symbol) { + fprintf( + stderr, "Cannot load symbol '%s' in shared library '%s'", + symbol_name, lib->library_path); + FreeLibrary(lib->lib_pointer); + free(lib->library_path); + free(lib); + return NULL; + } +#endif // _WIN32 + return lib_symbol; +} + +void +rcutils_unload_library(rcutils_shared_library_t * lib) +{ +#ifndef _WIN32 + dlclose(lib->lib_pointer); + free(lib->library_path); + free(lib); +#else + FreeLibrary(lib->lib_pointer); + free(lib->library_path); + free(lib); +#endif // _WIN32 +} + +#ifdef __cplusplus +} +#endif From 076723ea1bcb2cd5850c76d2a81a5c0c990cef83 Mon Sep 17 00:00:00 2001 From: ahcorde Date: Tue, 10 Mar 2020 17:41:31 +0100 Subject: [PATCH 02/35] platform independent code moved outside of the conditional blocks Signed-off-by: ahcorde --- src/shared_library.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/shared_library.c b/src/shared_library.c index e1b5c551..5e3853c8 100644 --- a/src/shared_library.c +++ b/src/shared_library.c @@ -79,13 +79,11 @@ rcutils_unload_library(rcutils_shared_library_t * lib) { #ifndef _WIN32 dlclose(lib->lib_pointer); - free(lib->library_path); - free(lib); #else FreeLibrary(lib->lib_pointer); +#endif // _WIN32 free(lib->library_path); free(lib); -#endif // _WIN32 } #ifdef __cplusplus From e87960b5380bf5838e4be1e46b170ee9fc05ab3f Mon Sep 17 00:00:00 2001 From: ahcorde Date: Wed, 11 Mar 2020 16:30:10 +0100 Subject: [PATCH 03/35] Included feedback Signed-off-by: ahcorde Added shared library test fix return error on shared library for windows Signed-off-by: ahcorde --- CMakeLists.txt | 15 +++ include/rcutils/shared_library.h | 81 ++++++++++--- src/shared_library.c | 109 +++++++++++------- .../dummy_shared_library.cpp | 48 ++++++++ test/test_shared_library.cpp | 79 +++++++++++++ 5 files changed, 274 insertions(+), 58 deletions(-) create mode 100644 test/dummy_shared_library/dummy_shared_library.cpp create mode 100644 test/test_shared_library.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 9b2272fb..b4fe5cca 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -315,6 +315,21 @@ if(BUILD_TESTING) target_link_libraries(test_repl_str ${PROJECT_NAME}) endif() + if(WIN32) + set(append_library_dirs "$") + else() + set(append_library_dirs "${CMAKE_CURRENT_BINARY_DIR}") + endif() + + ament_add_gtest(test_shared_library test/test_shared_library.cpp + APPEND_LIBRARY_DIRS "${append_library_dirs}" + ) + + if(TARGET test_shared_library) + add_library(dummy_shared_library test/dummy_shared_library/dummy_shared_library.cpp) + target_link_libraries(test_shared_library ${PROJECT_NAME}) + endif() + rcutils_custom_add_gtest(test_time test/test_time.cpp ENV ${memory_tools_test_env_vars}) diff --git a/include/rcutils/shared_library.h b/include/rcutils/shared_library.h index 034e4a12..d3b480fd 100644 --- a/include/rcutils/shared_library.h +++ b/include/rcutils/shared_library.h @@ -24,53 +24,98 @@ extern "C" #ifndef _WIN32 #include +typedef void * shared_library_type; #else #include +typedef HINSTANCE shared_library_type; #endif // _WIN32 +#include "rcutils/allocator.h" +#include "rcutils/types/rcutils_ret.h" #include "rcutils/macros.h" #include "rcutils/visibility_control.h" -/// The structure -typedef struct rcutils_shared_library_t +/// Handle to a loaded shared library. +typedef struct RCUTILS_PUBLIC_TYPE rcutils_shared_library_t { - /// The pointer to the shared library. - #ifndef _WIN32 - void * lib_pointer; - #else - HINSTANCE lib_pointer; - #endif + /// The pointer to the shared library + shared_library_type lib_pointer; /// The path of the shared_library char * library_path; } rcutils_shared_library_t; +/// Return an empty shared library struct. +/* + * This function returns an empty and zero initialized shared library struct. + * + * Example: + * + * ```c + * // Do not do this: + * // rcutils_shared_library_t foo; + * // rcutils_allocator_t allocator = rcutils_get_default_allocator(); + * // rcutils_unload_library(&foo, allocator); // undefined behavior! + * + * // Do this instead: + * rcutils_shared_library_t bar = rcutils_get_zero_initialized_shared_library(); + * rcutils_allocator_t allocator = rcutils_get_default_allocator(); + * rcutils_unload_library(&bar, allocator); // ok + * ``` + * */ +RCUTILS_PUBLIC +RCUTILS_WARN_UNUSED +rcutils_shared_library_t +rcutils_get_zero_initialized_shared_library(void); + /// Return shared library pointer. /** - * \param[in] library_path path to the library - * \return void* shared library pointer. - * nullptr if library doesn't exist + * + * the memory of the library path inside the rcutils_shared_library_t struct should be + * reserved and defined outside this function + * + * \param[in] lib struct with the shared library pointer and shared library path name + * \return `RCUTILS_RET_OK` if successful, or + * \return `RCUTILS_RET_ERROR` if an unknown error occurs */ RCUTILS_PUBLIC -rcutils_shared_library_t * -rcutils_get_shared_library(const char * library_path); +RCUTILS_WARN_UNUSED +rcutils_ret_t +rcutils_get_shared_library(rcutils_shared_library_t * lib); /// Return shared library symbol pointer. /** + * \param[in] lib struct with the shared library pointer and shared library path name * \param[in] symbol_name name of the symbol inside the shared library - * \return void* symbol pointer. - * nullptr if symbol doesn't exist + * \return shared library symbol pointer. */ RCUTILS_PUBLIC +RCUTILS_WARN_UNUSED void * rcutils_get_symbol(rcutils_shared_library_t * lib, const char * symbol_name); +/// Return if the shared library contains a specific symbolname . +/** + * \param[in] lib struct with the shared library pointer and shared library path name + * \param[in] symbol_name name of the symbol inside the shared library + * \return returns true on success, and false otherwise. + */ +RCUTILS_PUBLIC +RCUTILS_WARN_UNUSED +bool +rcutils_has_symbol(rcutils_shared_library_t * lib, const char * symbol_name); + /// Unload the shared library. /** - * \param[in] lib struct to the shared library + * \param[inout] lib rcutils_shared_library_t to be finalized + * \param[in] allocator the allocator to use for allocation + * \return `RCUTILS_RET_OK` if successful, or + * \return `RCUTILS_RET_INVALID_ARGUMENT` for invalid arguments, or + * \return `RCUTILS_RET_ERROR` if an unknown error occurs */ RCUTILS_PUBLIC -void -rcutils_unload_library(rcutils_shared_library_t * lib); +RCUTILS_WARN_UNUSED +rcutils_ret_t +rcutils_unload_library(rcutils_shared_library_t * lib, rcutils_allocator_t allocator); #ifdef __cplusplus } diff --git a/src/shared_library.c b/src/shared_library.c index 5e3853c8..8e3211ed 100644 --- a/src/shared_library.c +++ b/src/shared_library.c @@ -16,32 +16,43 @@ extern "C" { #endif -#include "rcutils/shared_library.h" - #include #include -rcutils_shared_library_t * -rcutils_get_shared_library(const char * library_path) +#include "rcutils/shared_library.h" +#include "rcutils/error_handling.h" + + +rcutils_shared_library_t +rcutils_get_zero_initialized_shared_library(void) { - rcutils_shared_library_t * lib = (rcutils_shared_library_t *) - malloc(sizeof(rcutils_shared_library_t)); + rcutils_shared_library_t zero_initialized_shared_library; + zero_initialized_shared_library.library_path = NULL; + zero_initialized_shared_library.lib_pointer = NULL; + return zero_initialized_shared_library; +} + +rcutils_ret_t +rcutils_get_shared_library(rcutils_shared_library_t * lib) +{ + RCUTILS_CHECK_ARGUMENT_FOR_NULL(lib, RCUTILS_RET_INVALID_ARGUMENT); + RCUTILS_CHECK_ARGUMENT_FOR_NULL(lib->library_path, RCUTILS_RET_INVALID_ARGUMENT); #ifndef _WIN32 - lib->lib_pointer = dlopen(library_path, RTLD_LAZY); + lib->lib_pointer = dlopen(lib->library_path, RTLD_LAZY); + if (!lib->lib_pointer) { + RCUTILS_SET_ERROR_MSG(dlerror); + return RCUTILS_RET_ERROR; + } #else - lib->lib_pointer = LoadLibrary(library_path); -#endif // _WIN32 + lib->lib_pointer = LoadLibrary(lib->library_path); if (!lib->lib_pointer) { - free(lib); - return NULL; + DWORD error = GetLastError(); + RCUTILS_SET_ERROR_MSG("LoadLibrary error"); + return RCUTILS_RET_ERROR; } - - // +1 to accomodate for the null terminator - lib->library_path = (char *) malloc((strlen(library_path) + 1) * sizeof(char)); - snprintf(lib->library_path, strlen(library_path), "%s", library_path); - - return lib; +#endif // _WIN32 + return RCUTILS_RET_OK; } void * @@ -49,41 +60,59 @@ rcutils_get_symbol(rcutils_shared_library_t * lib, const char * symbol_name) { #ifndef _WIN32 void * lib_symbol = dlsym(lib->lib_pointer, symbol_name); - const char * dlsym_error = dlerror(); - if (dlsym_error) { - fprintf( - stderr, "Cannot load symbol '%s' in shared library '%s'", - symbol_name, lib->library_path); - dlclose(lib->lib_pointer); - free(lib->library_path); - free(lib); - return NULL; - } #else void * lib_symbol = GetProcAddress(lib->lib_pointer, symbol_name); +#endif // _WIN32 if (!lib_symbol) { - fprintf( - stderr, "Cannot load symbol '%s' in shared library '%s'", - symbol_name, lib->library_path); - FreeLibrary(lib->lib_pointer); - free(lib->library_path); - free(lib); return NULL; } -#endif // _WIN32 return lib_symbol; } -void -rcutils_unload_library(rcutils_shared_library_t * lib) +bool +rcutils_has_symbol(rcutils_shared_library_t * lib, const char * symbol_name) +{ +#ifndef _WIN32 + void * lib_symbol = dlsym(lib->lib_pointer, symbol_name); +#else + void * lib_symbol = GetProcAddress(lib->lib_pointer, symbol_name); +#endif // _WIN32 + if (!lib_symbol) { + return false; + } + return true; +} + +rcutils_ret_t +rcutils_unload_library(rcutils_shared_library_t * lib, rcutils_allocator_t allocator) { + RCUTILS_CHECK_ARGUMENT_FOR_NULL(lib, RCUTILS_RET_INVALID_ARGUMENT); + RCUTILS_CHECK_ARGUMENT_FOR_NULL(lib->lib_pointer, RCUTILS_RET_INVALID_ARGUMENT); + RCUTILS_CHECK_ARGUMENT_FOR_NULL(lib->library_path, RCUTILS_RET_INVALID_ARGUMENT); + + rcutils_ret_t ret = RCUTILS_RET_OK; + int error_code = 0; #ifndef _WIN32 - dlclose(lib->lib_pointer); + // The function dlclose() returns 0 on success, and nonzero on error. + error_code = dlclose(lib->lib_pointer); + const char * dlsym_error = dlerror(); + if (dlsym_error) { + RCUTILS_SET_ERROR_MSG(dlerror()); + ret = RCUTILS_RET_ERROR; + } #else - FreeLibrary(lib->lib_pointer); + // zero if the function succeeds + error_code = FreeLibrary(lib->lib_pointer); + if (!error_code) { + RCUTILS_SET_ERROR_MSG(GetLastError()); + ret = RCUTILS_RET_ERROR; + } #endif // _WIN32 - free(lib->library_path); - free(lib); + + allocator.deallocate(lib->library_path, allocator.state); + lib->library_path = NULL; + lib->lib_pointer = NULL; + return ret; } #ifdef __cplusplus diff --git a/test/dummy_shared_library/dummy_shared_library.cpp b/test/dummy_shared_library/dummy_shared_library.cpp new file mode 100644 index 00000000..06855717 --- /dev/null +++ b/test/dummy_shared_library/dummy_shared_library.cpp @@ -0,0 +1,48 @@ +// Copyright 2020 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. + +#ifndef DUMMY_SHARED_LIBRARY__DUMMY_SHARED_LIBRARY_BASE_HPP_ +#define DUMMY_SHARED_LIBRARY__DUMMY_SHARED_LIBRARY_BASE_HPP_ + +#include + +class DummySharedLibraryBase +{ +public: + virtual ~DummySharedLibraryBase() {} + virtual void speak() = 0; +}; + +class Bar : public DummySharedLibraryBase +{ +public: + virtual ~Bar() = default; + void speak() + { + printf("from plugin Bar\n"); + } +}; + +class Baz : public DummySharedLibraryBase +{ +public: + virtual ~Baz() = default; + void speak() + { + printf("from plugin Baz"); + } +}; + + +#endif // DUMMY_SHARED_LIBRARY__DUMMY_SHARED_LIBRARY_BASE_HPP_ diff --git a/test/test_shared_library.cpp b/test/test_shared_library.cpp new file mode 100644 index 00000000..a1ec38f2 --- /dev/null +++ b/test/test_shared_library.cpp @@ -0,0 +1,79 @@ +// Copyright 2020 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 "./allocator_testing_utils.h" + +#include "rcutils/allocator.h" +#include "rcutils/error_handling.h" +#include "rcutils/shared_library.h" + +#include "rcutils/get_env.h" + +class TestSharedLibrary : public ::testing::Test +{ +protected: + void SetUp() final + { + // Reset rcutil error global state in case a previously + // running test has failed. + rcutils_reset_error(); + allocator = rcutils_get_default_allocator(); + lib = rcutils_get_zero_initialized_shared_library(); + } + rcutils_allocator_t allocator; + rcutils_shared_library_t lib; +}; + +TEST_F(TestSharedLibrary, getters_initialize_to_zero) { + rcutils_ret_t ret; + + // checking rcutils_get_zero_initialized_shared_library + ASSERT_STRNE(lib.library_path, ""); + EXPECT_TRUE(lib.lib_pointer == NULL); + + // Getting RMW_IMPLEMENTATION env var to get the shared library + const char * env_var{}; + const char * err = rcutils_get_env("RMW_IMPLEMENTATION", &env_var); + // Is there any error getting the env var? + ASSERT_STRNE(err, ""); + // library path is not empty + const std::string library_path = std::string("libdummy_shared_library.so"); + EXPECT_FALSE(library_path.empty()); + + // allocating memory to + lib.library_path = reinterpret_cast(allocator.allocate( + (library_path.length() + 1) * sizeof(char), + allocator.state)); + + // checking allocation was fine + ASSERT_NE(lib.library_path, nullptr); + // copying string + snprintf(lib.library_path, library_path.length() + 1, "%s", library_path.c_str()); + + // getting shared library + ret = rcutils_get_shared_library(&lib); + ASSERT_EQ(RCUTILS_RET_OK, ret); + + // unload shared_library + ret = rcutils_unload_library(&lib, allocator); + ASSERT_EQ(RCUTILS_RET_OK, ret); + + // checking if we have unloaded and freed memory + ASSERT_STRNE(lib.library_path, ""); + EXPECT_TRUE(lib.lib_pointer == NULL); +} From 696e0ea5391779f3bfd5761933347895bae8ae63 Mon Sep 17 00:00:00 2001 From: ahcorde Date: Thu, 12 Mar 2020 09:37:33 +0100 Subject: [PATCH 04/35] Exported CMake_DL_LIBS Signed-off-by: ahcorde --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index b4fe5cca..15dc24e3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -368,7 +368,7 @@ endif() ament_export_dependencies(ament_cmake) ament_export_include_directories(include) -ament_export_libraries(${PROJECT_NAME}) +ament_export_libraries(${PROJECT_NAME} ${CMAKE_DL_LIBS}) ament_package() install( From 955e9aef51a81b63409fee623acc227712451066 Mon Sep 17 00:00:00 2001 From: ahcorde Date: Thu, 12 Mar 2020 10:35:19 +0100 Subject: [PATCH 05/35] Included more feedback Signed-off-by: ahcorde --- include/rcutils/shared_library.h | 34 ++++++++++---------- src/shared_library.c | 54 ++++++++++++++++++++++---------- 2 files changed, 55 insertions(+), 33 deletions(-) diff --git a/include/rcutils/shared_library.h b/include/rcutils/shared_library.h index d3b480fd..579e5b15 100644 --- a/include/rcutils/shared_library.h +++ b/include/rcutils/shared_library.h @@ -24,10 +24,10 @@ extern "C" #ifndef _WIN32 #include -typedef void * shared_library_type; +typedef void * rcutils_shared_library_handle_t; #else #include -typedef HINSTANCE shared_library_type; +typedef HINSTANCE rcutils_shared_library_handle_t; #endif // _WIN32 #include "rcutils/allocator.h" @@ -39,9 +39,11 @@ typedef HINSTANCE shared_library_type; typedef struct RCUTILS_PUBLIC_TYPE rcutils_shared_library_t { /// The pointer to the shared library - shared_library_type lib_pointer; + rcutils_shared_library_handle_t lib_pointer; /// The path of the shared_library char * library_path; + /// allocator + rcutils_allocator_t allocator; } rcutils_shared_library_t; /// Return an empty shared library struct. @@ -59,7 +61,7 @@ typedef struct RCUTILS_PUBLIC_TYPE rcutils_shared_library_t * // Do this instead: * rcutils_shared_library_t bar = rcutils_get_zero_initialized_shared_library(); * rcutils_allocator_t allocator = rcutils_get_default_allocator(); - * rcutils_unload_library(&bar, allocator); // ok + * rcutils_load_shared_library(&bar, "library_name"); // ok * ``` * */ RCUTILS_PUBLIC @@ -69,45 +71,43 @@ rcutils_get_zero_initialized_shared_library(void); /// Return shared library pointer. /** - * - * the memory of the library path inside the rcutils_shared_library_t struct should be - * reserved and defined outside this function - * - * \param[in] lib struct with the shared library pointer and shared library path name + * \param[inout] lib struct with the shared library pointer and shared library path name + * \param[in] library_path string with the path of the library * \return `RCUTILS_RET_OK` if successful, or - * \return `RCUTILS_RET_ERROR` if an unknown error occurs + * \return `RCUTILS_RET_BAD_ALLOC` if memory allocation fails, or + * \return `RCUTILS_RET_ERROR` if an unknown error occurs, or + * \return `RCUTILS_RET_INVALID_ARGUMENT` for invalid arguments */ RCUTILS_PUBLIC RCUTILS_WARN_UNUSED rcutils_ret_t -rcutils_get_shared_library(rcutils_shared_library_t * lib); +rcutils_load_shared_library(rcutils_shared_library_t * lib, const char * library_path); /// Return shared library symbol pointer. /** * \param[in] lib struct with the shared library pointer and shared library path name * \param[in] symbol_name name of the symbol inside the shared library - * \return shared library symbol pointer. + * \return shared library symbol pointer, if the symbol doesn't exist then returns NULL. */ RCUTILS_PUBLIC RCUTILS_WARN_UNUSED void * -rcutils_get_symbol(rcutils_shared_library_t * lib, const char * symbol_name); +rcutils_get_symbol(const rcutils_shared_library_t * lib, const char * symbol_name); /// Return if the shared library contains a specific symbolname . /** * \param[in] lib struct with the shared library pointer and shared library path name * \param[in] symbol_name name of the symbol inside the shared library - * \return returns true on success, and false otherwise. + * \return if symbols exists returns true, otherwise returns false. */ RCUTILS_PUBLIC RCUTILS_WARN_UNUSED bool -rcutils_has_symbol(rcutils_shared_library_t * lib, const char * symbol_name); +rcutils_has_symbol(const rcutils_shared_library_t * lib, const char * symbol_name); /// Unload the shared library. /** * \param[inout] lib rcutils_shared_library_t to be finalized - * \param[in] allocator the allocator to use for allocation * \return `RCUTILS_RET_OK` if successful, or * \return `RCUTILS_RET_INVALID_ARGUMENT` for invalid arguments, or * \return `RCUTILS_RET_ERROR` if an unknown error occurs @@ -115,7 +115,7 @@ rcutils_has_symbol(rcutils_shared_library_t * lib, const char * symbol_name); RCUTILS_PUBLIC RCUTILS_WARN_UNUSED rcutils_ret_t -rcutils_unload_library(rcutils_shared_library_t * lib, rcutils_allocator_t allocator); +rcutils_unload_library(rcutils_shared_library_t * lib); #ifdef __cplusplus } diff --git a/src/shared_library.c b/src/shared_library.c index 8e3211ed..3f415272 100644 --- a/src/shared_library.c +++ b/src/shared_library.c @@ -29,26 +29,38 @@ rcutils_get_zero_initialized_shared_library(void) rcutils_shared_library_t zero_initialized_shared_library; zero_initialized_shared_library.library_path = NULL; zero_initialized_shared_library.lib_pointer = NULL; + zero_initialized_shared_library.allocator = rcutils_get_default_allocator(); return zero_initialized_shared_library; } rcutils_ret_t -rcutils_get_shared_library(rcutils_shared_library_t * lib) +rcutils_load_shared_library(rcutils_shared_library_t * lib, const char * library_path) { RCUTILS_CHECK_ARGUMENT_FOR_NULL(lib, RCUTILS_RET_INVALID_ARGUMENT); - RCUTILS_CHECK_ARGUMENT_FOR_NULL(lib->library_path, RCUTILS_RET_INVALID_ARGUMENT); + + // allocating memory to + lib->library_path = (char *)(lib->allocator.allocate( + (strlen(library_path) + 1) * sizeof(char), + lib->allocator.state)); + + if (!lib->library_path) { + RCUTILS_SET_ERROR_MSG("unable to allocate memory"); + return RCUTILS_RET_BAD_ALLOC; + } + + // copying string + snprintf(lib->library_path, strlen(library_path) + 1, "%s", library_path); #ifndef _WIN32 lib->lib_pointer = dlopen(lib->library_path, RTLD_LAZY); if (!lib->lib_pointer) { - RCUTILS_SET_ERROR_MSG(dlerror); + RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("dlclose error: %s", dlerror()); return RCUTILS_RET_ERROR; } #else lib->lib_pointer = LoadLibrary(lib->library_path); if (!lib->lib_pointer) { - DWORD error = GetLastError(); - RCUTILS_SET_ERROR_MSG("LoadLibrary error"); + RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("LoadLibrary error: %lu", GetLastError()); return RCUTILS_RET_ERROR; } #endif // _WIN32 @@ -56,22 +68,34 @@ rcutils_get_shared_library(rcutils_shared_library_t * lib) } void * -rcutils_get_symbol(rcutils_shared_library_t * lib, const char * symbol_name) +rcutils_get_symbol(const rcutils_shared_library_t * lib, const char * symbol_name) { + if (!lib || !lib->lib_pointer || (symbol_name == NULL)) { + RCUTILS_SET_ERROR_MSG("invalid inputs arguments"); + return NULL; + } + #ifndef _WIN32 void * lib_symbol = dlsym(lib->lib_pointer, symbol_name); #else void * lib_symbol = GetProcAddress(lib->lib_pointer, symbol_name); #endif // _WIN32 if (!lib_symbol) { + RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( + "symbol '%s' doesnt' exit in library '%s'", + symbol_name, lib->library_path); return NULL; } return lib_symbol; } bool -rcutils_has_symbol(rcutils_shared_library_t * lib, const char * symbol_name) +rcutils_has_symbol(const rcutils_shared_library_t * lib, const char * symbol_name) { + if (!lib || !lib->lib_pointer || symbol_name) { + return false; + } + #ifndef _WIN32 void * lib_symbol = dlsym(lib->lib_pointer, symbol_name); #else @@ -84,32 +108,30 @@ rcutils_has_symbol(rcutils_shared_library_t * lib, const char * symbol_name) } rcutils_ret_t -rcutils_unload_library(rcutils_shared_library_t * lib, rcutils_allocator_t allocator) +rcutils_unload_library(rcutils_shared_library_t * lib) { RCUTILS_CHECK_ARGUMENT_FOR_NULL(lib, RCUTILS_RET_INVALID_ARGUMENT); RCUTILS_CHECK_ARGUMENT_FOR_NULL(lib->lib_pointer, RCUTILS_RET_INVALID_ARGUMENT); RCUTILS_CHECK_ARGUMENT_FOR_NULL(lib->library_path, RCUTILS_RET_INVALID_ARGUMENT); rcutils_ret_t ret = RCUTILS_RET_OK; - int error_code = 0; #ifndef _WIN32 // The function dlclose() returns 0 on success, and nonzero on error. - error_code = dlclose(lib->lib_pointer); - const char * dlsym_error = dlerror(); - if (dlsym_error) { - RCUTILS_SET_ERROR_MSG(dlerror()); + int error_code = dlclose(lib->lib_pointer); + if (error_code) { + RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("dlclose error: %s", dlerror()); ret = RCUTILS_RET_ERROR; } #else // zero if the function succeeds - error_code = FreeLibrary(lib->lib_pointer); + int error_code = FreeLibrary(lib->lib_pointer); if (!error_code) { - RCUTILS_SET_ERROR_MSG(GetLastError()); + RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("FreeLibrary error: %lu", GetLastError()); ret = RCUTILS_RET_ERROR; } #endif // _WIN32 - allocator.deallocate(lib->library_path, allocator.state); + lib->allocator.deallocate(lib->library_path, lib->allocator.state); lib->library_path = NULL; lib->lib_pointer = NULL; return ret; From d1926b718a10ce464d8e7fce2dbb1cd39465a6d3 Mon Sep 17 00:00:00 2001 From: ahcorde Date: Thu, 12 Mar 2020 10:35:44 +0100 Subject: [PATCH 06/35] Improved tests Signed-off-by: ahcorde --- CMakeLists.txt | 2 +- .../dummy_shared_library.c | 20 +++++++ ...red_library.cpp => dummy_shared_library.h} | 45 ++++++---------- test/test_shared_library.cpp | 53 +++++++++++-------- 4 files changed, 67 insertions(+), 53 deletions(-) create mode 100644 test/dummy_shared_library/dummy_shared_library.c rename test/dummy_shared_library/{dummy_shared_library.cpp => dummy_shared_library.h} (51%) diff --git a/CMakeLists.txt b/CMakeLists.txt index 15dc24e3..3c28af7d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -326,7 +326,7 @@ if(BUILD_TESTING) ) if(TARGET test_shared_library) - add_library(dummy_shared_library test/dummy_shared_library/dummy_shared_library.cpp) + add_library(dummy_shared_library test/dummy_shared_library/dummy_shared_library.c) target_link_libraries(test_shared_library ${PROJECT_NAME}) endif() diff --git a/test/dummy_shared_library/dummy_shared_library.c b/test/dummy_shared_library/dummy_shared_library.c new file mode 100644 index 00000000..7da551d1 --- /dev/null +++ b/test/dummy_shared_library/dummy_shared_library.c @@ -0,0 +1,20 @@ +// Copyright 2020 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 "./dummy_shared_library.h" // NOLINT + +void print_name() +{ + printf("print_name\n"); +} diff --git a/test/dummy_shared_library/dummy_shared_library.cpp b/test/dummy_shared_library/dummy_shared_library.h similarity index 51% rename from test/dummy_shared_library/dummy_shared_library.cpp rename to test/dummy_shared_library/dummy_shared_library.h index 06855717..8c724b79 100644 --- a/test/dummy_shared_library/dummy_shared_library.cpp +++ b/test/dummy_shared_library/dummy_shared_library.h @@ -12,37 +12,22 @@ // See the License for the specific language governing permissions and // limitations under the License. -#ifndef DUMMY_SHARED_LIBRARY__DUMMY_SHARED_LIBRARY_BASE_HPP_ -#define DUMMY_SHARED_LIBRARY__DUMMY_SHARED_LIBRARY_BASE_HPP_ +#ifndef DUMMY_SHARED_LIBRARY__DUMMY_SHARED_LIBRARY_H_ +#define DUMMY_SHARED_LIBRARY__DUMMY_SHARED_LIBRARY_H_ -#include +#if _WIN32 +#ifdef DUMMY_SHARED_LIBRARY_BUILDING_DLL +#define DUMMY_SHARED_LIBRARY_PUBLIC __declspec(dllexport) +#else +#define DUMMY_SHARED_LIBRARY_PUBLIC __declspec(dllimport) +#endif +#else +#define DUMMY_SHARED_LIBRARY_PUBLIC +#endif -class DummySharedLibraryBase -{ -public: - virtual ~DummySharedLibraryBase() {} - virtual void speak() = 0; -}; +#include -class Bar : public DummySharedLibraryBase -{ -public: - virtual ~Bar() = default; - void speak() - { - printf("from plugin Bar\n"); - } -}; +DUMMY_SHARED_LIBRARY_PUBLIC +void print_foo(); -class Baz : public DummySharedLibraryBase -{ -public: - virtual ~Baz() = default; - void speak() - { - printf("from plugin Baz"); - } -}; - - -#endif // DUMMY_SHARED_LIBRARY__DUMMY_SHARED_LIBRARY_BASE_HPP_ +#endif // DUMMY_SHARED_LIBRARY__DUMMY_SHARED_LIBRARY_H_ diff --git a/test/test_shared_library.cpp b/test/test_shared_library.cpp index a1ec38f2..4a39faf3 100644 --- a/test/test_shared_library.cpp +++ b/test/test_shared_library.cpp @@ -32,48 +32,57 @@ class TestSharedLibrary : public ::testing::Test // Reset rcutil error global state in case a previously // running test has failed. rcutils_reset_error(); - allocator = rcutils_get_default_allocator(); lib = rcutils_get_zero_initialized_shared_library(); } - rcutils_allocator_t allocator; rcutils_shared_library_t lib; }; -TEST_F(TestSharedLibrary, getters_initialize_to_zero) { +TEST_F(TestSharedLibrary, basic_load) { rcutils_ret_t ret; // checking rcutils_get_zero_initialized_shared_library ASSERT_STRNE(lib.library_path, ""); EXPECT_TRUE(lib.lib_pointer == NULL); - // Getting RMW_IMPLEMENTATION env var to get the shared library - const char * env_var{}; - const char * err = rcutils_get_env("RMW_IMPLEMENTATION", &env_var); - // Is there any error getting the env var? - ASSERT_STRNE(err, ""); - // library path is not empty + // library path const std::string library_path = std::string("libdummy_shared_library.so"); - EXPECT_FALSE(library_path.empty()); - - // allocating memory to - lib.library_path = reinterpret_cast(allocator.allocate( - (library_path.length() + 1) * sizeof(char), - allocator.state)); - - // checking allocation was fine - ASSERT_NE(lib.library_path, nullptr); - // copying string - snprintf(lib.library_path, library_path.length() + 1, "%s", library_path.c_str()); // getting shared library - ret = rcutils_get_shared_library(&lib); + ret = rcutils_load_shared_library(&lib, library_path.c_str()); ASSERT_EQ(RCUTILS_RET_OK, ret); // unload shared_library - ret = rcutils_unload_library(&lib, allocator); + ret = rcutils_unload_library(&lib); ASSERT_EQ(RCUTILS_RET_OK, ret); // checking if we have unloaded and freed memory ASSERT_STRNE(lib.library_path, ""); EXPECT_TRUE(lib.lib_pointer == NULL); } + +TEST_F(TestSharedLibrary, basic_symbol) { + void * symbol; + bool ret; + + symbol = rcutils_get_symbol(nullptr, "symbol"); + EXPECT_TRUE(symbol == NULL); + + ret = rcutils_has_symbol(nullptr, "symbol"); + EXPECT_FALSE(ret); + + const std::string library_path = std::string("libdummy_shared_library.so"); + + // getting shared library + ret = rcutils_load_shared_library(&lib, library_path.c_str()); + ASSERT_EQ(RCUTILS_RET_OK, ret); + + symbol = rcutils_get_symbol(&lib, "symbol"); + EXPECT_TRUE(symbol == NULL); + + symbol = rcutils_get_symbol(&lib, "print_name"); + EXPECT_TRUE(symbol != NULL); + + // unload shared_library + ret = rcutils_unload_library(&lib); + ASSERT_EQ(RCUTILS_RET_OK, ret); +} From 277b748f3eb18a1c373c7195639fce803ebc103e Mon Sep 17 00:00:00 2001 From: ahcorde Date: Fri, 13 Mar 2020 11:55:56 +0100 Subject: [PATCH 07/35] Fixed argument checks in rcutils_has_symbol Signed-off-by: ahcorde --- src/shared_library.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared_library.c b/src/shared_library.c index 3f415272..7cff5ad1 100644 --- a/src/shared_library.c +++ b/src/shared_library.c @@ -92,7 +92,7 @@ rcutils_get_symbol(const rcutils_shared_library_t * lib, const char * symbol_nam bool rcutils_has_symbol(const rcutils_shared_library_t * lib, const char * symbol_name) { - if (!lib || !lib->lib_pointer || symbol_name) { + if (!lib || !lib->lib_pointer || symbol_name == NULL) { return false; } From c91089ca5c79911c5af43dc111e49e1e444f68df Mon Sep 17 00:00:00 2001 From: ahcorde Date: Fri, 13 Mar 2020 12:28:12 +0100 Subject: [PATCH 08/35] fixed naming rcutils_unload_library to rcutils_unload_shared_library Signed-off-by: ahcorde --- include/rcutils/shared_library.h | 4 ++-- src/shared_library.c | 2 +- test/test_shared_library.cpp | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/include/rcutils/shared_library.h b/include/rcutils/shared_library.h index 579e5b15..06164a19 100644 --- a/include/rcutils/shared_library.h +++ b/include/rcutils/shared_library.h @@ -56,7 +56,7 @@ typedef struct RCUTILS_PUBLIC_TYPE rcutils_shared_library_t * // Do not do this: * // rcutils_shared_library_t foo; * // rcutils_allocator_t allocator = rcutils_get_default_allocator(); - * // rcutils_unload_library(&foo, allocator); // undefined behavior! + * // rcutils_unload_shared_library(&foo, allocator); // undefined behavior! * * // Do this instead: * rcutils_shared_library_t bar = rcutils_get_zero_initialized_shared_library(); @@ -115,7 +115,7 @@ rcutils_has_symbol(const rcutils_shared_library_t * lib, const char * symbol_nam RCUTILS_PUBLIC RCUTILS_WARN_UNUSED rcutils_ret_t -rcutils_unload_library(rcutils_shared_library_t * lib); +rcutils_unload_shared_library(rcutils_shared_library_t * lib); #ifdef __cplusplus } diff --git a/src/shared_library.c b/src/shared_library.c index 7cff5ad1..9d149a98 100644 --- a/src/shared_library.c +++ b/src/shared_library.c @@ -108,7 +108,7 @@ rcutils_has_symbol(const rcutils_shared_library_t * lib, const char * symbol_nam } rcutils_ret_t -rcutils_unload_library(rcutils_shared_library_t * lib) +rcutils_unload_shared_library(rcutils_shared_library_t * lib) { RCUTILS_CHECK_ARGUMENT_FOR_NULL(lib, RCUTILS_RET_INVALID_ARGUMENT); RCUTILS_CHECK_ARGUMENT_FOR_NULL(lib->lib_pointer, RCUTILS_RET_INVALID_ARGUMENT); diff --git a/test/test_shared_library.cpp b/test/test_shared_library.cpp index 4a39faf3..137bcb92 100644 --- a/test/test_shared_library.cpp +++ b/test/test_shared_library.cpp @@ -52,7 +52,7 @@ TEST_F(TestSharedLibrary, basic_load) { ASSERT_EQ(RCUTILS_RET_OK, ret); // unload shared_library - ret = rcutils_unload_library(&lib); + ret = rcutils_unload_shared_library(&lib); ASSERT_EQ(RCUTILS_RET_OK, ret); // checking if we have unloaded and freed memory @@ -83,6 +83,6 @@ TEST_F(TestSharedLibrary, basic_symbol) { EXPECT_TRUE(symbol != NULL); // unload shared_library - ret = rcutils_unload_library(&lib); + ret = rcutils_unload_shared_library(&lib); ASSERT_EQ(RCUTILS_RET_OK, ret); } From e07d449bb638bb08661d73969fabb5b9246c9b04 Mon Sep 17 00:00:00 2001 From: ahcorde Date: Fri, 13 Mar 2020 17:24:34 +0100 Subject: [PATCH 09/35] Added feedback Signed-off-by: ahcorde --- src/shared_library.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/shared_library.c b/src/shared_library.c index 9d149a98..c7bc4bdd 100644 --- a/src/shared_library.c +++ b/src/shared_library.c @@ -101,10 +101,7 @@ rcutils_has_symbol(const rcutils_shared_library_t * lib, const char * symbol_nam #else void * lib_symbol = GetProcAddress(lib->lib_pointer, symbol_name); #endif // _WIN32 - if (!lib_symbol) { - return false; - } - return true; + return lib_symbol != 0; } rcutils_ret_t @@ -123,7 +120,7 @@ rcutils_unload_shared_library(rcutils_shared_library_t * lib) ret = RCUTILS_RET_ERROR; } #else - // zero if the function succeeds + // If the function succeeds, the return value is nonzero. int error_code = FreeLibrary(lib->lib_pointer); if (!error_code) { RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("FreeLibrary error: %lu", GetLastError()); From a7456f2be00ea5c14319fbafbbe00c872243d57a Mon Sep 17 00:00:00 2001 From: ahcorde Date: Tue, 17 Mar 2020 11:49:50 +0100 Subject: [PATCH 10/35] deallocated memory when open fails Signed-off-by: ahcorde --- src/shared_library.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/shared_library.c b/src/shared_library.c index c7bc4bdd..6f1957d9 100644 --- a/src/shared_library.c +++ b/src/shared_library.c @@ -54,12 +54,14 @@ rcutils_load_shared_library(rcutils_shared_library_t * lib, const char * library #ifndef _WIN32 lib->lib_pointer = dlopen(lib->library_path, RTLD_LAZY); if (!lib->lib_pointer) { + lib->allocator.deallocate(lib->library_path, lib->allocator.state); RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("dlclose error: %s", dlerror()); return RCUTILS_RET_ERROR; } #else lib->lib_pointer = LoadLibrary(lib->library_path); if (!lib->lib_pointer) { + lib->allocator.deallocate(lib->library_path, lib->allocator.state); RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("LoadLibrary error: %lu", GetLastError()); return RCUTILS_RET_ERROR; } From 55fdb7d9d436236b942daabb908a5137e162ba4e Mon Sep 17 00:00:00 2001 From: ahcorde Date: Tue, 17 Mar 2020 13:24:59 +0100 Subject: [PATCH 11/35] Added null after deallocate memory Signed-off-by: ahcorde --- src/shared_library.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/shared_library.c b/src/shared_library.c index 6f1957d9..78459ad0 100644 --- a/src/shared_library.c +++ b/src/shared_library.c @@ -55,6 +55,7 @@ rcutils_load_shared_library(rcutils_shared_library_t * lib, const char * library lib->lib_pointer = dlopen(lib->library_path, RTLD_LAZY); if (!lib->lib_pointer) { lib->allocator.deallocate(lib->library_path, lib->allocator.state); + lib->library_path = NULL; RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("dlclose error: %s", dlerror()); return RCUTILS_RET_ERROR; } @@ -62,6 +63,7 @@ rcutils_load_shared_library(rcutils_shared_library_t * lib, const char * library lib->lib_pointer = LoadLibrary(lib->library_path); if (!lib->lib_pointer) { lib->allocator.deallocate(lib->library_path, lib->allocator.state); + lib->library_path = NULL; RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("LoadLibrary error: %lu", GetLastError()); return RCUTILS_RET_ERROR; } From f0213b66a8372e62912272772f435bcd12241d3c Mon Sep 17 00:00:00 2001 From: ahcorde Date: Wed, 18 Mar 2020 16:59:08 +0100 Subject: [PATCH 12/35] Updated doc shared_library Signed-off-by: ahcorde --- include/rcutils/shared_library.h | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/include/rcutils/shared_library.h b/include/rcutils/shared_library.h index 06164a19..5d7a5be5 100644 --- a/include/rcutils/shared_library.h +++ b/include/rcutils/shared_library.h @@ -49,19 +49,23 @@ typedef struct RCUTILS_PUBLIC_TYPE rcutils_shared_library_t /// Return an empty shared library struct. /* * This function returns an empty and zero initialized shared library struct. + * The default allocator is set. * * Example: * * ```c * // Do not do this: * // rcutils_shared_library_t foo; - * // rcutils_allocator_t allocator = rcutils_get_default_allocator(); - * // rcutils_unload_shared_library(&foo, allocator); // undefined behavior! + * // rcutils_load_shared_library(&foo, "library_name"); // undefined behavior! + * // or + * // rcutils_unload_shared_library(&foo); // undefined behavior! * * // Do this instead: * rcutils_shared_library_t bar = rcutils_get_zero_initialized_shared_library(); - * rcutils_allocator_t allocator = rcutils_get_default_allocator(); * rcutils_load_shared_library(&bar, "library_name"); // ok + * void * symbol = rcutils_get_symbol(&bar, "bazinga"); // ok + * bool is_bazinga_symbol = rcutils_has_symbol(&bar, "bazinga"); // ok + * rcutils_unload_shared_library(&bar); // ok * ``` * */ RCUTILS_PUBLIC @@ -94,7 +98,7 @@ RCUTILS_WARN_UNUSED void * rcutils_get_symbol(const rcutils_shared_library_t * lib, const char * symbol_name); -/// Return if the shared library contains a specific symbolname . +/// Return true if the shared library contains a specific symbol name otherwise returns false. /** * \param[in] lib struct with the shared library pointer and shared library path name * \param[in] symbol_name name of the symbol inside the shared library From 28cd1d5bf8380b76a683c37b83190109dd7a6a82 Mon Sep 17 00:00:00 2001 From: ahcorde Date: Wed, 18 Mar 2020 17:00:35 +0100 Subject: [PATCH 13/35] Improved error handling Signed-off-by: ahcorde --- src/shared_library.c | 38 +++++++++++++++++++++++++------------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/src/shared_library.c b/src/shared_library.c index 78459ad0..946461ee 100644 --- a/src/shared_library.c +++ b/src/shared_library.c @@ -19,9 +19,9 @@ extern "C" #include #include -#include "rcutils/shared_library.h" #include "rcutils/error_handling.h" - +#include "rcutils/shared_library.h" +#include "rcutils/strdup.h" rcutils_shared_library_t rcutils_get_zero_initialized_shared_library(void) @@ -38,25 +38,18 @@ rcutils_load_shared_library(rcutils_shared_library_t * lib, const char * library { RCUTILS_CHECK_ARGUMENT_FOR_NULL(lib, RCUTILS_RET_INVALID_ARGUMENT); - // allocating memory to - lib->library_path = (char *)(lib->allocator.allocate( - (strlen(library_path) + 1) * sizeof(char), - lib->allocator.state)); - - if (!lib->library_path) { + lib->library_path = rcutils_strdup(library_path, lib->allocator); + if (NULL == lib->library_path) { RCUTILS_SET_ERROR_MSG("unable to allocate memory"); return RCUTILS_RET_BAD_ALLOC; } - // copying string - snprintf(lib->library_path, strlen(library_path) + 1, "%s", library_path); - #ifndef _WIN32 lib->lib_pointer = dlopen(lib->library_path, RTLD_LAZY); if (!lib->lib_pointer) { lib->allocator.deallocate(lib->library_path, lib->allocator.state); lib->library_path = NULL; - RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("dlclose error: %s", dlerror()); + RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("dlopen error: %s", dlerror()); return RCUTILS_RET_ERROR; } #else @@ -81,8 +74,21 @@ rcutils_get_symbol(const rcutils_shared_library_t * lib, const char * symbol_nam #ifndef _WIN32 void * lib_symbol = dlsym(lib->lib_pointer, symbol_name); + char * error = dlerror(); + if (error != NULL) { + RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( + "Error getting the symbol '%s'. Error '%s'", + symbol_name, error); + return NULL; + } #else void * lib_symbol = GetProcAddress(lib->lib_pointer, symbol_name); + if ( lib_symbol == NULL) { + RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( + "Error getting the symbol '%s'. Error '%d'", + symbol_name, GetLastError()); + return NULL; + } #endif // _WIN32 if (!lib_symbol) { RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( @@ -101,11 +107,17 @@ rcutils_has_symbol(const rcutils_shared_library_t * lib, const char * symbol_nam } #ifndef _WIN32 + // the correct way to test for an error is to call dlerror() to clear any old error conditions, + // then call dlsym(), and then call dlerror() again, saving its return value into a variable, + // and check whether this saved value is not NULL. + dlerror(); /* Clear any existing error */ void * lib_symbol = dlsym(lib->lib_pointer, symbol_name); + return dlerror() == NULL && lib_symbol != 0; #else void * lib_symbol = GetProcAddress(lib->lib_pointer, symbol_name); + return GetLastError() == 0 && lib_symbol != 0; + } #endif // _WIN32 - return lib_symbol != 0; } rcutils_ret_t From 0b6d1716f0daf2fbb9a6340645fc059e114b6c60 Mon Sep 17 00:00:00 2001 From: ahcorde Date: Wed, 18 Mar 2020 17:22:30 +0100 Subject: [PATCH 14/35] Fixed uncrustify and cppcheck Signed-off-by: ahcorde --- src/shared_library.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/shared_library.c b/src/shared_library.c index 946461ee..b7ac4d9c 100644 --- a/src/shared_library.c +++ b/src/shared_library.c @@ -79,15 +79,15 @@ rcutils_get_symbol(const rcutils_shared_library_t * lib, const char * symbol_nam RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( "Error getting the symbol '%s'. Error '%s'", symbol_name, error); - return NULL; - } + return NULL; + } #else void * lib_symbol = GetProcAddress(lib->lib_pointer, symbol_name); - if ( lib_symbol == NULL) { + if (lib_symbol == NULL) { RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( "Error getting the symbol '%s'. Error '%d'", symbol_name, GetLastError()); - return NULL; + return NULL; } #endif // _WIN32 if (!lib_symbol) { @@ -116,7 +116,6 @@ rcutils_has_symbol(const rcutils_shared_library_t * lib, const char * symbol_nam #else void * lib_symbol = GetProcAddress(lib->lib_pointer, symbol_name); return GetLastError() == 0 && lib_symbol != 0; - } #endif // _WIN32 } From 2fb659ab659809c2f4c52ad43a1b9eb929deb8e9 Mon Sep 17 00:00:00 2001 From: ahcorde Date: Mon, 23 Mar 2020 10:00:33 +0100 Subject: [PATCH 15/35] Added allocator argument to rcutils_load_shared_library funcion Signed-off-by: ahcorde --- include/rcutils/shared_library.h | 13 ++++++++++--- src/shared_library.c | 8 ++++++-- test/test_shared_library.cpp | 4 ++-- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/include/rcutils/shared_library.h b/include/rcutils/shared_library.h index 5d7a5be5..494e79e3 100644 --- a/include/rcutils/shared_library.h +++ b/include/rcutils/shared_library.h @@ -56,13 +56,16 @@ typedef struct RCUTILS_PUBLIC_TYPE rcutils_shared_library_t * ```c * // Do not do this: * // rcutils_shared_library_t foo; - * // rcutils_load_shared_library(&foo, "library_name"); // undefined behavior! + * // rcutils_load_shared_library( + * // &foo, + * // "library_name", + * // rcutils_get_default_allocator()); // undefined behavior! * // or * // rcutils_unload_shared_library(&foo); // undefined behavior! * * // Do this instead: * rcutils_shared_library_t bar = rcutils_get_zero_initialized_shared_library(); - * rcutils_load_shared_library(&bar, "library_name"); // ok + * rcutils_load_shared_library(&bar, "library_name",rcutils_get_default_allocator()); // ok * void * symbol = rcutils_get_symbol(&bar, "bazinga"); // ok * bool is_bazinga_symbol = rcutils_has_symbol(&bar, "bazinga"); // ok * rcutils_unload_shared_library(&bar); // ok @@ -77,6 +80,7 @@ rcutils_get_zero_initialized_shared_library(void); /** * \param[inout] lib struct with the shared library pointer and shared library path name * \param[in] library_path string with the path of the library + * \param[in] allocator to be used to allocate and deallocate memory * \return `RCUTILS_RET_OK` if successful, or * \return `RCUTILS_RET_BAD_ALLOC` if memory allocation fails, or * \return `RCUTILS_RET_ERROR` if an unknown error occurs, or @@ -85,7 +89,10 @@ rcutils_get_zero_initialized_shared_library(void); RCUTILS_PUBLIC RCUTILS_WARN_UNUSED rcutils_ret_t -rcutils_load_shared_library(rcutils_shared_library_t * lib, const char * library_path); +rcutils_load_shared_library( + rcutils_shared_library_t * lib, + const char * library_path, + rcutils_allocator_t allocator); /// Return shared library symbol pointer. /** diff --git a/src/shared_library.c b/src/shared_library.c index b7ac4d9c..58042dee 100644 --- a/src/shared_library.c +++ b/src/shared_library.c @@ -29,15 +29,19 @@ rcutils_get_zero_initialized_shared_library(void) rcutils_shared_library_t zero_initialized_shared_library; zero_initialized_shared_library.library_path = NULL; zero_initialized_shared_library.lib_pointer = NULL; - zero_initialized_shared_library.allocator = rcutils_get_default_allocator(); return zero_initialized_shared_library; } rcutils_ret_t -rcutils_load_shared_library(rcutils_shared_library_t * lib, const char * library_path) +rcutils_load_shared_library( + rcutils_shared_library_t * lib, + const char * library_path, + rcutils_allocator_t allocator) { RCUTILS_CHECK_ARGUMENT_FOR_NULL(lib, RCUTILS_RET_INVALID_ARGUMENT); + lib->allocator = allocator; + lib->library_path = rcutils_strdup(library_path, lib->allocator); if (NULL == lib->library_path) { RCUTILS_SET_ERROR_MSG("unable to allocate memory"); diff --git a/test/test_shared_library.cpp b/test/test_shared_library.cpp index 137bcb92..2ba2efc5 100644 --- a/test/test_shared_library.cpp +++ b/test/test_shared_library.cpp @@ -48,7 +48,7 @@ TEST_F(TestSharedLibrary, basic_load) { const std::string library_path = std::string("libdummy_shared_library.so"); // getting shared library - ret = rcutils_load_shared_library(&lib, library_path.c_str()); + ret = rcutils_load_shared_library(&lib, library_path.c_str(), rcutils_get_default_allocator()); ASSERT_EQ(RCUTILS_RET_OK, ret); // unload shared_library @@ -73,7 +73,7 @@ TEST_F(TestSharedLibrary, basic_symbol) { const std::string library_path = std::string("libdummy_shared_library.so"); // getting shared library - ret = rcutils_load_shared_library(&lib, library_path.c_str()); + ret = rcutils_load_shared_library(&lib, library_path.c_str(), rcutils_get_default_allocator()); ASSERT_EQ(RCUTILS_RET_OK, ret); symbol = rcutils_get_symbol(&lib, "symbol"); From 4763b691c8cd66d816602a3ac37838f65a13c552 Mon Sep 17 00:00:00 2001 From: ahcorde Date: Mon, 23 Mar 2020 19:13:41 +0100 Subject: [PATCH 16/35] checked inputs and deduplicate code Signed-off-by: ahcorde --- CMakeLists.txt | 6 +----- src/shared_library.c | 19 +++++++++---------- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 3c28af7d..c9ba1787 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -315,11 +315,7 @@ if(BUILD_TESTING) target_link_libraries(test_repl_str ${PROJECT_NAME}) endif() - if(WIN32) - set(append_library_dirs "$") - else() - set(append_library_dirs "${CMAKE_CURRENT_BINARY_DIR}") - endif() + set(append_library_dirs "$") ament_add_gtest(test_shared_library test/test_shared_library.cpp APPEND_LIBRARY_DIRS "${append_library_dirs}" diff --git a/src/shared_library.c b/src/shared_library.c index 58042dee..a67a2b7f 100644 --- a/src/shared_library.c +++ b/src/shared_library.c @@ -29,6 +29,7 @@ rcutils_get_zero_initialized_shared_library(void) rcutils_shared_library_t zero_initialized_shared_library; zero_initialized_shared_library.library_path = NULL; zero_initialized_shared_library.lib_pointer = NULL; + zero_initialized_shared_library.allocator = rcutils_get_zero_initialized_allocator(); return zero_initialized_shared_library; } @@ -39,6 +40,8 @@ rcutils_load_shared_library( rcutils_allocator_t allocator) { RCUTILS_CHECK_ARGUMENT_FOR_NULL(lib, RCUTILS_RET_INVALID_ARGUMENT); + RCUTILS_CHECK_ARGUMENT_FOR_NULL(library_path, RCUTILS_RET_INVALID_ARGUMENT); + RCUTILS_CHECK_ALLOCATOR(&allocator, return RCUTILS_RET_INVALID_ARGUMENT); lib->allocator = allocator; @@ -51,20 +54,16 @@ rcutils_load_shared_library( #ifndef _WIN32 lib->lib_pointer = dlopen(lib->library_path, RTLD_LAZY); if (!lib->lib_pointer) { - lib->allocator.deallocate(lib->library_path, lib->allocator.state); - lib->library_path = NULL; - RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("dlopen error: %s", dlerror()); - return RCUTILS_RET_ERROR; - } + RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("LoadLibrary error: %s", dlerror()); #else lib->lib_pointer = LoadLibrary(lib->library_path); if (!lib->lib_pointer) { + RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("LoadLibrary error: %lu", GetLastError()); +#endif // _WIN32 lib->allocator.deallocate(lib->library_path, lib->allocator.state); lib->library_path = NULL; - RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("LoadLibrary error: %lu", GetLastError()); return RCUTILS_RET_ERROR; } -#endif // _WIN32 return RCUTILS_RET_OK; } @@ -129,6 +128,7 @@ rcutils_unload_shared_library(rcutils_shared_library_t * lib) RCUTILS_CHECK_ARGUMENT_FOR_NULL(lib, RCUTILS_RET_INVALID_ARGUMENT); RCUTILS_CHECK_ARGUMENT_FOR_NULL(lib->lib_pointer, RCUTILS_RET_INVALID_ARGUMENT); RCUTILS_CHECK_ARGUMENT_FOR_NULL(lib->library_path, RCUTILS_RET_INVALID_ARGUMENT); + RCUTILS_CHECK_ALLOCATOR(&lib->allocator, return RCUTILS_RET_INVALID_ARGUMENT); rcutils_ret_t ret = RCUTILS_RET_OK; #ifndef _WIN32 @@ -136,20 +136,19 @@ rcutils_unload_shared_library(rcutils_shared_library_t * lib) int error_code = dlclose(lib->lib_pointer); if (error_code) { RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("dlclose error: %s", dlerror()); - ret = RCUTILS_RET_ERROR; - } #else // If the function succeeds, the return value is nonzero. int error_code = FreeLibrary(lib->lib_pointer); if (!error_code) { RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("FreeLibrary error: %lu", GetLastError()); +#endif // _WIN32 ret = RCUTILS_RET_ERROR; } -#endif // _WIN32 lib->allocator.deallocate(lib->library_path, lib->allocator.state); lib->library_path = NULL; lib->lib_pointer = NULL; + lib->allocator = rcutils_get_zero_initialized_allocator(); return ret; } From 791214afc1a389a57bde249956c9efdd52c77676 Mon Sep 17 00:00:00 2001 From: ahcorde Date: Mon, 23 Mar 2020 19:14:06 +0100 Subject: [PATCH 17/35] added more test to shared_library Signed-off-by: ahcorde --- test/test_shared_library.cpp | 40 ++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/test/test_shared_library.cpp b/test/test_shared_library.cpp index 2ba2efc5..451fdce6 100644 --- a/test/test_shared_library.cpp +++ b/test/test_shared_library.cpp @@ -60,6 +60,46 @@ TEST_F(TestSharedLibrary, basic_load) { EXPECT_TRUE(lib.lib_pointer == NULL); } +TEST_F(TestSharedLibrary, error_load) { + rcutils_ret_t ret; + + rcutils_shared_library_t lib_empty; + ret = rcutils_load_shared_library(&lib_empty, NULL, rcutils_get_zero_initialized_allocator()); + ASSERT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret); + + lib_empty = rcutils_get_zero_initialized_shared_library(); + ret = rcutils_load_shared_library(&lib_empty, NULL, rcutils_get_zero_initialized_allocator()); + ASSERT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret); + + const std::string library_path = std::string("libdummy_shared_library.so"); + ret = rcutils_load_shared_library(&lib_empty, library_path.c_str(), rcutils_get_zero_initialized_allocator()); + ASSERT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret); +} + +TEST_F(TestSharedLibrary, error_unload) { + rcutils_ret_t ret; + + const std::string library_path = std::string("libdummy_shared_library.so"); + ret = rcutils_load_shared_library(&lib, library_path.c_str(), rcutils_get_default_allocator()); + ASSERT_EQ(RCUTILS_RET_OK, ret); + + // unload shared_library + ret = rcutils_unload_shared_library(&lib); + ASSERT_EQ(RCUTILS_RET_OK, ret); + + // unload again shared_library + ret = rcutils_unload_shared_library(&lib); + ASSERT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret); +} + +TEST_F(TestSharedLibrary, error_symbol) { + bool is_symbol = rcutils_has_symbol(&lib, "symbol"); + EXPECT_TRUE(is_symbol == false); + + void * symbol = rcutils_get_symbol(&lib, "print_name"); + EXPECT_TRUE(symbol == NULL); +} + TEST_F(TestSharedLibrary, basic_symbol) { void * symbol; bool ret; From 16a317339b2b9781ee198ec4d090c7614085d1a6 Mon Sep 17 00:00:00 2001 From: ahcorde Date: Mon, 23 Mar 2020 20:14:08 +0100 Subject: [PATCH 18/35] fixed unscrustify test_shared_library Signed-off-by: ahcorde --- test/test_shared_library.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/test_shared_library.cpp b/test/test_shared_library.cpp index 451fdce6..bc7beab1 100644 --- a/test/test_shared_library.cpp +++ b/test/test_shared_library.cpp @@ -72,7 +72,9 @@ TEST_F(TestSharedLibrary, error_load) { ASSERT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret); const std::string library_path = std::string("libdummy_shared_library.so"); - ret = rcutils_load_shared_library(&lib_empty, library_path.c_str(), rcutils_get_zero_initialized_allocator()); + ret = rcutils_load_shared_library( + &lib_empty, + library_path.c_str(), rcutils_get_zero_initialized_allocator()); ASSERT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret); } From f5da00b222830d50110d623cee376f69b1b618e9 Mon Sep 17 00:00:00 2001 From: ahcorde Date: Thu, 26 Mar 2020 11:30:29 +0100 Subject: [PATCH 19/35] Fixed docs Signed-off-by: ahcorde --- include/rcutils/shared_library.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/rcutils/shared_library.h b/include/rcutils/shared_library.h index 494e79e3..05d9f577 100644 --- a/include/rcutils/shared_library.h +++ b/include/rcutils/shared_library.h @@ -49,7 +49,6 @@ typedef struct RCUTILS_PUBLIC_TYPE rcutils_shared_library_t /// Return an empty shared library struct. /* * This function returns an empty and zero initialized shared library struct. - * The default allocator is set. * * Example: * From 60fe43cb78b036b75c040d5c1b22601a23853589 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Hern=C3=A1ndez=20Cordero?= Date: Thu, 26 Mar 2020 12:51:38 +0100 Subject: [PATCH 20/35] Update include/rcutils/shared_library.h Co-Authored-By: William Woodall Signed-off-by: ahcorde --- include/rcutils/shared_library.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/rcutils/shared_library.h b/include/rcutils/shared_library.h index 05d9f577..ae7e6ae6 100644 --- a/include/rcutils/shared_library.h +++ b/include/rcutils/shared_library.h @@ -64,7 +64,7 @@ typedef struct RCUTILS_PUBLIC_TYPE rcutils_shared_library_t * * // Do this instead: * rcutils_shared_library_t bar = rcutils_get_zero_initialized_shared_library(); - * rcutils_load_shared_library(&bar, "library_name",rcutils_get_default_allocator()); // ok + * rcutils_load_shared_library(&bar, "library_name", rcutils_get_default_allocator()); // ok * void * symbol = rcutils_get_symbol(&bar, "bazinga"); // ok * bool is_bazinga_symbol = rcutils_has_symbol(&bar, "bazinga"); // ok * rcutils_unload_shared_library(&bar); // ok From 13fe9ffa841b703855114b8ae8288179ca3d22e6 Mon Sep 17 00:00:00 2001 From: ahcorde Date: Thu, 26 Mar 2020 12:59:03 +0100 Subject: [PATCH 21/35] Improved docs Signed-off-by: ahcorde --- include/rcutils/shared_library.h | 9 ++++++--- src/shared_library.c | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/include/rcutils/shared_library.h b/include/rcutils/shared_library.h index ae7e6ae6..093cf13b 100644 --- a/include/rcutils/shared_library.h +++ b/include/rcutils/shared_library.h @@ -55,19 +55,22 @@ typedef struct RCUTILS_PUBLIC_TYPE rcutils_shared_library_t * ```c * // Do not do this: * // rcutils_shared_library_t foo; - * // rcutils_load_shared_library( + * // rcutils_ret_t ret = rcutils_load_shared_library( * // &foo, * // "library_name", * // rcutils_get_default_allocator()); // undefined behavior! * // or - * // rcutils_unload_shared_library(&foo); // undefined behavior! + * // rcutils_ret_t ret = rcutils_unload_shared_library(&foo); // undefined behavior! * * // Do this instead: * rcutils_shared_library_t bar = rcutils_get_zero_initialized_shared_library(); * rcutils_load_shared_library(&bar, "library_name", rcutils_get_default_allocator()); // ok * void * symbol = rcutils_get_symbol(&bar, "bazinga"); // ok * bool is_bazinga_symbol = rcutils_has_symbol(&bar, "bazinga"); // ok - * rcutils_unload_shared_library(&bar); // ok + * rcutils_ret_t ret = rcutils_unload_shared_library(&bar); // ok + * if (ret != RCUTILS_RET_ERROR) { + * // error handling + * } * ``` * */ RCUTILS_PUBLIC diff --git a/src/shared_library.c b/src/shared_library.c index a67a2b7f..1f021f2e 100644 --- a/src/shared_library.c +++ b/src/shared_library.c @@ -95,7 +95,7 @@ rcutils_get_symbol(const rcutils_shared_library_t * lib, const char * symbol_nam #endif // _WIN32 if (!lib_symbol) { RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( - "symbol '%s' doesnt' exit in library '%s'", + "symbol '%s' does not exist in the library '%s'", symbol_name, lib->library_path); return NULL; } From af0d73e87237267b1236f690468357f22544ace0 Mon Sep 17 00:00:00 2001 From: ahcorde Date: Fri, 27 Mar 2020 09:53:24 +0100 Subject: [PATCH 22/35] update shared library docs Signed-off-by: ahcorde --- include/rcutils/shared_library.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/rcutils/shared_library.h b/include/rcutils/shared_library.h index 093cf13b..4dc124e8 100644 --- a/include/rcutils/shared_library.h +++ b/include/rcutils/shared_library.h @@ -120,7 +120,7 @@ rcutils_has_symbol(const rcutils_shared_library_t * lib, const char * symbol_nam /// Unload the shared library. /** - * \param[inout] lib rcutils_shared_library_t to be finalized + * \param[in] lib rcutils_shared_library_t to be finalized * \return `RCUTILS_RET_OK` if successful, or * \return `RCUTILS_RET_INVALID_ARGUMENT` for invalid arguments, or * \return `RCUTILS_RET_ERROR` if an unknown error occurs From 8dc66df8868220f0d3052d1b3e560ac20275b1dc Mon Sep 17 00:00:00 2001 From: ahcorde Date: Mon, 30 Mar 2020 10:20:00 +0200 Subject: [PATCH 23/35] Avoided leak when a library is loaded two time without unload Signed-off-by: ahcorde --- src/shared_library.c | 4 ++++ test/test_shared_library.cpp | 17 +++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/src/shared_library.c b/src/shared_library.c index 1f021f2e..d126b8e0 100644 --- a/src/shared_library.c +++ b/src/shared_library.c @@ -45,6 +45,10 @@ rcutils_load_shared_library( lib->allocator = allocator; + if (lib->library_path != NULL) { + lib->allocator.deallocate(lib->library_path, lib->allocator.state); + } + lib->library_path = rcutils_strdup(library_path, lib->allocator); if (NULL == lib->library_path) { RCUTILS_SET_ERROR_MSG("unable to allocate memory"); diff --git a/test/test_shared_library.cpp b/test/test_shared_library.cpp index bc7beab1..94c0a5da 100644 --- a/test/test_shared_library.cpp +++ b/test/test_shared_library.cpp @@ -60,6 +60,23 @@ TEST_F(TestSharedLibrary, basic_load) { EXPECT_TRUE(lib.lib_pointer == NULL); } +TEST_F(TestSharedLibrary, load_two_times) { + rcutils_ret_t ret; + + const std::string library_path = std::string("libdummy_shared_library.so"); + // getting shared library + ret = rcutils_load_shared_library(&lib, library_path.c_str(), rcutils_get_default_allocator()); + ASSERT_EQ(RCUTILS_RET_OK, ret); + + // getting shared library + ret = rcutils_load_shared_library(&lib, library_path.c_str(), rcutils_get_default_allocator()); + ASSERT_EQ(RCUTILS_RET_OK, ret); + + // unload shared_library + ret = rcutils_unload_shared_library(&lib); + ASSERT_EQ(RCUTILS_RET_OK, ret); +} + TEST_F(TestSharedLibrary, error_load) { rcutils_ret_t ret; From e154f9c0a4556f45d930f99a824ac8ebee57c21b Mon Sep 17 00:00:00 2001 From: ahcorde Date: Tue, 31 Mar 2020 10:11:06 +0200 Subject: [PATCH 24/35] changed the order to deallocate memory Signed-off-by: ahcorde --- src/shared_library.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/shared_library.c b/src/shared_library.c index d126b8e0..84f5ee43 100644 --- a/src/shared_library.c +++ b/src/shared_library.c @@ -43,12 +43,12 @@ rcutils_load_shared_library( RCUTILS_CHECK_ARGUMENT_FOR_NULL(library_path, RCUTILS_RET_INVALID_ARGUMENT); RCUTILS_CHECK_ALLOCATOR(&allocator, return RCUTILS_RET_INVALID_ARGUMENT); - lib->allocator = allocator; - if (lib->library_path != NULL) { lib->allocator.deallocate(lib->library_path, lib->allocator.state); } + lib->allocator = allocator; + lib->library_path = rcutils_strdup(library_path, lib->allocator); if (NULL == lib->library_path) { RCUTILS_SET_ERROR_MSG("unable to allocate memory"); From 124cf99bf621a6b852ac4d769a4f800ba2951fee Mon Sep 17 00:00:00 2001 From: ahcorde Date: Tue, 31 Mar 2020 17:25:21 +0200 Subject: [PATCH 25/35] Added extension for different platforms in test_shared_libraries Signed-off-by: ahcorde --- test/test_shared_library.cpp | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/test/test_shared_library.cpp b/test/test_shared_library.cpp index 94c0a5da..b09c0ce4 100644 --- a/test/test_shared_library.cpp +++ b/test/test_shared_library.cpp @@ -33,8 +33,21 @@ class TestSharedLibrary : public ::testing::Test // running test has failed. rcutils_reset_error(); lib = rcutils_get_zero_initialized_shared_library(); + + library_path = std::string("libdummy_shared_library") + + #ifdef __linux__ + std::string(".so"); + #elif __APPLE__ + std::string(".dylib"); + #elif _WIN32 + std::string(".dll"); + #else + #error "Unsupported OS, dynamic library suffix is unknown." + #endif + } rcutils_shared_library_t lib; + std::string library_path; }; TEST_F(TestSharedLibrary, basic_load) { @@ -44,9 +57,6 @@ TEST_F(TestSharedLibrary, basic_load) { ASSERT_STRNE(lib.library_path, ""); EXPECT_TRUE(lib.lib_pointer == NULL); - // library path - const std::string library_path = std::string("libdummy_shared_library.so"); - // getting shared library ret = rcutils_load_shared_library(&lib, library_path.c_str(), rcutils_get_default_allocator()); ASSERT_EQ(RCUTILS_RET_OK, ret); @@ -63,7 +73,6 @@ TEST_F(TestSharedLibrary, basic_load) { TEST_F(TestSharedLibrary, load_two_times) { rcutils_ret_t ret; - const std::string library_path = std::string("libdummy_shared_library.so"); // getting shared library ret = rcutils_load_shared_library(&lib, library_path.c_str(), rcutils_get_default_allocator()); ASSERT_EQ(RCUTILS_RET_OK, ret); @@ -88,7 +97,6 @@ TEST_F(TestSharedLibrary, error_load) { ret = rcutils_load_shared_library(&lib_empty, NULL, rcutils_get_zero_initialized_allocator()); ASSERT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret); - const std::string library_path = std::string("libdummy_shared_library.so"); ret = rcutils_load_shared_library( &lib_empty, library_path.c_str(), rcutils_get_zero_initialized_allocator()); @@ -98,7 +106,6 @@ TEST_F(TestSharedLibrary, error_load) { TEST_F(TestSharedLibrary, error_unload) { rcutils_ret_t ret; - const std::string library_path = std::string("libdummy_shared_library.so"); ret = rcutils_load_shared_library(&lib, library_path.c_str(), rcutils_get_default_allocator()); ASSERT_EQ(RCUTILS_RET_OK, ret); @@ -129,8 +136,6 @@ TEST_F(TestSharedLibrary, basic_symbol) { ret = rcutils_has_symbol(nullptr, "symbol"); EXPECT_FALSE(ret); - const std::string library_path = std::string("libdummy_shared_library.so"); - // getting shared library ret = rcutils_load_shared_library(&lib, library_path.c_str(), rcutils_get_default_allocator()); ASSERT_EQ(RCUTILS_RET_OK, ret); From c6bbb2c5f30de628911b6025d4edee0a1681d289 Mon Sep 17 00:00:00 2001 From: ahcorde Date: Tue, 31 Mar 2020 17:31:45 +0200 Subject: [PATCH 26/35] fixed ccplint Signed-off-by: ahcorde --- test/test_shared_library.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/test/test_shared_library.cpp b/test/test_shared_library.cpp index b09c0ce4..2dc87b1a 100644 --- a/test/test_shared_library.cpp +++ b/test/test_shared_library.cpp @@ -44,7 +44,6 @@ class TestSharedLibrary : public ::testing::Test #else #error "Unsupported OS, dynamic library suffix is unknown." #endif - } rcutils_shared_library_t lib; std::string library_path; From 80b7ea1db71b4f7d5f196e8fa84b6c995a8030fc Mon Sep 17 00:00:00 2001 From: ahcorde Date: Tue, 31 Mar 2020 18:28:04 +0200 Subject: [PATCH 27/35] Fixed dummy library name Signed-off-by: ahcorde --- test/dummy_shared_library/dummy_shared_library.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/dummy_shared_library/dummy_shared_library.h b/test/dummy_shared_library/dummy_shared_library.h index 8c724b79..02c4a5c7 100644 --- a/test/dummy_shared_library/dummy_shared_library.h +++ b/test/dummy_shared_library/dummy_shared_library.h @@ -28,6 +28,6 @@ #include DUMMY_SHARED_LIBRARY_PUBLIC -void print_foo(); +void print_name(); #endif // DUMMY_SHARED_LIBRARY__DUMMY_SHARED_LIBRARY_H_ From 2779d6bd8c89b0f01de9f69eb26f175a5df84bc7 Mon Sep 17 00:00:00 2001 From: ahcorde Date: Tue, 31 Mar 2020 18:28:35 +0200 Subject: [PATCH 28/35] Right library name in windows in test_shared_library Signed-off-by: ahcorde --- test/test_shared_library.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/test/test_shared_library.cpp b/test/test_shared_library.cpp index 2dc87b1a..c413d35f 100644 --- a/test/test_shared_library.cpp +++ b/test/test_shared_library.cpp @@ -34,13 +34,12 @@ class TestSharedLibrary : public ::testing::Test rcutils_reset_error(); lib = rcutils_get_zero_initialized_shared_library(); - library_path = std::string("libdummy_shared_library") + #ifdef __linux__ - std::string(".so"); + library_path = std::string("libdummy_shared_library.so"); #elif __APPLE__ - std::string(".dylib"); + library_path = std::string("libdummy_shared_library.dylib"); #elif _WIN32 - std::string(".dll"); + library_path = std::string("dummy_shared_library.dll"); #else #error "Unsupported OS, dynamic library suffix is unknown." #endif From 8ab106f74f7b536bf4725adc8663e305d07a7238 Mon Sep 17 00:00:00 2001 From: ahcorde Date: Tue, 31 Mar 2020 18:35:02 +0200 Subject: [PATCH 29/35] preprocessor directives not indented Signed-off-by: ahcorde --- test/test_shared_library.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/test_shared_library.cpp b/test/test_shared_library.cpp index c413d35f..87a02e81 100644 --- a/test/test_shared_library.cpp +++ b/test/test_shared_library.cpp @@ -34,13 +34,13 @@ class TestSharedLibrary : public ::testing::Test rcutils_reset_error(); lib = rcutils_get_zero_initialized_shared_library(); - #ifdef __linux__ - library_path = std::string("libdummy_shared_library.so"); - #elif __APPLE__ - library_path = std::string("libdummy_shared_library.dylib"); - #elif _WIN32 - library_path = std::string("dummy_shared_library.dll"); - #else +#ifdef __linux__ + library_path = std::string("libdummy_shared_library.so"); +#elif __APPLE__ + library_path = std::string("libdummy_shared_library.dylib"); +#elif _WIN32 + library_path = std::string("dummy_shared_library.dll"); +#else #error "Unsupported OS, dynamic library suffix is unknown." #endif } From d941e44755bffda67808ca955b753c445cfcffb4 Mon Sep 17 00:00:00 2001 From: ahcorde Date: Tue, 31 Mar 2020 19:57:00 +0200 Subject: [PATCH 30/35] Added rcutils_get_platform_library_name fucntion to shared_library Signed-off-by: ahcorde --- include/rcutils/shared_library.h | 12 ++++++++++ src/shared_library.c | 24 ++++++++++++++++++++ test/test_shared_library.cpp | 39 ++++++++++++++++++-------------- 3 files changed, 58 insertions(+), 17 deletions(-) diff --git a/include/rcutils/shared_library.h b/include/rcutils/shared_library.h index 4dc124e8..b3c2194c 100644 --- a/include/rcutils/shared_library.h +++ b/include/rcutils/shared_library.h @@ -130,6 +130,18 @@ RCUTILS_WARN_UNUSED rcutils_ret_t rcutils_unload_shared_library(rcutils_shared_library_t * lib); +/// Get the library name for the compiled platform +/** + * \param[in] library_name library base name (without prefix and extension) + * \param[out] library_name_platform library name for the compiled platform + * \return `RCUTILS_RET_OK` if successful, or + * \return `RCUTILS_RET_ERROR` if an unknown error occurs + */ +RCUTILS_PUBLIC +RCUTILS_WARN_UNUSED +rcutils_ret_t +rcutils_get_platform_library_name(const char * library_name, char * library_name_platform); + #ifdef __cplusplus } #endif diff --git a/src/shared_library.c b/src/shared_library.c index 84f5ee43..19cc9d03 100644 --- a/src/shared_library.c +++ b/src/shared_library.c @@ -156,6 +156,30 @@ rcutils_unload_shared_library(rcutils_shared_library_t * lib) return ret; } +rcutils_ret_t +rcutils_get_platform_library_name(const char * library_name, char * library_name_platform) +{ + int written = 0; + +#ifdef __linux__ + written = rcutils_snprintf( + library_name_platform, strlen(library_name) + 7, "lib%s.so", library_name); +#elif __APPLE__ + written = rcutils_snprintf( + library_name_platform, strlen(library_name) + 8, "lib%s.dylib", library_name); +#elif _WIN32 + written = rcutils_snprintf( + library_name_platform, strlen(library_name) + 5, "%s.dll", library_name); +#endif + if (written < 0) { + RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( + "failed to format library name: '%s'\n", + library_name); + return RCUTILS_RET_ERROR; + } + return RCUTILS_RET_OK; +} + #ifdef __cplusplus } #endif diff --git a/test/test_shared_library.cpp b/test/test_shared_library.cpp index 87a02e81..04f95940 100644 --- a/test/test_shared_library.cpp +++ b/test/test_shared_library.cpp @@ -33,19 +33,9 @@ class TestSharedLibrary : public ::testing::Test // running test has failed. rcutils_reset_error(); lib = rcutils_get_zero_initialized_shared_library(); - -#ifdef __linux__ - library_path = std::string("libdummy_shared_library.so"); -#elif __APPLE__ - library_path = std::string("libdummy_shared_library.dylib"); -#elif _WIN32 - library_path = std::string("dummy_shared_library.dll"); -#else - #error "Unsupported OS, dynamic library suffix is unknown." - #endif } rcutils_shared_library_t lib; - std::string library_path; + char library_path[1024]; }; TEST_F(TestSharedLibrary, basic_load) { @@ -55,8 +45,11 @@ TEST_F(TestSharedLibrary, basic_load) { ASSERT_STRNE(lib.library_path, ""); EXPECT_TRUE(lib.lib_pointer == NULL); + ret = rcutils_get_platform_library_name("dummy_shared_library", library_path); + ASSERT_EQ(RCUTILS_RET_OK, ret); + // getting shared library - ret = rcutils_load_shared_library(&lib, library_path.c_str(), rcutils_get_default_allocator()); + ret = rcutils_load_shared_library(&lib, library_path, rcutils_get_default_allocator()); ASSERT_EQ(RCUTILS_RET_OK, ret); // unload shared_library @@ -71,12 +64,15 @@ TEST_F(TestSharedLibrary, basic_load) { TEST_F(TestSharedLibrary, load_two_times) { rcutils_ret_t ret; + ret = rcutils_get_platform_library_name("dummy_shared_library", library_path); + ASSERT_EQ(RCUTILS_RET_OK, ret); + // getting shared library - ret = rcutils_load_shared_library(&lib, library_path.c_str(), rcutils_get_default_allocator()); + ret = rcutils_load_shared_library(&lib, library_path, rcutils_get_default_allocator()); ASSERT_EQ(RCUTILS_RET_OK, ret); // getting shared library - ret = rcutils_load_shared_library(&lib, library_path.c_str(), rcutils_get_default_allocator()); + ret = rcutils_load_shared_library(&lib, library_path, rcutils_get_default_allocator()); ASSERT_EQ(RCUTILS_RET_OK, ret); // unload shared_library @@ -95,16 +91,22 @@ TEST_F(TestSharedLibrary, error_load) { ret = rcutils_load_shared_library(&lib_empty, NULL, rcutils_get_zero_initialized_allocator()); ASSERT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret); + ret = rcutils_get_platform_library_name("dummy_shared_library", library_path); + ASSERT_EQ(RCUTILS_RET_OK, ret); + ret = rcutils_load_shared_library( &lib_empty, - library_path.c_str(), rcutils_get_zero_initialized_allocator()); + library_path, rcutils_get_zero_initialized_allocator()); ASSERT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret); } TEST_F(TestSharedLibrary, error_unload) { rcutils_ret_t ret; - ret = rcutils_load_shared_library(&lib, library_path.c_str(), rcutils_get_default_allocator()); + ret = rcutils_get_platform_library_name("dummy_shared_library", library_path); + ASSERT_EQ(RCUTILS_RET_OK, ret); + + ret = rcutils_load_shared_library(&lib, library_path, rcutils_get_default_allocator()); ASSERT_EQ(RCUTILS_RET_OK, ret); // unload shared_library @@ -134,8 +136,11 @@ TEST_F(TestSharedLibrary, basic_symbol) { ret = rcutils_has_symbol(nullptr, "symbol"); EXPECT_FALSE(ret); + ret = rcutils_get_platform_library_name("dummy_shared_library", library_path); + ASSERT_EQ(RCUTILS_RET_OK, ret); + // getting shared library - ret = rcutils_load_shared_library(&lib, library_path.c_str(), rcutils_get_default_allocator()); + ret = rcutils_load_shared_library(&lib, library_path, rcutils_get_default_allocator()); ASSERT_EQ(RCUTILS_RET_OK, ret); symbol = rcutils_get_symbol(&lib, "symbol"); From 26b7157ac4e13d3574fd97a793b6d082aaa05a6c Mon Sep 17 00:00:00 2001 From: ahcorde Date: Tue, 31 Mar 2020 20:31:36 +0200 Subject: [PATCH 31/35] fixed rcutils_snprintf buffer size Signed-off-by: ahcorde --- src/shared_library.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared_library.c b/src/shared_library.c index 19cc9d03..e02aca87 100644 --- a/src/shared_library.c +++ b/src/shared_library.c @@ -166,7 +166,7 @@ rcutils_get_platform_library_name(const char * library_name, char * library_name library_name_platform, strlen(library_name) + 7, "lib%s.so", library_name); #elif __APPLE__ written = rcutils_snprintf( - library_name_platform, strlen(library_name) + 8, "lib%s.dylib", library_name); + library_name_platform, strlen(library_name) + 10, "lib%s.dylib", library_name); #elif _WIN32 written = rcutils_snprintf( library_name_platform, strlen(library_name) + 5, "%s.dll", library_name); From 66187a37d0db7787aa20d48104b451dc8a10f6cb Mon Sep 17 00:00:00 2001 From: ahcorde Date: Wed, 1 Apr 2020 09:44:04 +0200 Subject: [PATCH 32/35] Added target_compile_definitions for dummy_shared_library Signed-off-by: ahcorde --- CMakeLists.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index c9ba1787..ef85ade7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -323,6 +323,11 @@ if(BUILD_TESTING) if(TARGET test_shared_library) add_library(dummy_shared_library test/dummy_shared_library/dummy_shared_library.c) + if(WIN32) + # Causes the visibility macros to use dllexport rather than dllimport + # which is appropriate when building the dll but not consuming it. + target_compile_definitions(dummy_shared_library PRIVATE "DUMMY_SHARED_LIBRARY_BUILDING_DLL") + endif() target_link_libraries(test_shared_library ${PROJECT_NAME}) endif() From 510bf355f0cee666d1c096075cae1e6b255babbd Mon Sep 17 00:00:00 2001 From: ahcorde Date: Wed, 1 Apr 2020 11:36:46 +0200 Subject: [PATCH 33/35] defined len of str in rcutils_get_platform_library_name Signed-off-by: ahcorde --- include/rcutils/shared_library.h | 9 ++++++++- src/shared_library.c | 33 +++++++++++++++++++++++++------- test/test_shared_library.cpp | 10 +++++----- 3 files changed, 39 insertions(+), 13 deletions(-) diff --git a/include/rcutils/shared_library.h b/include/rcutils/shared_library.h index b3c2194c..01bb01bf 100644 --- a/include/rcutils/shared_library.h +++ b/include/rcutils/shared_library.h @@ -25,9 +25,12 @@ extern "C" #ifndef _WIN32 #include typedef void * rcutils_shared_library_handle_t; +# include +# define RCUTILS_DIR_PATH_MAX PATH_MAX #else #include typedef HINSTANCE rcutils_shared_library_handle_t; +# define RCUTILS_DIR_PATH_MAX MAX_PATH #endif // _WIN32 #include "rcutils/allocator.h" @@ -134,13 +137,17 @@ rcutils_unload_shared_library(rcutils_shared_library_t * lib); /** * \param[in] library_name library base name (without prefix and extension) * \param[out] library_name_platform library name for the compiled platform + * \param[in] buffer_size size of library_name_platform buffer * \return `RCUTILS_RET_OK` if successful, or * \return `RCUTILS_RET_ERROR` if an unknown error occurs */ RCUTILS_PUBLIC RCUTILS_WARN_UNUSED rcutils_ret_t -rcutils_get_platform_library_name(const char * library_name, char * library_name_platform); +rcutils_get_platform_library_name( + const char * library_name, + char * library_name_platform, + unsigned int buffer_size); #ifdef __cplusplus } diff --git a/src/shared_library.c b/src/shared_library.c index e02aca87..73572a0a 100644 --- a/src/shared_library.c +++ b/src/shared_library.c @@ -157,19 +157,38 @@ rcutils_unload_shared_library(rcutils_shared_library_t * lib) } rcutils_ret_t -rcutils_get_platform_library_name(const char * library_name, char * library_name_platform) +rcutils_get_platform_library_name( + const char * library_name, + char * library_name_platform, + unsigned int buffer_size) { + RCUTILS_CHECK_ARGUMENT_FOR_NULL(library_name, RCUTILS_RET_INVALID_ARGUMENT); + RCUTILS_CHECK_ARGUMENT_FOR_NULL(library_name_platform, RCUTILS_RET_INVALID_ARGUMENT); + + if (strlen(library_name) > RCUTILS_DIR_PATH_MAX) + { + RCUTILS_SET_ERROR_MSG( + "library_name is too long more than the maximum allowed size\n"); + return RCUTILS_RET_ERROR; + } + int written = 0; #ifdef __linux__ - written = rcutils_snprintf( - library_name_platform, strlen(library_name) + 7, "lib%s.so", library_name); + if (buffer_size > (strlen(library_name) + 7)) { + written = rcutils_snprintf( + library_name_platform, strlen(library_name) + 7, "lib%s.so", library_name); + } #elif __APPLE__ - written = rcutils_snprintf( - library_name_platform, strlen(library_name) + 10, "lib%s.dylib", library_name); + if (buffer_size > (strlen(library_name) + 10)) { + written = rcutils_snprintf( + library_name_platform, strlen(library_name) + 10, "lib%s.dylib", library_name); + } #elif _WIN32 - written = rcutils_snprintf( - library_name_platform, strlen(library_name) + 5, "%s.dll", library_name); + if (buffer_size > (strlen(library_name) + 7)) { + written = rcutils_snprintf( + library_name_platform, strlen(library_name) + 5, "%s.dll", library_name); + } #endif if (written < 0) { RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( diff --git a/test/test_shared_library.cpp b/test/test_shared_library.cpp index 04f95940..4edb087a 100644 --- a/test/test_shared_library.cpp +++ b/test/test_shared_library.cpp @@ -45,7 +45,7 @@ TEST_F(TestSharedLibrary, basic_load) { ASSERT_STRNE(lib.library_path, ""); EXPECT_TRUE(lib.lib_pointer == NULL); - ret = rcutils_get_platform_library_name("dummy_shared_library", library_path); + ret = rcutils_get_platform_library_name("dummy_shared_library", library_path, 1024); ASSERT_EQ(RCUTILS_RET_OK, ret); // getting shared library @@ -64,7 +64,7 @@ TEST_F(TestSharedLibrary, basic_load) { TEST_F(TestSharedLibrary, load_two_times) { rcutils_ret_t ret; - ret = rcutils_get_platform_library_name("dummy_shared_library", library_path); + ret = rcutils_get_platform_library_name("dummy_shared_library", library_path, 1024); ASSERT_EQ(RCUTILS_RET_OK, ret); // getting shared library @@ -91,7 +91,7 @@ TEST_F(TestSharedLibrary, error_load) { ret = rcutils_load_shared_library(&lib_empty, NULL, rcutils_get_zero_initialized_allocator()); ASSERT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret); - ret = rcutils_get_platform_library_name("dummy_shared_library", library_path); + ret = rcutils_get_platform_library_name("dummy_shared_library", library_path, 1024); ASSERT_EQ(RCUTILS_RET_OK, ret); ret = rcutils_load_shared_library( @@ -103,7 +103,7 @@ TEST_F(TestSharedLibrary, error_load) { TEST_F(TestSharedLibrary, error_unload) { rcutils_ret_t ret; - ret = rcutils_get_platform_library_name("dummy_shared_library", library_path); + ret = rcutils_get_platform_library_name("dummy_shared_library", library_path, 1024); ASSERT_EQ(RCUTILS_RET_OK, ret); ret = rcutils_load_shared_library(&lib, library_path, rcutils_get_default_allocator()); @@ -136,7 +136,7 @@ TEST_F(TestSharedLibrary, basic_symbol) { ret = rcutils_has_symbol(nullptr, "symbol"); EXPECT_FALSE(ret); - ret = rcutils_get_platform_library_name("dummy_shared_library", library_path); + ret = rcutils_get_platform_library_name("dummy_shared_library", library_path, 1024); ASSERT_EQ(RCUTILS_RET_OK, ret); // getting shared library From e049c2045a5035fd9cbf31658d3eadda5a848844 Mon Sep 17 00:00:00 2001 From: ahcorde Date: Wed, 1 Apr 2020 17:12:26 +0200 Subject: [PATCH 34/35] Removed limitation on the path size for the library name Signed-off-by: ahcorde --- include/rcutils/shared_library.h | 3 --- src/shared_library.c | 7 ------- 2 files changed, 10 deletions(-) diff --git a/include/rcutils/shared_library.h b/include/rcutils/shared_library.h index 01bb01bf..629a41c2 100644 --- a/include/rcutils/shared_library.h +++ b/include/rcutils/shared_library.h @@ -25,12 +25,9 @@ extern "C" #ifndef _WIN32 #include typedef void * rcutils_shared_library_handle_t; -# include -# define RCUTILS_DIR_PATH_MAX PATH_MAX #else #include typedef HINSTANCE rcutils_shared_library_handle_t; -# define RCUTILS_DIR_PATH_MAX MAX_PATH #endif // _WIN32 #include "rcutils/allocator.h" diff --git a/src/shared_library.c b/src/shared_library.c index 73572a0a..52098cdd 100644 --- a/src/shared_library.c +++ b/src/shared_library.c @@ -165,13 +165,6 @@ rcutils_get_platform_library_name( RCUTILS_CHECK_ARGUMENT_FOR_NULL(library_name, RCUTILS_RET_INVALID_ARGUMENT); RCUTILS_CHECK_ARGUMENT_FOR_NULL(library_name_platform, RCUTILS_RET_INVALID_ARGUMENT); - if (strlen(library_name) > RCUTILS_DIR_PATH_MAX) - { - RCUTILS_SET_ERROR_MSG( - "library_name is too long more than the maximum allowed size\n"); - return RCUTILS_RET_ERROR; - } - int written = 0; #ifdef __linux__ From 5cc468ea931e1ed07a1c4c973bc181904820b8cf Mon Sep 17 00:00:00 2001 From: ahcorde Date: Wed, 1 Apr 2020 17:13:30 +0200 Subject: [PATCH 35/35] fixed condition len string in rcutils_get_platform_library_name Signed-off-by: ahcorde --- src/shared_library.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/shared_library.c b/src/shared_library.c index 52098cdd..bf14d10d 100644 --- a/src/shared_library.c +++ b/src/shared_library.c @@ -168,17 +168,17 @@ rcutils_get_platform_library_name( int written = 0; #ifdef __linux__ - if (buffer_size > (strlen(library_name) + 7)) { + if (buffer_size >= (strlen(library_name) + 7)) { written = rcutils_snprintf( library_name_platform, strlen(library_name) + 7, "lib%s.so", library_name); } #elif __APPLE__ - if (buffer_size > (strlen(library_name) + 10)) { + if (buffer_size >= (strlen(library_name) + 10)) { written = rcutils_snprintf( library_name_platform, strlen(library_name) + 10, "lib%s.dylib", library_name); } #elif _WIN32 - if (buffer_size > (strlen(library_name) + 7)) { + if (buffer_size >= (strlen(library_name) + 7)) { written = rcutils_snprintf( library_name_platform, strlen(library_name) + 5, "%s.dll", library_name); }