Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[22469] Avoid calling setIPv4 in TCPTransportInterface #5492

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions include/fastdds/utils/IPLocator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,16 @@ class IPLocator
const Locator_t& loc2,
bool fullAddress = false);

/**
* Copies the whole address from one locator to another.
* @param loc1 Locator to copy from.
* @param loc2 Locator to copy to.
* @return True if the copy was successful.
*/
FASTDDS_EXPORTED_API static bool copyAddress(
const Locator_t& loc1,
Locator_t& loc2);

//! Checks if a both locators has the same IP address and physical port (as in RTCP protocol).
FASTDDS_EXPORTED_API static bool compareAddressAndPhysicalPort(
const Locator_t& loc1,
Expand Down
4 changes: 2 additions & 2 deletions src/cpp/rtps/transport/TCPTransportInterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ ResponseCode TCPTransportInterface::bind_socket(
Locator local_locator(channel->locator());
for (auto& interface_it : local_interfaces)
{
IPLocator::setIPv4(local_locator, interface_it.locator);
IPLocator::copyAddress(interface_it.locator, local_locator);
channel_resources_.insert(decltype(channel_resources_)::value_type{local_locator, channel});
}
}
Expand Down Expand Up @@ -1030,7 +1030,7 @@ bool TCPTransportInterface::CreateInitialConnect(
Locator local_locator(physical_locator);
for (auto& interface_it : local_interfaces)
{
IPLocator::setIPv4(local_locator, interface_it.locator);
IPLocator::copyAddress(interface_it.locator, local_locator);
channel_resources_[local_locator] = channel;
}
}
Expand Down
21 changes: 21 additions & 0 deletions src/cpp/utils/IPLocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -985,6 +985,27 @@ bool IPLocator::compareAddress(
}
}

bool IPLocator::copyAddress(
const Locator_t& loc1,
Locator_t& loc2)
{
if (loc1.kind != loc2.kind)
{
return false;
}

if (loc1.kind == LOCATOR_KIND_UDPv4 || loc1.kind == LOCATOR_KIND_TCPv4)
{
memcpy(loc2.address, loc1.address, 16 * sizeof(char));
return true;
}
else if (loc1.kind == LOCATOR_KIND_UDPv6 || loc1.kind == LOCATOR_KIND_TCPv6)
{
return copyIPv6(loc1, loc2.address);
}
return false;
}

bool IPLocator::compareAddressAndPhysicalPort(
const Locator_t& loc1,
const Locator_t& loc2)
Expand Down
1 change: 1 addition & 0 deletions test/unittest/transport/TCPv4Tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2035,6 +2035,7 @@ TEST_F(TCPv4Tests, client_announced_local_port_uniqueness)

std::this_thread::sleep_for(std::chrono::milliseconds(100));

EXPECT_GT(receiveTransportUnderTest.get_channel_resources().size(), 2u);
std::set<std::shared_ptr<TCPChannelResource>> channels_created;
for (const auto& channel_resource : receiveTransportUnderTest.get_channel_resources())
{
Expand Down
8 changes: 7 additions & 1 deletion test/unittest/transport/TCPv6Tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,13 @@ TEST_F(TCPv6Tests, client_announced_local_port_uniqueness)

std::this_thread::sleep_for(std::chrono::milliseconds(100));

ASSERT_EQ(receiveTransportUnderTest.get_channel_resources().size(), 2u);
EXPECT_GT(receiveTransportUnderTest.get_channel_resources().size(), 2u);
std::set<std::shared_ptr<TCPChannelResource>> channels_created;
for (const auto& channel_resource : receiveTransportUnderTest.get_channel_resources())
{
channels_created.insert(channel_resource.second);
}
ASSERT_EQ(channels_created.size(), 2u);
}

#ifndef _WIN32
Expand Down
31 changes: 31 additions & 0 deletions test/unittest/utils/LocatorTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,37 @@ TEST_F(IPLocatorTests, copyIPv6)
ASSERT_EQ(arr[15], 1u);
}

/*
* Check to copy an address
*/
TEST_F(IPLocatorTests, copyAddress)
{
// Copy IPv4
Locator_t locator1(LOCATOR_KIND_UDPv4);
Locator_t locator2(LOCATOR_KIND_UDPv4);
IPLocator::setIPv4(locator1, ipv4_lo_address);
ASSERT_FALSE(IPLocator::compareAddress(locator1, locator2));
ASSERT_TRUE(IPLocator::copyAddress(locator1, locator2));
ASSERT_TRUE(IPLocator::compareAddress(locator1, locator2));

// Check cannot copy between different kinds
locator1.kind = LOCATOR_KIND_UDPv6;
ASSERT_FALSE(IPLocator::copyAddress(locator1, locator2));

// Copy IPv6
locator2.kind = LOCATOR_KIND_UDPv6;
IPLocator::setIPv6(locator1, ipv6_lo_address);
ASSERT_FALSE(IPLocator::compareAddress(locator1, locator2));
ASSERT_TRUE(IPLocator::copyAddress(locator1, locator2));
ASSERT_TRUE(IPLocator::compareAddress(locator1, locator2));

// Check cannot copy between SHM locators
locator1.kind = LOCATOR_KIND_SHM;
Locator_t locator3(LOCATOR_KIND_SHM);
ASSERT_FALSE(IPLocator::copyAddress(locator1, locator3));
ASSERT_FALSE(IPLocator::compareAddress(locator1, locator3));
}
MiguelCompany marked this conversation as resolved.
Show resolved Hide resolved

/*
* Check to set ip of any kind
*/
Expand Down
Loading