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

iox-#932 Handle nullptr callbacks in Listener #937

Merged
merged 1 commit into from
Oct 8, 2021
Merged

iox-#932 Handle nullptr callbacks in Listener #937

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions iceoryx_binding_c/include/iceoryx_binding_c/enums.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ enum iox_ListenerResult
{
ListenerResult_LISTENER_FULL,
ListenerResult_EVENT_ALREADY_ATTACHED,
ListenerResult_EMPTY_EVENT_CALLBACK,
ListenerResult_EMPTY_INVALIDATION_CALLBACK,
ListenerResult_UNDEFINED_ERROR,
ListenerResult_SUCCESS
Expand Down
14 changes: 9 additions & 5 deletions iceoryx_binding_c/source/c_listener.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,9 @@ ENUM iox_ListenerResult iox_listener_attach_subscriber_event(iox_listener_t cons
const ENUM iox_SubscriberEvent subscriberEvent,
void (*callback)(iox_sub_t))
{
auto result =
self->attachEvent(*subscriber, c2cpp::subscriberEvent(subscriberEvent), createNotificationCallback(*callback));
auto result = self->attachEvent(*subscriber,
c2cpp::subscriberEvent(subscriberEvent),
NotificationCallback<cpp2c_Subscriber, internal::NoType_t>{callback, nullptr});
if (result.has_error())
{
return cpp2c::listenerResult(result.get_error());
Expand All @@ -68,7 +69,9 @@ iox_listener_attach_subscriber_event_with_context_data(iox_listener_t const self
notificationCallback.m_callback = callback;
notificationCallback.m_contextData = contextData;

auto result = self->attachEvent(*subscriber, c2cpp::subscriberEvent(subscriberEvent), notificationCallback);
auto result = self->attachEvent(*subscriber,
c2cpp::subscriberEvent(subscriberEvent),
NotificationCallback<cpp2c_Subscriber, void>{callback, contextData});
if (result.has_error())
{
return cpp2c::listenerResult(result.get_error());
Expand All @@ -80,7 +83,8 @@ ENUM iox_ListenerResult iox_listener_attach_user_trigger_event(iox_listener_t co
iox_user_trigger_t const userTrigger,
void (*callback)(iox_user_trigger_t))
{
auto result = self->attachEvent(*userTrigger, createNotificationCallback(*callback));
auto result =
self->attachEvent(*userTrigger, NotificationCallback<UserTrigger, internal::NoType_t>{callback, nullptr});
if (result.has_error())
{
return cpp2c::listenerResult(result.get_error());
Expand All @@ -98,7 +102,7 @@ ENUM iox_ListenerResult iox_listener_attach_user_trigger_event_with_context_data
notificationCallback.m_callback = callback;
notificationCallback.m_contextData = contextData;

auto result = self->attachEvent(*userTrigger, notificationCallback);
auto result = self->attachEvent(*userTrigger, NotificationCallback<UserTrigger, void>{callback, contextData});
if (result.has_error())
{
return cpp2c::listenerResult(result.get_error());
Expand Down
2 changes: 2 additions & 0 deletions iceoryx_binding_c/source/cpp2c_enum_translation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ iox_ListenerResult listenerResult(const iox::popo::ListenerError value) noexcept
return ListenerResult_LISTENER_FULL;
case ListenerError::INVALID_STATE:
return ListenerResult_UNDEFINED_ERROR;
case ListenerError::EMPTY_EVENT_CALLBACK:
return ListenerResult_EMPTY_EVENT_CALLBACK;
case ListenerError::EMPTY_INVALIDATION_CALLBACK:
return ListenerResult_EMPTY_INVALIDATION_CALLBACK;
}
Expand Down
4 changes: 4 additions & 0 deletions iceoryx_binding_c/test/moduletests/test_cpp2c_enum_translation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ TEST(cpp2c_enum_translation_test, ListenerResult)
constexpr EnumMapping<iox::popo::ListenerError, iox_ListenerResult> LISTENER_ERRORS[]{
{iox::popo::ListenerError::LISTENER_FULL, ListenerResult_LISTENER_FULL},
{iox::popo::ListenerError::EVENT_ALREADY_ATTACHED, ListenerResult_EVENT_ALREADY_ATTACHED},
{iox::popo::ListenerError::EMPTY_EVENT_CALLBACK, ListenerResult_EMPTY_EVENT_CALLBACK},
{iox::popo::ListenerError::EMPTY_INVALIDATION_CALLBACK, ListenerResult_EMPTY_INVALIDATION_CALLBACK},
{iox::popo::ListenerError::INVALID_STATE, ListenerResult_UNDEFINED_ERROR}};

Expand All @@ -194,6 +195,9 @@ TEST(cpp2c_enum_translation_test, ListenerResult)
case iox::popo::ListenerError::EVENT_ALREADY_ATTACHED:
EXPECT_EQ(cpp2c::listenerResult(listenerError.cpp), listenerError.c);
break;
case iox::popo::ListenerError::EMPTY_EVENT_CALLBACK:
EXPECT_EQ(cpp2c::listenerResult(listenerError.cpp), listenerError.c);
break;
case iox::popo::ListenerError::EMPTY_INVALIDATION_CALLBACK:
EXPECT_EQ(cpp2c::listenerResult(listenerError.cpp), listenerError.c);
break;
Expand Down
13 changes: 13 additions & 0 deletions iceoryx_binding_c/test/moduletests/test_listener.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,19 @@ TEST_F(iox_listener_test, AttachingSubscriberEventWorks)
Eq(iox_ListenerResult::ListenerResult_SUCCESS));
}

TEST_F(iox_listener_test, AttachingSubscriberEventWithNullptrCallbackFails)
{
EXPECT_THAT(iox_listener_attach_subscriber_event(
&m_sut, &m_subscriber[0U], iox_SubscriberEvent::SubscriberEvent_DATA_RECEIVED, NULL),
Eq(iox_ListenerResult::ListenerResult_EMPTY_EVENT_CALLBACK));
}

TEST_F(iox_listener_test, AttachingUserTriggerEventWithNullptrCallbackFails)
{
EXPECT_THAT(iox_listener_attach_user_trigger_event(&m_sut, m_userTrigger[0U], NULL),
Eq(iox_ListenerResult::ListenerResult_EMPTY_EVENT_CALLBACK));
}

TEST_F(iox_listener_test, AttachingSubscriberTillListenerFullWorks)
{
AttachAllSubscriber();
Expand Down
22 changes: 16 additions & 6 deletions iceoryx_posh/include/iceoryx_posh/internal/popo/listener.inl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,33 +20,43 @@ namespace iox
{
namespace popo
{
template <typename T, typename UserType>
template <typename T, typename ContextDataType>
inline cxx::expected<ListenerError>
Listener::attachEvent(T& eventOrigin, const NotificationCallback<T, UserType>& eventCallback) noexcept
Listener::attachEvent(T& eventOrigin, const NotificationCallback<T, ContextDataType>& eventCallback) noexcept
{
if (eventCallback.m_callback == nullptr)
{
return cxx::error<ListenerError>(ListenerError::EMPTY_EVENT_CALLBACK);
}

return addEvent(&eventOrigin,
eventCallback.m_contextData,
static_cast<uint64_t>(NoEnumUsed::PLACEHOLDER),
typeid(NoEnumUsed).hash_code(),
reinterpret_cast<internal::GenericCallbackRef_t>(*eventCallback.m_callback),
internal::TranslateAndCallTypelessCallback<T, UserType>::call,
internal::TranslateAndCallTypelessCallback<T, ContextDataType>::call,
NotificationAttorney::getInvalidateTriggerMethod(eventOrigin))
.and_then([&](auto& eventId) {
NotificationAttorney::enableEvent(
eventOrigin, TriggerHandle(*m_conditionVariableData, {*this, &Listener::removeTrigger}, eventId));
});
}

template <typename T, typename EventType, typename UserType, typename>
template <typename T, typename EventType, typename ContextDataType, typename>
inline cxx::expected<ListenerError> Listener::attachEvent(
T& eventOrigin, const EventType eventType, const NotificationCallback<T, UserType>& eventCallback) noexcept
T& eventOrigin, const EventType eventType, const NotificationCallback<T, ContextDataType>& eventCallback) noexcept
{
if (eventCallback.m_callback == nullptr)
{
return cxx::error<ListenerError>(ListenerError::EMPTY_EVENT_CALLBACK);
}

return addEvent(&eventOrigin,
eventCallback.m_contextData,
static_cast<uint64_t>(eventType),
typeid(EventType).hash_code(),
reinterpret_cast<internal::GenericCallbackRef_t>(*eventCallback.m_callback),
internal::TranslateAndCallTypelessCallback<T, UserType>::call,
internal::TranslateAndCallTypelessCallback<T, ContextDataType>::call,
NotificationAttorney::getInvalidateTriggerMethod(eventOrigin))
.and_then([&](auto& eventId) {
NotificationAttorney::enableEvent(
Expand Down
1 change: 1 addition & 0 deletions iceoryx_posh/include/iceoryx_posh/popo/listener.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ enum class ListenerError
INVALID_STATE,
LISTENER_FULL,
EVENT_ALREADY_ATTACHED,
EMPTY_EVENT_CALLBACK,
EMPTY_INVALIDATION_CALLBACK
};

Expand Down
22 changes: 22 additions & 0 deletions iceoryx_posh/test/moduletests/test_popo_listener.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,28 @@ TEST_F(Listener_test, AttachingSameClassWithTwoDifferentEventsWorks)
.has_error());
}

TEST_F(Listener_test, AttachingNullptrCallbackFails)
{
auto empty_callback = createNotificationCallback(attachCallback);
empty_callback.m_callback = nullptr;
empty_callback.m_contextData = nullptr;

auto result = m_sut->attachEvent(m_simpleEvents[0U], empty_callback);
ASSERT_TRUE(result.has_error());
EXPECT_THAT(result.get_error(), Eq(ListenerError::EMPTY_EVENT_CALLBACK));
}

TEST_F(Listener_test, AttachingNullptrCallbackWithEventFails)
{
auto empty_callback = createNotificationCallback(attachCallback);
empty_callback.m_callback = nullptr;
empty_callback.m_contextData = nullptr;

auto result = m_sut->attachEvent(m_simpleEvents[0U], SimpleEvent::StoepselBachelorParty, empty_callback);
ASSERT_TRUE(result.has_error());
EXPECT_THAT(result.get_error(), Eq(ListenerError::EMPTY_EVENT_CALLBACK));
}

TEST_F(Listener_test, DetachingSameClassWithDifferentEventEnumChangesNothing)
{
ASSERT_FALSE(m_sut
Expand Down