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.

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

Change the contract of get_typesupport_handle_function. #102

Draft
wants to merge 1 commit into
base: rolling
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,16 @@ extern "C"
/// Get the message type support handle specific to this identifier.
/**
* If the identifier is the same as this handle's typesupport_identifier, then the handle is
* simply returned, otherwise it's loaded from a shared library.
* simply returned. If the identifier is the hard-coded one for this typesupport, then
* the handle is loaded from a shared library. If the identifier is *not* the hard-coded
* one for this typesupport, then a nullptr is returned and no error is set. Finally,
* if an error occurs while loading the handle from a shared library, then an error
* is set and a nullptr is returned.
*
* The above contract allows this function to be used to probe for typesupports, and
* also return errors in the case that something went wrong. Callers should take care
* to handle all cases above, and can disambiguate whether the probe failed or a
* library loading error occurred by calling `rcutils_error_is_set` after this function.
*
* \param handle Handle to message type support
* \param identifier The typesupport identifier to get the handle function for
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,18 @@ extern "C"
/// Get the service type support handle specific to this identifier.
/**
* If the identifier is the same as this handle's typesupport_identifier, then the handle is
* simply returned, otherwise it's loaded from a shared library.
* simply returned. If the identifier is the hard-coded one for this typesupport, then
* the handle is loaded from a shared library. If the identifier is *not* the hard-coded
* one for this typesupport, then a nullptr is returned and no error is set. Finally,
* if an error occurs while loading the handle from a shared library, then an error
* is set and a nullptr is returned.
*
* \param handle Handle to message type support
* The above contract allows this function to be used to probe for typesupports, and
* also return errors in the case that something went wrong. Callers should take care
* to handle all cases above, and can disambiguate whether the probe failed or a
* library loading error occurred by calling `rcutils_error_is_set` after this function.
*
* \param handle Handle to service type support
* \param identifier The typesupport identifier to get the handle function for
* \return The associated service typesupport handle if found, otherwise NULL
*/
Expand Down
141 changes: 79 additions & 62 deletions rosidl_typesupport_c/src/type_support_dispatch.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#include <cstddef>
#include <cstdio>
#include <cstring>

#include <memory>
#include <stdexcept>
#include <string>
Expand All @@ -34,85 +33,103 @@ namespace rosidl_typesupport_c

extern const char * typesupport_identifier;

/// Get the type support handle specific to this identifier.
/**
* If the identifier is the same as this handle's typesupport_identifier, then the handle is
* simply returned. If the identifier is the hard-coded one for this typesupport, then
* the handle is loaded from a shared library. If the identifier is *not* the hard-coded
* one for this typesupport, then a nullptr is returned and no error is set. Finally,
* if an error occurs while loading the handle from a shared library, then an error
* is set and a nullptr is returned.
*
* The above contract allows this function to be used to probe for typesupports, and
* also return errors in the case that something went wrong. Callers should take care
* to handle all cases above, and can disambiguate whether the probe failed or a
* library loading error occurred by calling `rcutils_error_is_set` after this function.
*
* \param handle Handle to type support
* \param identifier The typesupport identifier to get the handle function for
* \return The associated typesupport handle if found, otherwise NULL
*/
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;
}

if (handle->typesupport_identifier == rosidl_typesupport_c__typesupport_identifier) {
const type_support_map_t * map = \
static_cast<const type_support_map_t *>(handle->data);
for (size_t i = 0; i < map->size; ++i) {
if (strcmp(map->typesupport_identifier[i], identifier) != 0) {
continue;
}
rcpputils::SharedLibrary * lib = nullptr;

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

std::string library_name;
try {
library_name = rcpputils::get_platform_library_name(library_basename);
} catch (const std::exception & e) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Failed to compute library name for '%s' due to %s",
library_basename, e.what());
return nullptr;
}

try {
lib = new rcpputils::SharedLibrary(library_name);
} catch (const std::runtime_error & e) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Could not load library %s: %s", library_name.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", library_name.c_str(), e.what());
return nullptr;
}
map->data[i] = lib;
if (handle->typesupport_identifier != rosidl_typesupport_c__typesupport_identifier) {
return nullptr;
}

