Skip to content

Commit

Permalink
Bugfix: Revert XML Flow controller names to const char* (#4911)
Browse files Browse the repository at this point in the history 
* Refs #21054: Revert to const char* for flow controller names

Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>

* Refs #21054: Handle flow controller names in a XMLParser collection

Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>

* Refs #21054: Apply Miguel suggestion

Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>

* Refs #21054: Apply second suggestion

Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>

---------

Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>
  • Loading branch information
@Mario-DL @MiguelCompany
Mario-DL authored and MiguelCompany committed Sep 6, 2024
1 parent d570a6b commit ffb1e0a
Show file tree
Hide file tree
Showing 9 changed files with 57 additions and 17 deletions.
4 changes: 2 additions & 2 deletions include/fastdds/dds/core/policy/QosPolicies.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2047,7 +2047,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
{
Expand All @@ -2059,7 +2059,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);
}

Expand Down
2 changes: 1 addition & 1 deletion include/fastdds/rtps/attributes/WriterAttributes.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
4 changes: 1 addition & 3 deletions include/fastdds/rtps/flowcontrol/FlowControllerDescriptor.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
#ifndef FASTDDS_RTPS_FLOWCONTROL_FLOWCONTROLLERDESCRIPTOR_HPP
#define FASTDDS_RTPS_FLOWCONTROL_FLOWCONTROLLERDESCRIPTOR_HPP

#include <string>

#include "FlowControllerConsts.hpp"
#include "FlowControllerSchedulerPolicy.hpp"

Expand All @@ -33,7 +31,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.
//!
Expand Down
13 changes: 13 additions & 0 deletions include/fastrtps/xmlparser/XMLParser.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,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(
Expand Down Expand Up @@ -627,6 +635,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
Expand Down
31 changes: 25 additions & 6 deletions src/cpp/rtps/xmlparser/XMLElementParser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@

#include <utils/string_utilities.hpp>

std::mutex XMLParser::collections_mtx_;
std::set<std::string> XMLParser::flow_controller_descriptor_names_;

using namespace eprosima::fastrtps;
using namespace eprosima::fastrtps::rtps;
using namespace eprosima::fastrtps::xmlparser;
Expand Down Expand Up @@ -848,13 +851,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)
Expand Down Expand Up @@ -2663,13 +2675,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
{
Expand Down
7 changes: 7 additions & 0 deletions src/cpp/rtps/xmlparser/XMLParser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2328,6 +2328,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
3 changes: 3 additions & 0 deletions src/cpp/rtps/xmlparser/XMLProfileManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -779,4 +779,7 @@ void XMLProfileManager::DeleteInstance()
}
dynamic_types_.clear();
}

// Clear XML Parser collections
XMLParser::clear();
}
2 changes: 1 addition & 1 deletion test/unittest/dds/publisher/PublisherTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,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())
{
Expand Down
8 changes: 4 additions & 4 deletions test/unittest/xmlparser/XMLProfileParserTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,7 @@ TEST_F(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);
Expand Down Expand Up @@ -724,7 +724,7 @@ TEST_F(XMLProfileParserTests, 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);
Expand Down Expand Up @@ -798,7 +798,7 @@ TEST_F(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);
Expand Down Expand Up @@ -872,7 +872,7 @@ TEST_F(XMLProfileParserTests, 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);
Expand Down

0 comments on commit ffb1e0a

Please sign in to comment.