From 451741a9d778a260ceee608a26b5fdf2d9926982 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Thu, 2 Apr 2020 14:40:59 +0200 Subject: [PATCH] [lldb] Change Communication::SetConnection to take a unique_ptr The function takes ownership of the object. This makes that explicit, and avoids unowned pointers floating around. --- lldb/include/lldb/Core/Communication.h | 2 +- lldb/source/API/SBCommunication.cpp | 5 ++-- lldb/source/Core/Communication.cpp | 4 +-- .../gdb-server/PlatformRemoteGDBServer.cpp | 2 +- .../Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp | 2 +- .../Process/gdb-remote/GDBRemoteCommunication.cpp | 7 +++--- .../GDBRemoteCommunicationServerLLGS.cpp | 6 ++--- .../gdb-remote/GDBRemoteCommunicationServerLLGS.h | 2 +- .../Process/gdb-remote/ProcessGDBRemote.cpp | 5 ++-- .../Python/ScriptInterpreterPython.cpp | 2 +- lldb/source/Target/Process.cpp | 29 +++++++++------------- lldb/tools/lldb-server/lldb-platform.cpp | 2 +- .../Process/gdb-remote/GDBRemoteTestUtils.h | 2 +- .../tools/lldb-server/tests/TestClient.cpp | 2 +- 14 files changed, 35 insertions(+), 37 deletions(-) diff --git a/lldb/include/lldb/Core/Communication.h b/lldb/include/lldb/Core/Communication.h index 048541a..4233d5b 100644 --- a/lldb/include/lldb/Core/Communication.h +++ b/lldb/include/lldb/Core/Communication.h @@ -221,7 +221,7 @@ public: /// /// \see /// class Connection - void SetConnection(Connection *connection); + void SetConnection(std::unique_ptr connection); /// Starts a read thread whose sole purpose it to read bytes from the /// current connection. This function will call connection's read function: diff --git a/lldb/source/API/SBCommunication.cpp b/lldb/source/API/SBCommunication.cpp index 7c7ba39..d55ecd3 100644 --- a/lldb/source/API/SBCommunication.cpp +++ b/lldb/source/API/SBCommunication.cpp @@ -63,7 +63,7 @@ ConnectionStatus SBCommunication::Connect(const char *url) { if (m_opaque) { if (!m_opaque->HasConnection()) - m_opaque->SetConnection(Host::CreateDefaultConnection(url).release()); + m_opaque->SetConnection(Host::CreateDefaultConnection(url)); return m_opaque->Connect(url, nullptr); } return eConnectionStatusNoConnection; @@ -79,7 +79,8 @@ ConnectionStatus SBCommunication::AdoptFileDesriptor(int fd, bool owns_fd) { if (m_opaque->IsConnected()) m_opaque->Disconnect(); } - m_opaque->SetConnection(new ConnectionFileDescriptor(fd, owns_fd)); + m_opaque->SetConnection( + std::make_unique(fd, owns_fd)); if (m_opaque->IsConnected()) status = eConnectionStatusSuccess; else diff --git a/lldb/source/Core/Communication.cpp b/lldb/source/Core/Communication.cpp index 88f6a21e..f416384 100644 --- a/lldb/source/Core/Communication.cpp +++ b/lldb/source/Core/Communication.cpp @@ -399,10 +399,10 @@ void Communication::SynchronizeWithReadThread() { listener_sp->GetEvent(event_sp, llvm::None); } -void Communication::SetConnection(Connection *connection) { +void Communication::SetConnection(std::unique_ptr connection) { Disconnect(nullptr); StopReadThread(nullptr); - m_connection_sp.reset(connection); + m_connection_sp = std::move(connection); } const char * diff --git a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp index b046755..657b8fd 100644 --- a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp +++ b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp @@ -290,7 +290,7 @@ Status PlatformRemoteGDBServer::ConnectRemote(Args &args) { GetHostname()); } else { if (args.GetArgumentCount() == 1) { - m_gdb_client.SetConnection(new ConnectionFileDescriptor()); + m_gdb_client.SetConnection(std::make_unique()); // we're going to reuse the hostname when we connect to the debugserver int port; std::string path; diff --git a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp index 09d1965..5b728a5 100644 --- a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp +++ b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp @@ -250,7 +250,7 @@ Status ProcessKDP::DoConnectRemote(Stream *strm, llvm::StringRef remote_url) { const uint16_t reply_port = socket.GetLocalPortNumber(); if (reply_port != 0) { - m_comm.SetConnection(conn_up.release()); + m_comm.SetConnection(std::move(conn_up)); if (m_comm.SendRequestReattach(reply_port)) { if (m_comm.SendRequestConnect(reply_port, reply_port, diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp index 73bd9fb..070220e 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp @@ -869,7 +869,7 @@ Status GDBRemoteCommunication::StartListenThread(const char *hostname, else snprintf(listen_url, sizeof(listen_url), "listen://%i", port); m_listen_url = listen_url; - SetConnection(new ConnectionFileDescriptor()); + SetConnection(std::make_unique()); llvm::Expected listen_thread = ThreadLauncher::LaunchThread( listen_url, GDBRemoteCommunication::ListenThread, this); if (!listen_thread) @@ -1252,11 +1252,12 @@ GDBRemoteCommunication::ConnectLocally(GDBRemoteCommunication &client, return llvm::createStringError(llvm::inconvertibleErrorCode(), "Unable to connect: %s", status.AsCString()); - client.SetConnection(conn_up.release()); + client.SetConnection(std::move(conn_up)); if (llvm::Error error = accept_status.get().ToError()) return error; - server.SetConnection(new ConnectionFileDescriptor(accept_socket)); + server.SetConnection( + std::make_unique(accept_socket)); return llvm::Error::success(); } diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp index 508bc2a..7d6cb2a 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp @@ -1015,9 +1015,9 @@ void GDBRemoteCommunicationServerLLGS::DataAvailableCallback() { } Status GDBRemoteCommunicationServerLLGS::InitializeConnection( - std::unique_ptr &&connection) { + std::unique_ptr connection) { IOObjectSP read_object_sp = connection->GetReadObject(); - GDBRemoteCommunicationServer::SetConnection(connection.release()); + GDBRemoteCommunicationServer::SetConnection(std::move(connection)); Status error; m_network_handle_up = m_mainloop.RegisterReadObject( @@ -1053,7 +1053,7 @@ Status GDBRemoteCommunicationServerLLGS::SetSTDIOFileDescriptor(int fd) { } m_stdio_communication.SetCloseOnEOF(false); - m_stdio_communication.SetConnection(conn_up.release()); + m_stdio_communication.SetConnection(std::move(conn_up)); if (!m_stdio_communication.IsConnected()) { error.SetErrorString( "failed to set connection for inferior I/O communication"); diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h index ba2fc37..f2cb3a8 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h @@ -67,7 +67,7 @@ public: void DidExec(NativeProcessProtocol *process) override; - Status InitializeConnection(std::unique_ptr &&connection); + Status InitializeConnection(std::unique_ptr connection); protected: MainLoop &m_mainloop; diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index a3c19f7..618ed7c 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -960,7 +960,7 @@ Status ProcessGDBRemote::ConnectToDebugserver(llvm::StringRef connect_url) { uint32_t retry_count = 0; while (!m_gdb_comm.IsConnected()) { if (conn_up->Connect(connect_url, &error) == eConnectionStatusSuccess) { - m_gdb_comm.SetConnection(conn_up.release()); + m_gdb_comm.SetConnection(std::move(conn_up)); break; } else if (error.WasInterrupted()) { // If we were interrupted, don't keep retrying. @@ -3482,7 +3482,8 @@ Status ProcessGDBRemote::LaunchAndConnectToDebugserver( // Our process spawned correctly, we can now set our connection to use // our end of the socket pair cleanup_our.release(); - m_gdb_comm.SetConnection(new ConnectionFileDescriptor(our_socket, true)); + m_gdb_comm.SetConnection( + std::make_unique(our_socket, true)); #endif StartAsyncThread(); } diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp index f59b70a..ee94a18 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp +++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp @@ -953,7 +953,7 @@ bool ScriptInterpreterPythonImpl::ExecuteOneLine( true)); #endif if (conn_up->IsConnected()) { - output_comm.SetConnection(conn_up.release()); + output_comm.SetConnection(std::move(conn_up)); output_comm.SetReadThreadBytesReceivedCallback( ReadThreadBytesReceived, &result->GetOutputStream()); output_comm.StartReadThread(); diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index af990c9..7dac2be 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -4428,23 +4428,18 @@ protected: void Process::SetSTDIOFileDescriptor(int fd) { // First set up the Read Thread for reading/handling process I/O - - std::unique_ptr conn_up( - new ConnectionFileDescriptor(fd, true)); - - if (conn_up) { - m_stdio_communication.SetConnection(conn_up.release()); - if (m_stdio_communication.IsConnected()) { - m_stdio_communication.SetReadThreadBytesReceivedCallback( - STDIOReadThreadBytesReceived, this); - m_stdio_communication.StartReadThread(); - - // Now read thread is set up, set up input reader. - - if (!m_process_input_reader) - m_process_input_reader = - std::make_shared(this, fd); - } + m_stdio_communication.SetConnection( + std::make_unique(fd, true)); + if (m_stdio_communication.IsConnected()) { + m_stdio_communication.SetReadThreadBytesReceivedCallback( + STDIOReadThreadBytesReceived, this); + m_stdio_communication.StartReadThread(); + + // Now read thread is set up, set up input reader. + + if (!m_process_input_reader) + m_process_input_reader = + std::make_shared(this, fd); } } diff --git a/lldb/tools/lldb-server/lldb-platform.cpp b/lldb/tools/lldb-server/lldb-platform.cpp index d045660..33f918f 100644 --- a/lldb/tools/lldb-server/lldb-platform.cpp +++ b/lldb/tools/lldb-server/lldb-platform.cpp @@ -343,7 +343,7 @@ int main_platform(int argc, char *argv[]) { // connections while a connection is active. acceptor_up.reset(); } - platform.SetConnection(conn); + platform.SetConnection(std::unique_ptr(conn)); if (platform.IsConnected()) { if (inferior_arguments.GetArgumentCount() > 0) { diff --git a/lldb/unittests/Process/gdb-remote/GDBRemoteTestUtils.h b/lldb/unittests/Process/gdb-remote/GDBRemoteTestUtils.h index 0a48f34..27ce6b9 100644 --- a/lldb/unittests/Process/gdb-remote/GDBRemoteTestUtils.h +++ b/lldb/unittests/Process/gdb-remote/GDBRemoteTestUtils.h @@ -77,7 +77,7 @@ public: class MockServerWithMockConnection : public MockServer { public: MockServerWithMockConnection() : MockServer() { - SetConnection(new MockConnection(m_packets)); + SetConnection(std::make_unique(m_packets)); } llvm::ArrayRef GetPackets() { return m_packets; }; diff --git a/lldb/unittests/tools/lldb-server/tests/TestClient.cpp b/lldb/unittests/tools/lldb-server/tests/TestClient.cpp index b5a5ab3..52428e4 100644 --- a/lldb/unittests/tools/lldb-server/tests/TestClient.cpp +++ b/lldb/unittests/tools/lldb-server/tests/TestClient.cpp @@ -30,7 +30,7 @@ using namespace llgs_tests; #endif TestClient::TestClient(std::unique_ptr Conn) { - SetConnection(Conn.release()); + SetConnection(std::move(Conn)); SetPacketTimeout(std::chrono::seconds(10)); } -- 2.7.4