From 34948215f891822a0dad7086a1ed7c7ac2849b24 Mon Sep 17 00:00:00 2001 From: Mario Dominguez Date: Fri, 7 Jun 2024 09:08:28 +0200 Subject: [PATCH 1/4] Refs #21054: Revert to const char* for flow controller names Signed-off-by: Mario Dominguez --- include/fastdds/dds/core/policy/QosPolicies.hpp | 4 ++-- include/fastdds/rtps/attributes/WriterAttributes.h | 2 +- .../fastdds/rtps/flowcontrol/FlowControllerDescriptor.hpp | 4 +--- test/unittest/dds/publisher/PublisherTests.cpp | 2 +- test/unittest/xmlparser/XMLProfileParserTests.cpp | 8 ++++---- 5 files changed, 9 insertions(+), 11 deletions(-) diff --git a/include/fastdds/dds/core/policy/QosPolicies.hpp b/include/fastdds/dds/core/policy/QosPolicies.hpp index c145395e4e9..26b5c0b6ac9 100644 --- a/include/fastdds/dds/core/policy/QosPolicies.hpp +++ b/include/fastdds/dds/core/policy/QosPolicies.hpp @@ -2048,7 +2048,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 { @@ -2060,7 +2060,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 index 3e1d4f33cf9..7c9dd2d5ac1 100644 --- 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 index 55d05780bf8..86db3e70a6e 100644 --- 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 - #include #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/test/unittest/dds/publisher/PublisherTests.cpp b/test/unittest/dds/publisher/PublisherTests.cpp index e3b26cc13b2..1b14b36dd1d 100644 --- a/test/unittest/dds/publisher/PublisherTests.cpp +++ b/test/unittest/dds/publisher/PublisherTests.cpp @@ -397,7 +397,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 index a3cd14e4e4b..218e7790fbe 100644 --- a/test/unittest/xmlparser/XMLProfileParserTests.cpp +++ b/test/unittest/xmlparser/XMLProfileParserTests.cpp @@ -885,7 +885,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); @@ -961,7 +961,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); @@ -1035,7 +1035,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); @@ -1109,7 +1109,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); From 0b5b994a71970bf7651f5b5190c702f0ac391f07 Mon Sep 17 00:00:00 2001 From: Mario Dominguez Date: Fri, 7 Jun 2024 09:09:21 +0200 Subject: [PATCH 2/4] Refs #21054: Handle flow controller names in a XMLParser collection Signed-off-by: Mario Dominguez --- include/fastrtps/xmlparser/XMLParser.h | 13 +++++++++++++ src/cpp/rtps/xmlparser/XMLElementParser.cpp | 16 +++++++++++----- src/cpp/rtps/xmlparser/XMLParser.cpp | 7 +++++++ src/cpp/rtps/xmlparser/XMLProfileManager.cpp | 3 +++ 4 files changed, 34 insertions(+), 5 deletions(-) diff --git a/include/fastrtps/xmlparser/XMLParser.h b/include/fastrtps/xmlparser/XMLParser.h index 30bc32c9f29..ddc50d3534d 100644 --- 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( @@ -706,6 +714,11 @@ class XMLParser eprosima::fastdds::rtps::BuiltinTransports* bt, eprosima::fastdds::rtps::BuiltinTransportsOptions* bt_opts, uint8_t ident); + +private: + + static std::mutex collections_mtx_; + static std::vector flow_controller_descriptor_names_; }; } // namespace xmlparser diff --git a/src/cpp/rtps/xmlparser/XMLElementParser.cpp b/src/cpp/rtps/xmlparser/XMLElementParser.cpp index 49608899c08..c313b40a04c 100644 --- a/src/cpp/rtps/xmlparser/XMLElementParser.cpp +++ b/src/cpp/rtps/xmlparser/XMLElementParser.cpp @@ -136,6 +136,9 @@ namespace eprosima { namespace fastrtps { namespace xmlparser { +std::mutex XMLParser::collections_mtx_; +std::vector XMLParser::flow_controller_descriptor_names_; + using namespace eprosima::fastrtps::rtps; using namespace eprosima::fastdds::xml::detail; @@ -1053,9 +1056,11 @@ XMLP_ret XMLParser::getXMLFlowControllerDescriptorList( if (strcmp(name, NAME) == 0) { + std::lock_guard lock(collections_mtx_); // name - stringType - flow_controller_descriptor->name = get_element_text(p_aux1); - if (flow_controller_descriptor->name.empty()) + flow_controller_descriptor_names_.emplace_back(get_element_text(p_aux1)); + flow_controller_descriptor->name = flow_controller_descriptor_names_.back().c_str(); + if (flow_controller_descriptor_names_.back().empty()) { EPROSIMA_LOG_ERROR(XMLPARSER, "<" << p_aux1->Value() << "> getXMLString XML_ERROR!"); return XMLP_ret::XML_ERROR; @@ -2839,9 +2844,10 @@ 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 lock(collections_mtx_); + flow_controller_descriptor_names_.emplace_back(get_element_text(p_aux0)); + publishMode.flow_controller_name = flow_controller_descriptor_names_.back().c_str(); + if (flow_controller_descriptor_names_.back().empty()) { EPROSIMA_LOG_ERROR(XMLPARSER, "Node '" << FLOW_CONTROLLER_NAME << "' without content"); return XMLP_ret::XML_ERROR; diff --git a/src/cpp/rtps/xmlparser/XMLParser.cpp b/src/cpp/rtps/xmlparser/XMLParser.cpp index 9f4206e4b08..1e4b410531a 100644 --- a/src/cpp/rtps/xmlparser/XMLParser.cpp +++ b/src/cpp/rtps/xmlparser/XMLParser.cpp @@ -2833,6 +2833,13 @@ XMLP_ret XMLParser::fillDataNode( return XMLP_ret::XML_OK; } +XMLP_ret XMLParser::clear() +{ + std::lock_guard 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 index c13e70eb711..ac331a59665 100644 --- 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(); } From 1c35ef6e32247c89fe748246d14bee5020750e59 Mon Sep 17 00:00:00 2001 From: Mario Dominguez Date: Fri, 7 Jun 2024 10:03:18 +0200 Subject: [PATCH 3/4] Refs #21054: Apply Miguel suggestion Signed-off-by: Mario Dominguez --- include/fastrtps/xmlparser/XMLParser.h | 2 +- src/cpp/rtps/xmlparser/XMLElementParser.cpp | 20 +++++++++++--------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/include/fastrtps/xmlparser/XMLParser.h b/include/fastrtps/xmlparser/XMLParser.h index ddc50d3534d..3ae0cc4f1bf 100644 --- a/include/fastrtps/xmlparser/XMLParser.h +++ b/include/fastrtps/xmlparser/XMLParser.h @@ -718,7 +718,7 @@ class XMLParser private: static std::mutex collections_mtx_; - static std::vector flow_controller_descriptor_names_; + static std::set flow_controller_descriptor_names_; }; } // namespace xmlparser diff --git a/src/cpp/rtps/xmlparser/XMLElementParser.cpp b/src/cpp/rtps/xmlparser/XMLElementParser.cpp index c313b40a04c..8021e390073 100644 --- a/src/cpp/rtps/xmlparser/XMLElementParser.cpp +++ b/src/cpp/rtps/xmlparser/XMLElementParser.cpp @@ -137,7 +137,7 @@ namespace fastrtps { namespace xmlparser { std::mutex XMLParser::collections_mtx_; -std::vector XMLParser::flow_controller_descriptor_names_; +std::set XMLParser::flow_controller_descriptor_names_; using namespace eprosima::fastrtps::rtps; using namespace eprosima::fastdds::xml::detail; @@ -1058,13 +1058,14 @@ XMLP_ret XMLParser::getXMLFlowControllerDescriptorList( { std::lock_guard lock(collections_mtx_); // name - stringType - flow_controller_descriptor_names_.emplace_back(get_element_text(p_aux1)); - flow_controller_descriptor->name = flow_controller_descriptor_names_.back().c_str(); - if (flow_controller_descriptor_names_.back().empty()) + auto element_inserted = flow_controller_descriptor_names_.insert(get_element_text(p_aux1)); + if (element_inserted.first == flow_controller_descriptor_names_.end() || + element_inserted.first->empty()) { - EPROSIMA_LOG_ERROR(XMLPARSER, "<" << p_aux1->Value() << "> getXMLString XML_ERROR!"); + EPROSIMA_LOG_ERROR(XMLPARSER, "Node flow controller node '" << NAME << "' invalid"); return XMLP_ret::XML_ERROR; } + flow_controller_descriptor->name = element_inserted.first->c_str(); name_defined = true; } else if (strcmp(name, SCHEDULER) == 0) @@ -2845,13 +2846,14 @@ XMLP_ret XMLParser::getXMLPublishModeQos( else if (strcmp(name, FLOW_CONTROLLER_NAME) == 0) { std::lock_guard lock(collections_mtx_); - flow_controller_descriptor_names_.emplace_back(get_element_text(p_aux0)); - publishMode.flow_controller_name = flow_controller_descriptor_names_.back().c_str(); - if (flow_controller_descriptor_names_.back().empty()) + auto element_inserted = flow_controller_descriptor_names_.insert(get_element_text(p_aux0)); + if (element_inserted.first == flow_controller_descriptor_names_.end() || + element_inserted.first->empty()) { - EPROSIMA_LOG_ERROR(XMLPARSER, "Node '" << FLOW_CONTROLLER_NAME << "' without content"); + EPROSIMA_LOG_ERROR(XMLPARSER, "Node '" << FLOW_CONTROLLER_NAME << "' invalid"); return XMLP_ret::XML_ERROR; } + publishMode.flow_controller_name = element_inserted.first->c_str(); } else { From 07afc3b31085fa801dc7937ff94403b1df0fad88 Mon Sep 17 00:00:00 2001 From: Mario Dominguez Date: Fri, 7 Jun 2024 10:30:53 +0200 Subject: [PATCH 4/4] Refs #21054: Apply second suggestion Signed-off-by: Mario Dominguez --- src/cpp/rtps/xmlparser/XMLElementParser.cpp | 27 +++++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/src/cpp/rtps/xmlparser/XMLElementParser.cpp b/src/cpp/rtps/xmlparser/XMLElementParser.cpp index 8021e390073..aaa1fb8f2b7 100644 --- a/src/cpp/rtps/xmlparser/XMLElementParser.cpp +++ b/src/cpp/rtps/xmlparser/XMLElementParser.cpp @@ -1058,11 +1058,17 @@ XMLP_ret XMLParser::getXMLFlowControllerDescriptorList( { std::lock_guard lock(collections_mtx_); // name - stringType - auto element_inserted = flow_controller_descriptor_names_.insert(get_element_text(p_aux1)); - if (element_inserted.first == flow_controller_descriptor_names_.end() || - element_inserted.first->empty()) + std::string element = get_element_text(p_aux1); + if (element.empty()) { - EPROSIMA_LOG_ERROR(XMLPARSER, "Node flow controller node '" << NAME << "' invalid"); + 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(); @@ -2846,11 +2852,16 @@ XMLP_ret XMLParser::getXMLPublishModeQos( else if (strcmp(name, FLOW_CONTROLLER_NAME) == 0) { std::lock_guard lock(collections_mtx_); - auto element_inserted = flow_controller_descriptor_names_.insert(get_element_text(p_aux0)); - if (element_inserted.first == flow_controller_descriptor_names_.end() || - element_inserted.first->empty()) + 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, "Node '" << FLOW_CONTROLLER_NAME << "' invalid"); + 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();