Skip to content

Commit

Permalink
Add in a missing data_reader check when creating subscription. (#697)
Browse files Browse the repository at this point in the history
The end stanza of create_datareader() is supposed to be
attempting to create a datareader with the underlying Fast-DDS
library.  If creation of the datareader returns a valid
pointer, we've succeeded, and if it returns a nullptr, it
has failed.

However, there were two separate problems of the logic
checking for these conditions:

1. It was checking the datareader pointer-to-a-pointer,
which should always be non-null.  What it really meant to
check was the datareader pointer (dereferenced).
2. There is a fallback mechanism for when unique network
flow endpoints were optionally required.  The problem with
that is that when using the fallback, we never check again
to make sure the fallback was successful.  Therefore, if
we failed for another reason (like security), we would
not discover it until a crash later on.

This commit fixes both of these issues.

Signed-off-by: Chris Lalancette <[email protected]>
  • Loading branch information
clalancette authored Jul 19, 2023
1 parent f02a72f commit efa94f7
Showing 1 changed file with 6 additions and 1 deletion.
7 changes: 6 additions & 1 deletion rmw_fastrtps_shared_cpp/src/utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ create_datareader(
updated_qos,
listener,
eprosima::fastdds::dds::StatusMask::subscription_matched());
if (!data_reader &&
if (!(*data_reader) &&
(RMW_UNIQUE_NETWORK_FLOW_ENDPOINTS_OPTIONALLY_REQUIRED ==
subscription_options->require_unique_network_flow_endpoints))
{
Expand All @@ -172,6 +172,11 @@ create_datareader(
listener,
eprosima::fastdds::dds::StatusMask::subscription_matched());
}

if (!(*data_reader)) {
return false;
}

return true;
}

Expand Down

0 comments on commit efa94f7

Please sign in to comment.