Force aligned param length (#1059)

* Refs #7837. Remove friend class ParameterList from QosPolicies * Refs #7837. Added octet vector (de)serialization methods to QosPolicy. * Refs #7837. Using common base GenericDataQosPolicy for all custom data policies. * Refs #7837. Apply changes to mock QosPolicies.hpp * Refs #7877. Uncrustify. * Refs #7877. Use default on dtors. * Refs #7877. Assign length when assigning from collection. * Refs #7877. Add copy assignment operation on TemplateDataQosPolicy. * Refs #7841. Fixing blackbox tests. * Refs #7841. Addressed review comments.
eProsima · Mar 12, 2020 · 817b256 · 817b256
1 parent eeab863
commit 817b256
Show file tree

Hide file tree

Showing 11 changed files with 424 additions and 642 deletions.
diff --git a/include/fastdds/dds/core/policy/QosPolicies.hpp b/include/fastdds/dds/core/policy/QosPolicies.hpp
diff --git a/include/fastdds/dds/publisher/qos/PublisherQos.hpp b/include/fastdds/dds/publisher/qos/PublisherQos.hpp
@@ -87,7 +87,7 @@ class PublisherQos
     fastrtps::LifespanQosPolicy lifespan;
 
     //!UserData Qos, NOT implemented in the library.
-    fastrtps::UserDataQosPolicy user_data;
+    UserDataQosPolicy user_data;
 
     //!Time Based Filter Qos, NOT implemented in the library.
     fastrtps::TimeBasedFilterQosPolicy time_based_filter;

diff --git a/include/fastdds/dds/subscriber/qos/SubscriberQos.hpp b/include/fastdds/dds/subscriber/qos/SubscriberQos.hpp
@@ -85,7 +85,7 @@ class SubscriberQos
     fastrtps::DestinationOrderQosPolicy destination_order;
 
     //!UserData Qos, NOT implemented in the library.
-    fastrtps::UserDataQosPolicy user_data;
+    UserDataQosPolicy user_data;
 
     //!Time Based Filter Qos, NOT implemented in the library.
     fastrtps::TimeBasedFilterQosPolicy time_based_filter;

diff --git a/include/fastdds/dds/topic/qos/DataReaderQos.hpp b/include/fastdds/dds/topic/qos/DataReaderQos.hpp
@@ -62,7 +62,7 @@ class DataReaderQos
     fastrtps::ResourceLimitsQosPolicy resource_limits;
 
     //!User Data Qos, NOT implemented in the library.
-    fastrtps::UserDataQosPolicy user_data;
+    UserDataQosPolicy user_data;
 
     //!Ownership Qos, NOT implemented in the library.
     fastrtps::OwnershipQosPolicy ownership;

diff --git a/include/fastdds/dds/topic/qos/DataWriterQos.hpp b/include/fastdds/dds/topic/qos/DataWriterQos.hpp
@@ -71,7 +71,7 @@ class DataWriterQos
     fastrtps::LifespanQosPolicy lifespan;
 
     //!User Data Qos, implemented in the library.
-    fastrtps::UserDataQosPolicy user_data;
+    UserDataQosPolicy user_data;
 
     //!Ownership Qos, NOT implemented in the library.
     fastrtps::OwnershipQosPolicy ownership;

diff --git a/include/fastdds/rtps/builtin/data/ParticipantProxyData.h b/include/fastdds/rtps/builtin/data/ParticipantProxyData.h
@@ -133,7 +133,7 @@ class ParticipantProxyData
     //!
     ParameterPropertyList_t m_properties;
     //!
-    UserDataQosPolicy m_userData;
+    fastdds::dds::UserDataQosPolicy m_userData;
     //!
     TimedEvent* lease_duration_event;
     //!

diff --git a/include/fastrtps/qos/QosPolicies.h b/include/fastrtps/qos/QosPolicies.h
@@ -62,7 +62,6 @@ constexpr DestinationOrderQosPolicyKind BY_RECEPTION_TIMESTAMP_DESTINATIONORDER_
 constexpr DestinationOrderQosPolicyKind BY_SOURCE_TIMESTAMP_DESTINATIONORDER_QOS =
         DestinationOrderQosPolicyKind::BY_SOURCE_TIMESTAMP_DESTINATIONORDER_QOS;
 using DestinationOrderQosPolicy = fastdds::dds::DestinationOrderQosPolicy;
-using UserDataQosPolicy = fastdds::dds::UserDataQosPolicy;
 using TimeBasedFilterQosPolicy = fastdds::dds::TimeBasedFilterQosPolicy;
 using PresentationQosPolicyAccessScopeKind = fastdds::dds::PresentationQosPolicyAccessScopeKind;
 constexpr PresentationQosPolicyAccessScopeKind INSTANCE_PRESENTATION_QOS =
@@ -73,8 +72,8 @@ constexpr PresentationQosPolicyAccessScopeKind GROUP_PRESENTATION_QOS =
         PresentationQosPolicyAccessScopeKind::GROUP_PRESENTATION_QOS;
 using PresentationQosPolicy = fastdds::dds::PresentationQosPolicy;
 using PartitionQosPolicy = fastdds::dds::PartitionQosPolicy;
-using TopicDataQosPolicy = fastdds::dds::TopicDataQosPolicy;
-using GroupDataQosPolicy = fastdds::dds::GroupDataQosPolicy;
+using TopicDataQosPolicy = fastdds::dds::TemplateDataQosPolicy<fastdds::dds::PID_TOPIC_DATA>;
+using GroupDataQosPolicy = fastdds::dds::TemplateDataQosPolicy<fastdds::dds::PID_GROUP_DATA>;
 using HistoryQosPolicyKind = fastdds::dds::HistoryQosPolicyKind;
 constexpr HistoryQosPolicyKind KEEP_LAST_HISTORY_QOS = HistoryQosPolicyKind::KEEP_LAST_HISTORY_QOS;
 constexpr HistoryQosPolicyKind KEEP_ALL_HISTORY_QOS = HistoryQosPolicyKind::KEEP_ALL_HISTORY_QOS;

diff --git a/include/fastrtps/xmlparser/XMLParser.h b/include/fastrtps/xmlparser/XMLParser.h
@@ -397,7 +397,7 @@ class XMLParser
 
     RTPS_DllAPI static XMLP_ret getXMLUserDataQos(
             tinyxml2::XMLElement* elem,
-            UserDataQosPolicy& userData,
+            fastdds::dds::UserDataQosPolicy& userData,
             uint8_t ident);
 
     RTPS_DllAPI static XMLP_ret getXMLLifespanQos(

diff --git a/src/cpp/dds/core/policy/QosPolicies.cpp b/src/cpp/dds/core/policy/QosPolicies.cpp
@@ -40,6 +40,55 @@ uint32_t QosPolicy::get_cdr_serialized_size(
     return 2 + 2 + 4 + data_size;
 }
 
+bool QosPolicy::serialize_generic_data(
+        CDRMessage_t* msg,
+        uint16_t pid,
+        const std::vector<fastrtps::rtps::octet>& data)
+{
+    bool valid = CDRMessage::addUInt16(msg, pid);
+    uint16_t siz = static_cast<uint16_t>(data.size());
+    siz = (siz + 3) & ~3;
+    valid &= CDRMessage::addUInt16(msg, static_cast<uint16_t>(4 + siz));
+    valid &= CDRMessage::addOctetVector(msg, &data, true);
+    return valid;
+}
+
+bool QosPolicy::deserialize_generic_data(
+        CDRMessage_t* msg,
+        uint16_t size,
+        size_t max_size,
+        std::vector<fastrtps::rtps::octet>& data)
+{
+    uint32_t pos_ref = msg->pos;
+
+    // Read size of data
+    uint32_t len;
+    if (!CDRMessage::readUInt32(msg, &len))
+    {
+        return false;
+    }
+
+    if ( (len + sizeof(uint32_t) > size)  // Exceeds parameter length
+        || (len > max_size) )             // Exceeds size limit
+    {
+        return false;
+    }
+
+    // Either the data is size limited and already has max_size() allocated
+    // or it is not limited and we resize if needed
+    data.resize(len);
+    if (!CDRMessage::readData(msg, data.data(), len))
+    {
+        return false;
+    }
+
+    // Skip padding
+    msg->pos += ( (len + 3) & ~3) - len;
+
+    // Should have consumed whole size
+    return (pos_ref + size == msg->pos);
+}
+
 bool DurabilityQosPolicy::addToCDRMessage(
         CDRMessage_t* msg) const
 {
@@ -372,82 +421,6 @@ bool PartitionQosPolicy::readFromCDRMessage(
     return valid;
 }
 
-bool UserDataQosPolicy::addToCDRMessage(
-        CDRMessage_t* msg) const
-{
-    bool valid = CDRMessage::addUInt16(msg, Pid);
-    uint32_t siz = (uint32_t)size();
-    uint32_t align = ((siz + 3) & ~3) - siz;
-    valid &= CDRMessage::addUInt16(msg, static_cast<uint16_t>(4 + siz));
-    valid &= CDRMessage::addUInt32(msg, siz);
-    valid &= CDRMessage::addData(msg, collection_.data(), siz);
-    for (uint32_t count = 0; count < align; ++count)
-    {
-        valid &= CDRMessage::addOctet(msg, 0);
-    }
-
-    return valid;
-}
-
-bool UserDataQosPolicy::readFromCDRMessage(
-        CDRMessage_t* msg,
-        uint16_t size)
-{
-    if (size > max_size())
-    {
-        return false;
-    }
-    length = size;
-
-    //Either the data is size limited and already has max_size() allocated
-    // or it is not limited and readOctedVector will resize if needed
-    return CDRMessage::readOctetVector(msg, &collection_);
-}
-
-bool TopicDataQosPolicy::addToCDRMessage(
-        CDRMessage_t* msg) const
-{
-    bool valid = CDRMessage::addUInt16(msg, this->Pid);
-    valid &= CDRMessage::addUInt16(msg, this->length);
-    valid &= CDRMessage::addOctetVector(msg, &value);
-    return valid;
-}
-
-bool TopicDataQosPolicy::readFromCDRMessage(
-        CDRMessage_t* msg,
-        uint16_t size)
-{
-    length = size;
-
-    uint32_t pos_ref = msg->pos;
-    bool valid = CDRMessage::readOctetVector(msg, &value);
-    uint32_t length_diff = msg->pos - pos_ref;
-    valid &= (size == length_diff);
-    return valid;
-}
-
-bool GroupDataQosPolicy::addToCDRMessage(
-        CDRMessage_t* msg) const
-{
-    bool valid = CDRMessage::addUInt16(msg, this->Pid);
-    valid &= CDRMessage::addUInt16(msg, this->length);
-    valid &= CDRMessage::addOctetVector(msg, &value);
-    return valid;
-}
-
-bool GroupDataQosPolicy::readFromCDRMessage(
-        CDRMessage_t* msg,
-        uint16_t size)
-{
-    length = size;
-
-    uint32_t pos_ref = msg->pos;
-    bool valid = CDRMessage::readOctetVector(msg, &value);
-    uint32_t length_diff = msg->pos - pos_ref;
-    valid &= (size == length_diff);
-    return valid;
-}
-
 bool HistoryQosPolicy::addToCDRMessage(
         CDRMessage_t* msg) const
 {

diff --git a/test/blackbox/BlackboxTestsPubSubBasic.cpp b/test/blackbox/BlackboxTestsPubSubBasic.cpp
@@ -337,7 +337,7 @@ TEST_P(PubSubBasic, ReceivedDynamicDataWithinSizeLimit)
             R"(<profiles>
                 <participant><rtps>
                     <allocation>
-                        <max_user_data> 8 </max_user_data>
+                        <max_user_data> 4 </max_user_data>
                         <max_partitions> 28 </max_partitions>
                     </allocation>
                 </rtps></participant>
@@ -379,7 +379,7 @@ TEST_P(PubSubBasic, ReceivedUserDataExceedsSizeLimit)
             R"(<profiles>
                 <participant><rtps>
                     <allocation>
-                        <max_user_data> 8 </max_user_data>
+                        <max_user_data> 4 </max_user_data>
                     </allocation>
                 </rtps></participant>
             </profiles>)";