Handle too large QoS queue depths. (#457)

Signed-off-by: Michel Hidalgo <[email protected]>
ros2 · Sep 30, 2020 · 65bb626 · 65bb626
1 parent 7bfe150
commit 65bb626
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 7 deletions.
diff --git a/rmw_fastrtps_shared_cpp/src/qos.cpp b/rmw_fastrtps_shared_cpp/src/qos.cpp
@@ -85,14 +85,10 @@ bool fill_entity_qos_from_profile(
       return false;
   }
 
-  if (qos_policies.depth != RMW_QOS_POLICY_DEPTH_SYSTEM_DEFAULT) {
-    history_qos.depth = static_cast<int32_t>(qos_policies.depth);
-  }
-
   // ensure the history depth is at least the requested queue size
   assert(history_qos.depth >= 0);
   if (
-    eprosima::fastrtps::KEEP_LAST_HISTORY_QOS == history_qos.kind &&
+    qos_policies.depth != RMW_QOS_POLICY_DEPTH_SYSTEM_DEFAULT &&
     static_cast<size_t>(history_qos.depth) < qos_policies.depth)
   {
     if (qos_policies.depth > static_cast<size_t>((std::numeric_limits<int32_t>::max)())) {

diff --git a/rmw_fastrtps_shared_cpp/test/test_rmw_qos_to_dds_attributes.cpp b/rmw_fastrtps_shared_cpp/test/test_rmw_qos_to_dds_attributes.cpp
@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include <limits>
 #include <tuple>
 
 #include "gtest/gtest.h"
@@ -94,7 +95,32 @@ TEST_F(GetDataReaderQoSTest, nominal_conversion) {
   EXPECT_EQ(
     eprosima::fastrtps::KEEP_LAST_HISTORY_QOS,
     subscriber_attributes_.topic.historyQos.kind);
-  EXPECT_EQ(10, subscriber_attributes_.topic.historyQos.depth);
+  EXPECT_GE(10, subscriber_attributes_.topic.historyQos.depth);
+}
+
+TEST_F(GetDataReaderQoSTest, large_depth_conversion) {
+  size_t depth = subscriber_attributes_.topic.historyQos.depth + 1;
+  qos_profile_.depth = depth;
+  qos_profile_.history = RMW_QOS_POLICY_HISTORY_KEEP_LAST;
+
+  EXPECT_TRUE(get_datareader_qos(qos_profile_, subscriber_attributes_));
+
+  EXPECT_EQ(
+    eprosima::fastrtps::KEEP_LAST_HISTORY_QOS,
+    subscriber_attributes_.topic.historyQos.kind);
+  EXPECT_LE(depth, static_cast<size_t>(subscriber_attributes_.topic.historyQos.depth));
+
+  using depth_type = decltype(subscriber_attributes_.topic.historyQos.depth);
+  constexpr size_t max_depth = static_cast<size_t>(std::numeric_limits<depth_type>::max());
+
+  qos_profile_.depth = max_depth;
+  EXPECT_TRUE(get_datareader_qos(qos_profile_, subscriber_attributes_));
+  EXPECT_LE(depth, static_cast<size_t>(subscriber_attributes_.topic.historyQos.depth));
+
+  if (max_depth < std::numeric_limits<size_t>::max()) {
+    qos_profile_.depth = max_depth + 1;
+    EXPECT_FALSE(get_datareader_qos(qos_profile_, subscriber_attributes_));
+  }
 }
 
 using eprosima::fastrtps::PublisherAttributes;
@@ -167,5 +193,30 @@ TEST_F(GetDataWriterQoSTest, nominal_conversion) {
   EXPECT_EQ(
     eprosima::fastrtps::KEEP_LAST_HISTORY_QOS,
     publisher_attributes_.topic.historyQos.kind);
-  EXPECT_EQ(10, publisher_attributes_.topic.historyQos.depth);
+  EXPECT_GE(10, publisher_attributes_.topic.historyQos.depth);
+}
+
+TEST_F(GetDataWriterQoSTest, large_depth_conversion) {
+  size_t depth = publisher_attributes_.topic.historyQos.depth + 1;
+  qos_profile_.depth = depth;
+  qos_profile_.history = RMW_QOS_POLICY_HISTORY_KEEP_LAST;
+
+  EXPECT_TRUE(get_datawriter_qos(qos_profile_, publisher_attributes_));
+
+  EXPECT_EQ(
+    eprosima::fastrtps::KEEP_LAST_HISTORY_QOS,
+    publisher_attributes_.topic.historyQos.kind);
+  EXPECT_LE(depth, static_cast<size_t>(publisher_attributes_.topic.historyQos.depth));
+
+  using depth_type = decltype(publisher_attributes_.topic.historyQos.depth);
+  constexpr size_t max_depth = static_cast<size_t>(std::numeric_limits<depth_type>::max());
+
+  qos_profile_.depth = max_depth;
+  EXPECT_TRUE(get_datawriter_qos(qos_profile_, publisher_attributes_));
+  EXPECT_LE(depth, static_cast<size_t>(publisher_attributes_.topic.historyQos.depth));
+
+  if (max_depth < std::numeric_limits<size_t>::max()) {
+    qos_profile_.depth = max_depth + 1;
+    EXPECT_FALSE(get_datawriter_qos(qos_profile_, publisher_attributes_));
+  }
 }