Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure typesupport handle functions do not throw. #99

Merged
merged 2 commits into from
Dec 14, 2020
Merged

Ensure typesupport handle functions do not throw. #99

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
16c8090
Ensure typesupport handle functions do not throw.
hidmic Dec 14, 2020
c27aeb7
Make rosidl_typesupport_cpp API noexcept
hidmic Dec 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 13 additions & 9 deletions rosidl_typesupport_c/src/type_support_dispatch.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@

#include <memory>
#include <stdexcept>
#include <list>
#include <string>

#include "rcpputils/find_library.hpp"
#include "rcpputils/shared_library.hpp"
#include "rcutils/error_handling.h"
#include "rcutils/snprintf.h"
#include "rosidl_typesupport_c/identifier.h"
#include "rosidl_typesupport_c/type_support_map.h"

Expand Down Expand Up @@ -55,35 +55,39 @@ get_typesupport_handle_function(

if (!map->data[i]) {
char library_name[1024];
snprintf(
int ret = rcutils_snprintf(
library_name, 1023, "%s__%s",
map->package_name, identifier);
if (ret < 0) {
RCUTILS_SET_ERROR_MSG("Failed to format library name");
return nullptr;
}

std::string library_path;
try {
library_path = rcpputils::find_library_path(library_name);
} catch (const std::exception & e) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Failed to find library '%s' due to %s\n",
"Failed to find library '%s' due to %s",
library_name, e.what());
return nullptr;
}

if (library_path.empty()) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Failed to find library '%s'\n", library_name);
"Failed to find library '%s'", library_name);
return nullptr;
}

try {
lib = new rcpputils::SharedLibrary(library_path.c_str());
} catch (const std::runtime_error & e) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Could not load library %s: %s\n", library_path.c_str(), e.what());
"Could not load library %s: %s", library_path.c_str(), e.what());
return nullptr;
} catch (const std::bad_alloc & e) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Could not load library %s: %s\n", library_path.c_str(), e.what());
"Could not load library %s: %s", library_path.c_str(), e.what());
return nullptr;
}
map->data[i] = lib;
Expand All @@ -96,13 +100,13 @@ get_typesupport_handle_function(
try {
if (!lib->has_symbol(map->symbol_name[i])) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Failed to find symbol '%s' in library\n", map->symbol_name[i]);
"Failed to find symbol '%s' in library", map->symbol_name[i]);
return nullptr;
}
sym = lib->get_symbol(map->symbol_name[i]);
} catch (const std::exception & e) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Failed to get symbol '%s' in library: %s\n",
"Failed to get symbol '%s' in library: %s",
map->symbol_name[i], e.what());
return nullptr;
}
Expand All @@ -114,7 +118,7 @@ get_typesupport_handle_function(
}
}
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Handle's typesupport identifier (%s) is not supported by this library\n",
"Handle's typesupport identifier (%s) is not supported by this library",
handle->typesupport_identifier);
return nullptr;
}
Expand Down
2 changes: 1 addition & 1 deletion rosidl_typesupport_cpp/include/rosidl_typesupport_cpp/message_type_support_dispatch.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ namespace rosidl_typesupport_cpp
ROSIDL_TYPESUPPORT_CPP_PUBLIC
const rosidl_message_type_support_t *
get_message_typesupport_handle_function(
const rosidl_message_type_support_t * handle, const char * identifier);
const rosidl_message_type_support_t * handle, const char * identifier) noexcept;

} // namespace rosidl_typesupport_cpp

Expand Down
2 changes: 1 addition & 1 deletion rosidl_typesupport_cpp/include/rosidl_typesupport_cpp/service_type_support_dispatch.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ namespace rosidl_typesupport_cpp
ROSIDL_TYPESUPPORT_CPP_PUBLIC
const rosidl_service_type_support_t *
get_service_typesupport_handle_function(
const rosidl_service_type_support_t * handle, const char * identifier);
const rosidl_service_type_support_t * handle, const char * identifier) noexcept;

} // namespace rosidl_typesupport_cpp

Expand Down
2 changes: 1 addition & 1 deletion rosidl_typesupport_cpp/src/message_type_support_dispatch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ namespace rosidl_typesupport_cpp

