Bugfix: Revert XML Flow controller names to const char* (#4911)

* Refs #21054: Revert to const char* for flow controller names Signed-off-by: Mario Dominguez <[email protected]> * Refs #21054: Handle flow controller names in a XMLParser collection Signed-off-by: Mario Dominguez <[email protected]> * Refs #21054: Apply Miguel suggestion Signed-off-by: Mario Dominguez <[email protected]> * Refs #21054: Apply second suggestion Signed-off-by: Mario Dominguez <[email protected]> --------- Signed-off-by: Mario Dominguez <[email protected]>
eProsima · Jun 10, 2024 · c81a13e · c81a13e
1 parent 2237970
commit c81a13e
Show file tree

Hide file tree

Showing 9 changed files with 57 additions and 17 deletions.
diff --git a/include/fastdds/dds/core/policy/QosPolicies.hpp b/include/fastdds/dds/core/policy/QosPolicies.hpp
@@ -2046,7 +2046,7 @@ class PublishModeQosPolicy : public QosPolicy
      *
      * @since 2.4.0
      */
-    std::string flow_controller_name = fastdds::rtps::FASTDDS_FLOW_CONTROLLER_DEFAULT;
+    const char* flow_controller_name = fastdds::rtps::FASTDDS_FLOW_CONTROLLER_DEFAULT;
 
     inline void clear() override
     {
@@ -2058,7 +2058,7 @@ class PublishModeQosPolicy : public QosPolicy
             const PublishModeQosPolicy& b) const
     {
         return (this->kind == b.kind) &&
-               flow_controller_name == b.flow_controller_name.c_str() &&
+               0 == strcmp(flow_controller_name, b.flow_controller_name) &&
                QosPolicy::operator ==(b);
     }
 

diff --git a/include/fastdds/rtps/attributes/WriterAttributes.h b/include/fastdds/rtps/attributes/WriterAttributes.h
@@ -143,7 +143,7 @@ class WriterAttributes
     Duration_t keep_duration;
 
     //! Flow controller name. Default: fastdds::rtps::FASTDDS_FLOW_CONTROLLER_DEFAULT.
-    std::string flow_controller_name = fastdds::rtps::FASTDDS_FLOW_CONTROLLER_DEFAULT;
+    const char* flow_controller_name = fastdds::rtps::FASTDDS_FLOW_CONTROLLER_DEFAULT;
 };
 
 } /* namespace rtps */

diff --git a/include/fastdds/rtps/flowcontrol/FlowControllerDescriptor.hpp b/include/fastdds/rtps/flowcontrol/FlowControllerDescriptor.hpp
@@ -15,8 +15,6 @@
 #ifndef FASTDDS_RTPS_FLOWCONTROL_FLOWCONTROLLERDESCRIPTOR_HPP
 #define FASTDDS_RTPS_FLOWCONTROL_FLOWCONTROLLERDESCRIPTOR_HPP
 
-#include <string>
-
 #include <fastdds/rtps/attributes/ThreadSettings.hpp>
 
 #include "FlowControllerConsts.hpp"
@@ -35,7 +33,7 @@ namespace rtps {
 struct FlowControllerDescriptor
 {
     //! Name of the flow controller.
-    std::string name = FASTDDS_FLOW_CONTROLLER_DEFAULT;
+    const char* name = FASTDDS_FLOW_CONTROLLER_DEFAULT;
 
     //! Scheduler policy used by the flow controller.
     //!

diff --git a/include/fastrtps/xmlparser/XMLParser.h b/include/fastrtps/xmlparser/XMLParser.h
@@ -174,6 +174,14 @@ class XMLParser
     RTPS_DllAPI static XMLP_ret loadXMLDynamicTypes(
             tinyxml2::XMLElement& types);
 
+
+    /**
+     * Clears the private static collections.
+     *
+     * @return XMLP_ret::XML_OK on success, XMLP_ret::XML_ERROR in other case.
+     */
+    RTPS_DllAPI static XMLP_ret clear();
+
 protected:
 
     RTPS_DllAPI static XMLP_ret parseXML(
@@ -693,6 +701,11 @@ class XMLParser
             tinyxml2::XMLElement* elem,
             eprosima::fastdds::rtps::BuiltinTransports* bt,
             uint8_t ident);
+
+private:
+
+    static std::mutex collections_mtx_;
+    static std::set<std::string> flow_controller_descriptor_names_;
 };
 
 } // namespace xmlparser

