Author: Pavel Labath Date: 2025-06-24T11:11:35+02:00 New Revision: cf9546b826619890e965367a3e4d0da1991ba929
URL: https://github.com/llvm/llvm-project/commit/cf9546b826619890e965367a3e4d0da1991ba929 DIFF: https://github.com/llvm/llvm-project/commit/cf9546b826619890e965367a3e4d0da1991ba929.diff LOG: [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>. Added: Modified: lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h lldb/tools/lldb-server/lldb-platform.cpp lldb/unittests/Core/CommunicationTest.cpp lldb/unittests/Host/ConnectionFileDescriptorTest.cpp lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationTest.cpp lldb/unittests/tools/lldb-server/tests/TestClient.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h index b771f9c3f45a2..df0e196fea688 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 @@ class ConnectionFileDescriptor : public Connection { ConnectionFileDescriptor(int fd, bool owns_fd); - ConnectionFileDescriptor(Socket *socket); + ConnectionFileDescriptor(std::unique_ptr<Socket> socket_up); ~ConnectionFileDescriptor() override; @@ -136,8 +133,6 @@ class ConnectionFileDescriptor : public Connection { 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 ae935dda5af4e..57dce44812c89 100644 --- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp +++ b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp @@ -72,9 +72,11 @@ ConnectionFileDescriptor::ConnectionFileDescriptor(int fd, bool owns_fd) 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 @@ ConnectionStatus ConnectionFileDescriptor::ConnectSerialPort( #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 d1f57cc22d8bd..0d3ead840b080 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp @@ -1128,8 +1128,8 @@ Status GDBRemoteCommunication::StartDebugserverProcess( 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 @@ Status GDBRemoteCommunication::StartDebugserverProcess( 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 ee87629d9077b..fc86f801f0d8a 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h @@ -149,9 +149,6 @@ class GDBRemoteCommunication : public Communication { 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 37ec8455c63a7..3c79dc001f65e 100644 --- a/lldb/tools/lldb-server/lldb-platform.cpp +++ b/lldb/tools/lldb-server/lldb-platform.cpp @@ -485,8 +485,8 @@ int main_platform(int argc, char *argv[]) { 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 fd07b4da9f8c1..51e218af8baf4 100644 --- a/lldb/unittests/Core/CommunicationTest.cpp +++ b/lldb/unittests/Core/CommunicationTest.cpp @@ -41,7 +41,7 @@ static void CommunicationReadTest(bool use_read_thread) { 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 @@ TEST_F(CommunicationTest, SynchronizeWhileClosing) { 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 250ed6fe8fd37..171481acca01b 100644 --- a/lldb/unittests/Host/ConnectionFileDescriptorTest.cpp +++ b/lldb/unittests/Host/ConnectionFileDescriptorTest.cpp @@ -22,11 +22,11 @@ class ConnectionFileDescriptorTest : public testing::Test { 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 ed77e86ac3701..b28b8013bcdc7 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 @@ struct TestClient : public GDBRemoteClientBase { 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 96f377ec7dfac..f940229985887 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 @@ std::string one_register_hex = "41424344"; 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 3da1f0a718fc5..e96d587b10e25 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 TestClient : public GDBRemoteCommunication { 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 48e089bc0fcff..f3510cad22e7f 100644 --- a/lldb/unittests/tools/lldb-server/tests/TestClient.cpp +++ b/lldb/unittests/tools/lldb-server/tests/TestClient.cpp @@ -125,7 +125,8 @@ TestClient::launchCustom(StringRef Log, bool disable_stdio, 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()) _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