const rosidl_message_type_support_t *
get_message_typesupport_handle_function(
const rosidl_message_type_support_t * handle, const char * identifier)
const rosidl_message_type_support_t * handle, const char * identifier) noexcept
{
return rosidl_typesupport_cpp::get_typesupport_handle_function<
rosidl_message_type_support_t>(handle, identifier);
Expand Down
2 changes: 1 addition & 1 deletion rosidl_typesupport_cpp/src/service_type_support_dispatch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ namespace rosidl_typesupport_cpp

const rosidl_service_type_support_t *
get_service_typesupport_handle_function(
const rosidl_service_type_support_t * handle, const char * identifier)
const rosidl_service_type_support_t * handle, const char * identifier) noexcept
{
return rosidl_typesupport_cpp::get_typesupport_handle_function<
rosidl_service_type_support_t>(handle, identifier);
Expand Down
58 changes: 40 additions & 18 deletions rosidl_typesupport_cpp/src/type_support_dispatch.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,12 @@
#include <cstring>
#include <memory>
#include <stdexcept>

#include <string>

#include "rcpputils/find_library.hpp"
#include "rcpputils/shared_library.hpp"
#include "rcutils/error_handling.h"
#include "rcutils/snprintf.h"
#include "rosidl_typesupport_c/type_support_map.h"

namespace rosidl_typesupport_cpp
Expand All @@ -35,7 +36,7 @@ extern const char * typesupport_identifier;
template<typename TypeSupport>
const TypeSupport *
get_typesupport_handle_function(
const TypeSupport * handle, const char * identifier)
const TypeSupport * handle, const char * identifier) noexcept
{
if (strcmp(handle->typesupport_identifier, identifier) == 0) {
return handle;
Expand All @@ -51,51 +52,72 @@ get_typesupport_handle_function(
rcpputils::SharedLibrary * lib = nullptr;

if (!map->data[i]) {
std::string library_name{map->package_name};
library_name += "__";
library_name += identifier;
char library_name[1024];
int ret = rcutils_snprintf(
library_name, 1023, "%s__%s",
map->package_name, identifier);
if (ret < 0) {
RCUTILS_SET_ERROR_MSG("Failed to format library name");
return nullptr;
}

std::string library_path;
try {
library_path = rcpputils::find_library_path(library_name);
} catch (const std::runtime_error & e) {
const std::string message =
"Failed to find library '" + library_name + "' due to " + e.what();
throw std::runtime_error(message);
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Failed to find library '%s' due to %s",
library_name, e.what());
return nullptr;
}

if (library_path.empty()) {
fprintf(stderr, "Failed to find library '%s'\n", library_name.c_str());
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Failed to find library '%s'", library_name);
return nullptr;
}

try {
lib = new rcpputils::SharedLibrary(library_path.c_str());
} catch (const std::runtime_error & e) {
throw std::runtime_error(
"Could not load library " + library_path + ": " +
std::string(e.what()));
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Could not load library %s: %s", library_name, e.what());
return nullptr;
} catch (const std::bad_alloc & e) {
throw std::runtime_error(
"Could not load library " + library_path + ": " +
std::string(e.what()));
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Could not load library %s: %s", library_name, e.what());
return nullptr;
}
map->data[i] = lib;
}
auto clib = static_cast<const rcpputils::SharedLibrary *>(map->data[i]);
lib = const_cast<rcpputils::SharedLibrary *>(clib);
if (!lib->has_symbol(map->symbol_name[i])) {
fprintf(stderr, "Failed to find symbol '%s' in library\n", map->symbol_name[i]);

void * sym = nullptr;

try {
if (!lib->has_symbol(map->symbol_name[i])) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Failed to find symbol '%s' in library", map->symbol_name[i]);
return nullptr;
}
sym = lib->get_symbol(map->symbol_name[i]);
} catch (const std::exception & e) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Failed to get symbol '%s' in library: %s",
map->symbol_name[i], e.what());
return nullptr;
}
void * sym = lib->get_symbol(map->symbol_name[i]);

typedef const TypeSupport * (* funcSignature)(void);
funcSignature func = reinterpret_cast<funcSignature>(sym);
const TypeSupport * ts = func();
return ts;
}
}
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Handle's typesupport identifier (%s) is not supported by this library",
handle->typesupport_identifier);
return nullptr;
}

Expand Down
27 changes: 17 additions & 10 deletions rosidl_typesupport_cpp/test/test_message_type_support_dispatch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
// limitations under the License.

#include <gtest/gtest.h>
#include "rcutils/error_handling.h"
#include "rcutils/testing/fault_injection.h"
#include "rcpputils/shared_library.hpp"
#include "rosidl_typesupport_c/type_support_map.h"
Expand Down Expand Up @@ -84,6 +85,8 @@ TEST(TestMessageTypeSupportDispatch, get_handle_function) {
rosidl_typesupport_cpp::get_message_typesupport_handle_function(
&type_support,
"different_identifier"), nullptr);
EXPECT_TRUE(rcutils_error_is_set());
rcutils_reset_error();