const type_support_map_t * map =
static_cast<const type_support_map_t *>(handle->data);
for (size_t i = 0; i < map->size; ++i) {
if (strcmp(map->typesupport_identifier[i], identifier) != 0) {
continue;
}
rcpputils::SharedLibrary * lib = nullptr;

if (!map->data[i]) {
char library_basename[1024];
int ret = rcutils_snprintf(
library_basename, 1023, "%s__%s",
map->package_name, identifier);
if (ret < 0) {
RCUTILS_SET_ERROR_MSG("Failed to format library name");
return nullptr;
}
auto clib = static_cast<const rcpputils::SharedLibrary *>(map->data[i]);
lib = const_cast<rcpputils::SharedLibrary *>(clib);

void * sym = nullptr;
std::string library_name;
try {
library_name = rcpputils::get_platform_library_name(library_basename);
} catch (const std::runtime_error & e) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, exactly.

RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Failed to compute library name for '%s' due to %s",
library_basename, e.what());
return 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) {
lib = new rcpputils::SharedLibrary(library_name);
} catch (const std::runtime_error & e) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Failed to get symbol '%s' in library: %s",
map->symbol_name[i], e.what());
"Could not load library %s: %s", library_name.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", library_name.c_str(), e.what());
return nullptr;
}
map->data[i] = lib;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any explanation of how this code works that you're aware of? It's a little strange that it modifies a non-const void ** member of a const type_support_t *.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, I just moved it around. @hidmic was involved in the latest rewrite of it, I believe. Maybe he can shed some light.

}
auto clib = static_cast<const rcpputils::SharedLibrary *>(map->data[i]);
lib = const_cast<rcpputils::SharedLibrary *>(clib);

typedef const TypeSupport * (* funcSignature)(void);
funcSignature func = reinterpret_cast<funcSignature>(sym);
const TypeSupport * ts = func();
return ts;
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;
}

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
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ TEST(TestMessageTypeSupportDispatch, get_handle_function) {
rosidl_typesupport_c__get_message_typesupport_handle_function(
&type_support,
"different_identifier"), nullptr);
EXPECT_TRUE(rcutils_error_is_set());
EXPECT_FALSE(rcutils_error_is_set());
rcutils_reset_error();

rosidl_message_type_support_t type_support_c_identifier =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ TEST(TestServiceTypeSupportDispatch, get_handle_function) {
rosidl_typesupport_c__get_service_typesupport_handle_function(
&type_support,
"different_identifier"), nullptr);
EXPECT_TRUE(rcutils_error_is_set());
EXPECT_FALSE(rcutils_error_is_set());
rcutils_reset_error();

rosidl_service_type_support_t type_support_c_identifier =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,16 @@ namespace rosidl_typesupport_cpp
/// Get the message type support handle specific to this identifier.
/**
* If the identifier is the same as this handle's typesupport_identifier, then the handle is
* simply returned, otherwise it's loaded from a shared library.
* simply returned. If the identifier is the hard-coded one for this typesupport, then
* the handle is loaded from a shared library. If the identifier is *not* the hard-coded
* one for this typesupport, then a nullptr is returned and no error is set. Finally,
* if an error occurs while loading the handle from a shared library, then an error
* is set and a nullptr is returned.
*
* The above contract allows this function to be used to probe for typesupports, and
* also return errors in the case that something went wrong. Callers should take care
* to handle all cases above, and can disambiguate whether the probe failed or a
* library loading error occurred by calling `rcutils_error_is_set` after this function.
*
* \param handle Handle to message type support
* \param identifier The typesupport identifier to get the handle function for
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,18 @@ namespace rosidl_typesupport_cpp
/// Get the service type support handle specific to this identifier.
/**
* If the identifier is the same as this handle's typesupport_identifier, then the handle is
* simply returned, otherwise it's loaded from a shared library.
* simply returned. If the identifier is the hard-coded one for this typesupport, then
* the handle is loaded from a shared library. If the identifier is *not* the hard-coded
* one for this typesupport, then a nullptr is returned and no error is set. Finally,
* if an error occurs while loading the handle from a shared library, then an error
* is set and a nullptr is returned.
*
* \param handle Handle to message type support
* The above contract allows this function to be used to probe for typesupports, and
* also return errors in the case that something went wrong. Callers should take care
* to handle all cases above, and can disambiguate whether the probe failed or a
* library loading error occurred by calling `rcutils_error_is_set` after this function.
*
* \param handle Handle to service type support
* \param identifier The typesupport identifier to get the handle function for
* \return The associated service typesupport handle if found, otherwise NULL
*/
Expand Down
Loading