diff --git a/src/cpp/rtps/xmlparser/XMLElementParser.cpp b/src/cpp/rtps/xmlparser/XMLElementParser.cpp
@@ -135,6 +135,9 @@ namespace eprosima {
 namespace fastrtps {
 namespace xmlparser {
 
+std::mutex XMLParser::collections_mtx_;
+std::set<std::string> XMLParser::flow_controller_descriptor_names_;
+
 using namespace eprosima::fastrtps::rtps;
 using namespace eprosima::fastdds::xml::detail;
 
@@ -1052,13 +1055,22 @@ XMLP_ret XMLParser::getXMLFlowControllerDescriptorList(
 
             if (strcmp(name, NAME) == 0)
             {
+                std::lock_guard<std::mutex> lock(collections_mtx_);
                 // name - stringType
-                flow_controller_descriptor->name = get_element_text(p_aux1);
-                if (flow_controller_descriptor->name.empty())
+                std::string element = get_element_text(p_aux1);
+                if (element.empty())
                 {
-                    EPROSIMA_LOG_ERROR(XMLPARSER, "<" << p_aux1->Value() << "> getXMLString XML_ERROR!");
+                    EPROSIMA_LOG_ERROR(XMLPARSER, "Node '" << NAME << "' without content");
+                    return XMLP_ret::XML_ERROR;
+                }
+                auto element_inserted = flow_controller_descriptor_names_.insert(element);
+                if (element_inserted.first == flow_controller_descriptor_names_.end())
+                {
+                    EPROSIMA_LOG_ERROR(XMLPARSER,
+                            "Insertion error for flow controller node '" << FLOW_CONTROLLER_NAME << "'");
                     return XMLP_ret::XML_ERROR;
                 }
+                flow_controller_descriptor->name = element_inserted.first->c_str();
                 name_defined = true;
             }
             else if (strcmp(name, SCHEDULER) == 0)
@@ -2838,13 +2850,20 @@ XMLP_ret XMLParser::getXMLPublishModeQos(
         }
         else if (strcmp(name, FLOW_CONTROLLER_NAME) == 0)
         {
-
-            publishMode.flow_controller_name = get_element_text(p_aux0);
-            if (publishMode.flow_controller_name.empty())
+            std::lock_guard<std::mutex> lock(collections_mtx_);
+            std::string element = get_element_text(p_aux0);
+            if (element.empty())
             {
                 EPROSIMA_LOG_ERROR(XMLPARSER, "Node '" << FLOW_CONTROLLER_NAME << "' without content");
                 return XMLP_ret::XML_ERROR;
             }
+            auto element_inserted = flow_controller_descriptor_names_.insert(element);
+            if (element_inserted.first == flow_controller_descriptor_names_.end())
+            {
+                EPROSIMA_LOG_ERROR(XMLPARSER, "Insertion error for node '" << FLOW_CONTROLLER_NAME << "'");
+                return XMLP_ret::XML_ERROR;
+            }
+            publishMode.flow_controller_name = element_inserted.first->c_str();
         }
         else
         {

diff --git a/src/cpp/rtps/xmlparser/XMLParser.cpp b/src/cpp/rtps/xmlparser/XMLParser.cpp
@@ -2595,6 +2595,13 @@ XMLP_ret XMLParser::fillDataNode(
     return XMLP_ret::XML_OK;
 }
 
+XMLP_ret XMLParser::clear()
+{
+    std::lock_guard<std::mutex> lock(collections_mtx_);
+    flow_controller_descriptor_names_.clear();
+    return XMLP_ret::XML_OK;
+}
+
 } // namespace xmlparser
 } // namespace fastrtps
 } // namespace eprosima
diff --git a/src/cpp/rtps/xmlparser/XMLProfileManager.cpp b/src/cpp/rtps/xmlparser/XMLProfileManager.cpp
@@ -854,4 +854,7 @@ void XMLProfileManager::DeleteInstance()
         }
         dynamic_types_.clear();
     }
+
+    // Clear XML Parser collections
+    XMLParser::clear();
 }
diff --git a/test/unittest/dds/publisher/PublisherTests.cpp b/test/unittest/dds/publisher/PublisherTests.cpp
@@ -400,7 +400,7 @@ TEST(PublisherTests, ChangeDefaultDataWriterQos)
     EXPECT_FALSE(wqos.writer_data_lifecycle().autodispose_unregistered_instances);
     // .publish_mode
     EXPECT_EQ(eprosima::fastdds::dds::ASYNCHRONOUS_PUBLISH_MODE, wqos.publish_mode().kind);
