[lldb] Remove GDBRemoteCommunication::ConnectLocally (#145293)
Originally added for reproducers, it is now only used for test code.
While we could make it a test helper, I think that after #145015 it is
simple enough to not be needed.
Also squeeze in a change to make ConnectionFileDescriptor accept a
unique_ptr<Socket>.
diff --git a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
index b771f9c..df0e196 100644
--- a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
+++ b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
@@ -18,13 +18,10 @@
#include "lldb/Host/Pipe.h"
#include "lldb/Host/Socket.h"
#include "lldb/Utility/Connection.h"
-#include "lldb/Utility/IOObject.h"
namespace lldb_private {
class Status;
-class Socket;
-class SocketAddress;
class ConnectionFileDescriptor : public Connection {
public:
@@ -35,7 +32,7 @@
ConnectionFileDescriptor(int fd, bool owns_fd);
- ConnectionFileDescriptor(Socket *socket);
+ ConnectionFileDescriptor(std::unique_ptr<Socket> socket_up);
~ConnectionFileDescriptor() override;
@@ -136,8 +133,6 @@
std::string m_uri;
private:
- void InitializeSocket(Socket *socket);
-
ConnectionFileDescriptor(const ConnectionFileDescriptor &) = delete;
const ConnectionFileDescriptor &
operator=(const ConnectionFileDescriptor &) = delete;
diff --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
index ae935dd..57dce44 100644
--- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -72,9 +72,11 @@
OpenCommandPipe();
}
-ConnectionFileDescriptor::ConnectionFileDescriptor(Socket *socket)
- : Connection(), m_pipe(), m_mutex(), m_shutting_down(false) {
- InitializeSocket(socket);
+ConnectionFileDescriptor::ConnectionFileDescriptor(
+ std::unique_ptr<Socket> socket_up)
+ : m_shutting_down(false) {
+ m_uri = socket_up->GetRemoteConnectionURI();
+ m_io_sp = std::move(socket_up);
}
ConnectionFileDescriptor::~ConnectionFileDescriptor() {
@@ -796,8 +798,3 @@
#endif // LLDB_ENABLE_POSIX
llvm_unreachable("this function should be only called w/ LLDB_ENABLE_POSIX");
}
-
-void ConnectionFileDescriptor::InitializeSocket(Socket *socket) {
- m_io_sp.reset(socket);
- m_uri = socket->GetRemoteConnectionURI();
-}
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
index d1f57cc..0d3ead8 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -1128,8 +1128,8 @@
Socket *accepted_socket = nullptr;
error = sock_up->Accept(/*timeout=*/std::nullopt, accepted_socket);
if (accepted_socket) {
- SetConnection(
- std::make_unique<ConnectionFileDescriptor>(accepted_socket));
+ SetConnection(std::make_unique<ConnectionFileDescriptor>(
+ std::unique_ptr<Socket>(accepted_socket)));
}
}
@@ -1138,20 +1138,6 @@
void GDBRemoteCommunication::DumpHistory(Stream &strm) { m_history.Dump(strm); }
-llvm::Error
-GDBRemoteCommunication::ConnectLocally(GDBRemoteCommunication &client,
- GDBRemoteCommunication &server) {
- llvm::Expected<Socket::Pair> pair = Socket::CreatePair();
- if (!pair)
- return pair.takeError();
-
- client.SetConnection(
- std::make_unique<ConnectionFileDescriptor>(pair->first.release()));
- server.SetConnection(
- std::make_unique<ConnectionFileDescriptor>(pair->second.release()));
- return llvm::Error::success();
-}
-
GDBRemoteCommunication::ScopedTimeout::ScopedTimeout(
GDBRemoteCommunication &gdb_comm, std::chrono::seconds timeout)
: m_gdb_comm(gdb_comm), m_saved_timeout(0), m_timeout_modified(false) {
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
index ee87629..fc86f80 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
@@ -149,9 +149,6 @@
void DumpHistory(Stream &strm);
- static llvm::Error ConnectLocally(GDBRemoteCommunication &client,
- GDBRemoteCommunication &server);
-
/// Expand GDB run-length encoding.
static std::optional<std::string> ExpandRLE(std::string);
diff --git a/lldb/tools/lldb-server/lldb-platform.cpp b/lldb/tools/lldb-server/lldb-platform.cpp
index 37ec845..3c79dc0 100644
--- a/lldb/tools/lldb-server/lldb-platform.cpp
+++ b/lldb/tools/lldb-server/lldb-platform.cpp
@@ -485,8 +485,8 @@
GDBRemoteCommunicationServerPlatform platform(socket->GetSocketProtocol(),
gdbserver_port);
- platform.SetConnection(std::unique_ptr<Connection>(
- new ConnectionFileDescriptor(socket.release())));
+ platform.SetConnection(
+ std::make_unique<ConnectionFileDescriptor>(std::move(socket)));
client_handle(platform, inferior_arguments);
return 0;
}
diff --git a/lldb/unittests/Core/CommunicationTest.cpp b/lldb/unittests/Core/CommunicationTest.cpp
index fd07b4d..51e218a 100644
--- a/lldb/unittests/Core/CommunicationTest.cpp
+++ b/lldb/unittests/Core/CommunicationTest.cpp
@@ -41,7 +41,7 @@
ThreadedCommunication comm("test");
comm.SetConnection(
- std::make_unique<ConnectionFileDescriptor>(pair->second.release()));
+ std::make_unique<ConnectionFileDescriptor>(std::move(pair->second)));
comm.SetCloseOnEOF(true);
if (use_read_thread) {
@@ -126,7 +126,7 @@
ThreadedCommunication comm("test");
comm.SetConnection(
- std::make_unique<ConnectionFileDescriptor>(pair->second.release()));
+ std::make_unique<ConnectionFileDescriptor>(std::move(pair->second)));
comm.SetCloseOnEOF(true);
ASSERT_TRUE(comm.StartReadThread());
diff --git a/lldb/unittests/Host/ConnectionFileDescriptorTest.cpp b/lldb/unittests/Host/ConnectionFileDescriptorTest.cpp
index 250ed6f..171481a 100644
--- a/lldb/unittests/Host/ConnectionFileDescriptorTest.cpp
+++ b/lldb/unittests/Host/ConnectionFileDescriptorTest.cpp
@@ -22,11 +22,11 @@
std::unique_ptr<TCPSocket> socket_a_up;
std::unique_ptr<TCPSocket> socket_b_up;
CreateTCPConnectedSockets(ip, &socket_a_up, &socket_b_up);
- auto *socket = socket_a_up.release();
- ConnectionFileDescriptor connection_file_descriptor(socket);
+ uint16_t socket_a_remote_port = socket_a_up->GetRemotePortNumber();
+ ConnectionFileDescriptor connection_file_descriptor(std::move(socket_a_up));
std::string uri(connection_file_descriptor.GetURI());
- EXPECT_EQ((URI{"connect", ip, socket->GetRemotePortNumber(), "/"}),
+ EXPECT_EQ((URI{"connect", ip, socket_a_remote_port, "/"}),
*URI::Parse(uri));
}
};
diff --git a/lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp b/lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
index ed77e86..b28b801 100644
--- a/lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
+++ b/lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
@@ -10,6 +10,7 @@
#include "GDBRemoteTestUtils.h"
#include "Plugins/Process/Utility/LinuxSignals.h"
#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h"
+#include "lldb/Host/ConnectionFileDescriptor.h"
#include "lldb/Utility/GDBRemote.h"
#include "lldb/Utility/Listener.h"
#include "llvm/ADT/StringRef.h"
@@ -51,8 +52,12 @@
class GDBRemoteClientBaseTest : public GDBRemoteTest {
public:
void SetUp() override {
- ASSERT_THAT_ERROR(GDBRemoteCommunication::ConnectLocally(client, server),
- llvm::Succeeded());
+ llvm::Expected<Socket::Pair> pair = Socket::CreatePair();
+ ASSERT_THAT_EXPECTED(pair, llvm::Succeeded());
+ client.SetConnection(
+ std::make_unique<ConnectionFileDescriptor>(std::move(pair->first)));
+ server.SetConnection(
+ std::make_unique<ConnectionFileDescriptor>(std::move(pair->second)));
ASSERT_EQ(TestClient::eBroadcastBitRunPacketSent,
listener_sp->StartListeningForEvents(
&client, TestClient::eBroadcastBitRunPacketSent));
diff --git a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
index 96f377e..f940229 100644
--- a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -8,6 +8,7 @@
#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h"
#include "GDBRemoteTestUtils.h"
#include "lldb/Core/ModuleSpec.h"
+#include "lldb/Host/ConnectionFileDescriptor.h"
#include "lldb/Host/XML.h"
#include "lldb/Target/MemoryRegionInfo.h"
#include "lldb/Utility/DataBuffer.h"
@@ -63,8 +64,12 @@
class GDBRemoteCommunicationClientTest : public GDBRemoteTest {
public:
void SetUp() override {
- ASSERT_THAT_ERROR(GDBRemoteCommunication::ConnectLocally(client, server),
- llvm::Succeeded());
+ llvm::Expected<Socket::Pair> pair = Socket::CreatePair();
+ ASSERT_THAT_EXPECTED(pair, llvm::Succeeded());
+ client.SetConnection(
+ std::make_unique<ConnectionFileDescriptor>(std::move(pair->first)));
+ server.SetConnection(
+ std::make_unique<ConnectionFileDescriptor>(std::move(pair->second)));
}
protected:
diff --git a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationTest.cpp b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationTest.cpp
index 3da1f0a..e96d587 100644
--- a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationTest.cpp
+++ b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationTest.cpp
@@ -5,7 +5,9 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
+
#include "GDBRemoteTestUtils.h"
+#include "lldb/Host/ConnectionFileDescriptor.h"
#include "llvm/Testing/Support/Error.h"
using namespace lldb_private::process_gdb_remote;
@@ -28,8 +30,12 @@
class GDBRemoteCommunicationTest : public GDBRemoteTest {
public:
void SetUp() override {
- ASSERT_THAT_ERROR(GDBRemoteCommunication::ConnectLocally(client, server),
- llvm::Succeeded());
+ llvm::Expected<Socket::Pair> pair = Socket::CreatePair();
+ ASSERT_THAT_EXPECTED(pair, llvm::Succeeded());
+ client.SetConnection(
+ std::make_unique<ConnectionFileDescriptor>(std::move(pair->first)));
+ server.SetConnection(
+ std::make_unique<ConnectionFileDescriptor>(std::move(pair->second)));
}
protected:
diff --git a/lldb/unittests/tools/lldb-server/tests/TestClient.cpp b/lldb/unittests/tools/lldb-server/tests/TestClient.cpp
index 48e089b..f3510ca 100644
--- a/lldb/unittests/tools/lldb-server/tests/TestClient.cpp
+++ b/lldb/unittests/tools/lldb-server/tests/TestClient.cpp
@@ -125,7 +125,8 @@
listen_socket.Accept(2 * GetDefaultTimeout(), accept_socket)
.takeError())
return E;
- auto Conn = std::make_unique<ConnectionFileDescriptor>(accept_socket);
+ auto Conn = std::make_unique<ConnectionFileDescriptor>(
+ std::unique_ptr<Socket>(accept_socket));
auto Client = std::unique_ptr<TestClient>(new TestClient(std::move(Conn)));
if (Error E = Client->initializeConnection())