rosidl_message_type_support_t type_support_cpp_identifier =
get_rosidl_message_type_support(rosidl_typesupport_cpp::typesupport_identifier);
Expand Down Expand Up @@ -113,18 +116,24 @@ TEST(TestMessageTypeSupportDispatch, get_handle_function) {
rosidl_typesupport_cpp::get_message_typesupport_handle_function(
&type_support_cpp_identifier,
"test_type_support2"), nullptr);
EXPECT_TRUE(rcutils_error_is_set());
rcutils_reset_error();

// Library file exists, but loading shared library fails
EXPECT_THROW(
EXPECT_EQ(
rosidl_typesupport_cpp::get_message_typesupport_handle_function(
&type_support_cpp_identifier,
"test_type_support3"), std::runtime_error);
"test_type_support3"), nullptr);
EXPECT_TRUE(rcutils_error_is_set());
rcutils_reset_error();

// Library doesn't exist
EXPECT_EQ(
rosidl_typesupport_cpp::get_message_typesupport_handle_function(
&type_support_cpp_identifier,
"test_type_support4"), nullptr);
EXPECT_TRUE(rcutils_error_is_set());
rcutils_reset_error();
}

TEST(TestMessageTypeSupportDispatch, get_message_typesupport_maybe_fail_test)
Expand All @@ -138,14 +147,12 @@ TEST(TestMessageTypeSupportDispatch, get_message_typesupport_maybe_fail_test)
RCUTILS_FAULT_INJECTION_TEST(
{
// load library and find symbols
try {
auto * result = rosidl_typesupport_cpp::get_message_typesupport_handle_function(
&type_support_cpp_identifier,
"test_type_support1");
EXPECT_NE(result, nullptr);
} catch (const std::runtime_error &) {
} catch (...) {
ADD_FAILURE() << "Unexpected exception type in fault injection test";
auto * result = rosidl_typesupport_cpp::get_message_typesupport_handle_function(
&type_support_cpp_identifier,
"test_type_support1");
if (nullptr == result) {
EXPECT_TRUE(rcutils_error_is_set());
rcutils_reset_error();
}
});
}
26 changes: 17 additions & 9 deletions rosidl_typesupport_cpp/test/test_service_type_support_dispatch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
// limitations under the License.

#include <gtest/gtest.h>
#include "rcutils/error_handling.h"
#include "rcutils/testing/fault_injection.h"
#include "rcpputils/shared_library.hpp"
#include "rosidl_typesupport_c/type_support_map.h"
Expand Down Expand Up @@ -84,6 +85,8 @@ TEST(TestServiceTypeSupportDispatch, get_handle_function) {
rosidl_typesupport_cpp::get_service_typesupport_handle_function(
&type_support,
"different_identifier"), nullptr);
EXPECT_TRUE(rcutils_error_is_set());
rcutils_reset_error();

rosidl_service_type_support_t type_support_cpp_identifier =
get_rosidl_service_type_support(rosidl_typesupport_cpp::typesupport_identifier);
Expand All @@ -110,18 +113,24 @@ TEST(TestServiceTypeSupportDispatch, get_handle_function) {
rosidl_typesupport_cpp::get_service_typesupport_handle_function(
&type_support_cpp_identifier,
"test_type_support2"), nullptr);
EXPECT_TRUE(rcutils_error_is_set());
rcutils_reset_error();

// Library file exists, but loading shared library fails
EXPECT_THROW(
EXPECT_EQ(
rosidl_typesupport_cpp::get_service_typesupport_handle_function(
&type_support_cpp_identifier,
"test_type_support3"), std::runtime_error);
"test_type_support3"), nullptr);
EXPECT_TRUE(rcutils_error_is_set());
rcutils_reset_error();

// Library doesn't exist
EXPECT_EQ(
rosidl_typesupport_cpp::get_service_typesupport_handle_function(
&type_support_cpp_identifier,
"test_type_support4"), nullptr);
EXPECT_TRUE(rcutils_error_is_set());
rcutils_reset_error();
}

TEST(TestServiceTypeSupportDispatch, get_service_typesupport_maybe_fail_test)
Expand All @@ -135,13 +144,12 @@ TEST(TestServiceTypeSupportDispatch, get_service_typesupport_maybe_fail_test)
RCUTILS_FAULT_INJECTION_TEST(
{
// load library and find symbols
try {
auto * result = rosidl_typesupport_cpp::get_service_typesupport_handle_function(
&type_support_cpp_identifier,
"test_type_support1");
EXPECT_NE(result, nullptr);
} catch (const std::runtime_error &) {
} catch (...) {
auto * result = rosidl_typesupport_cpp::get_service_typesupport_handle_function(
&type_support_cpp_identifier,
"test_type_support1");
if (nullptr == result) {
EXPECT_TRUE(rcutils_error_is_set());
rcutils_reset_error();
}
});
}