-    EXPECT_EQ(true, wqos.publish_mode().flow_controller_name == "Prueba");
+    EXPECT_EQ(0, strcmp(wqos.publish_mode().flow_controller_name, "Prueba"));
     count = 1;
     for (auto prop : wqos.properties().properties())
     {

diff --git a/test/unittest/xmlparser/XMLProfileParserTests.cpp b/test/unittest/xmlparser/XMLProfileParserTests.cpp
@@ -889,7 +889,7 @@ TEST_P(XMLProfileParserTests, XMLParserPublisher)
     EXPECT_EQ(pub_qos.m_partition.names()[0], "partition_name_a");
     EXPECT_EQ(pub_qos.m_partition.names()[1], "partition_name_b");
     EXPECT_EQ(pub_qos.m_publishMode.kind, ASYNCHRONOUS_PUBLISH_MODE);
-    EXPECT_EQ(pub_qos.m_publishMode.flow_controller_name, "test_flow_controller");
+    EXPECT_EQ(0, strcmp(pub_qos.m_publishMode.flow_controller_name, "test_flow_controller"));
     EXPECT_EQ(pub_times.initialHeartbeatDelay, c_TimeZero);
     EXPECT_EQ(pub_times.heartbeatPeriod.seconds, 11);
     EXPECT_EQ(pub_times.heartbeatPeriod.nanosec, 32u);
@@ -965,7 +965,7 @@ TEST_F(XMLProfileParserBasicTests, XMLParserPublisherDeprecated)
     EXPECT_EQ(pub_qos.m_partition.names()[0], "partition_name_a");
     EXPECT_EQ(pub_qos.m_partition.names()[1], "partition_name_b");
     EXPECT_EQ(pub_qos.m_publishMode.kind, ASYNCHRONOUS_PUBLISH_MODE);
-    EXPECT_EQ(pub_qos.m_publishMode.flow_controller_name, "test_flow_controller");
+    EXPECT_EQ(0, strcmp(pub_qos.m_publishMode.flow_controller_name, "test_flow_controller"));
     EXPECT_EQ(pub_times.initialHeartbeatDelay, c_TimeZero);
     EXPECT_EQ(pub_times.heartbeatPeriod.seconds, 11);
     EXPECT_EQ(pub_times.heartbeatPeriod.nanosec, 32u);
@@ -1039,7 +1039,7 @@ TEST_P(XMLProfileParserTests, XMLParserDefaultPublisherProfile)
     EXPECT_EQ(pub_qos.m_partition.names()[0], "partition_name_a");
     EXPECT_EQ(pub_qos.m_partition.names()[1], "partition_name_b");
     EXPECT_EQ(pub_qos.m_publishMode.kind, ASYNCHRONOUS_PUBLISH_MODE);
-    EXPECT_EQ(pub_qos.m_publishMode.flow_controller_name, "test_flow_controller");
+    EXPECT_EQ(0, strcmp(pub_qos.m_publishMode.flow_controller_name, "test_flow_controller"));
     EXPECT_EQ(pub_times.initialHeartbeatDelay, c_TimeZero);
     EXPECT_EQ(pub_times.heartbeatPeriod.seconds, 11);
     EXPECT_EQ(pub_times.heartbeatPeriod.nanosec, 32u);
@@ -1113,7 +1113,7 @@ TEST_F(XMLProfileParserBasicTests, XMLParserDefaultPublisherProfileDeprecated)
     EXPECT_EQ(pub_qos.m_partition.names()[0], "partition_name_a");
     EXPECT_EQ(pub_qos.m_partition.names()[1], "partition_name_b");
     EXPECT_EQ(pub_qos.m_publishMode.kind, ASYNCHRONOUS_PUBLISH_MODE);
-    EXPECT_EQ(pub_qos.m_publishMode.flow_controller_name, "test_flow_controller");
+    EXPECT_EQ(0, strcmp(pub_qos.m_publishMode.flow_controller_name, "test_flow_controller"));
     EXPECT_EQ(pub_times.initialHeartbeatDelay, c_TimeZero);
     EXPECT_EQ(pub_times.heartbeatPeriod.seconds, 11);
     EXPECT_EQ(pub_times.heartbeatPeriod.nanosec, 32u);