From fee461b1d830564048c09aef90631b1d4be4e450 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Micha=C5=82=20G=C3=B3rny?= Date: Fri, 8 Oct 2021 11:54:25 +0200 Subject: [PATCH] [lldb] [ConnectionFileDescriptorPosix] Combine m_read_sp & m_write_sp Combine m_read_sp and m_write_sp into a single m_io_sp. In all currently existing code paths, they are pointing to the same object anyway. Differential Revision: https://reviews.llvm.org/D111396 --- .../Host/posix/ConnectionFileDescriptorPosix.h | 5 +- .../Host/posix/ConnectionFileDescriptorPosix.cpp | 79 ++++++++-------------- 2 files changed, 32 insertions(+), 52 deletions(-) diff --git a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h index 57bb01d..720c019 100644 --- a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h +++ b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h @@ -55,7 +55,7 @@ public: bool InterruptRead() override; - lldb::IOObjectSP GetReadObject() override { return m_read_sp; } + lldb::IOObjectSP GetReadObject() override { return m_io_sp; } uint16_t GetListeningPort(const Timeout &timeout); @@ -88,8 +88,7 @@ protected: lldb::ConnectionStatus ConnectFile(llvm::StringRef args, Status *error_ptr); - lldb::IOObjectSP m_read_sp; - lldb::IOObjectSP m_write_sp; + lldb::IOObjectSP m_io_sp; Predicate m_port_predicate; // Used when binding to port zero to wait for the thread diff --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp index b192c65..8d17ae0 100644 --- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp +++ b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp @@ -63,9 +63,8 @@ ConnectionFileDescriptor::ConnectionFileDescriptor(bool child_processes_inherit) ConnectionFileDescriptor::ConnectionFileDescriptor(int fd, bool owns_fd) : Connection(), m_pipe(), m_mutex(), m_shutting_down(false), m_waiting_for_accept(false), m_child_processes_inherit(false) { - m_write_sp = + m_io_sp = std::make_shared(fd, File::eOpenOptionReadWrite, owns_fd); - m_read_sp = m_write_sp; Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION | LIBLLDB_LOG_OBJECT)); @@ -120,8 +119,7 @@ void ConnectionFileDescriptor::CloseCommandPipe() { } bool ConnectionFileDescriptor::IsConnected() const { - return (m_read_sp && m_read_sp->IsValid()) || - (m_write_sp && m_write_sp->IsValid()); + return m_io_sp && m_io_sp->IsValid(); } ConnectionStatus ConnectionFileDescriptor::Connect(llvm::StringRef path, @@ -191,9 +189,8 @@ ConnectionStatus ConnectionFileDescriptor::Disconnect(Status *error_ptr) { return eConnectionStatusSuccess; } - if (m_read_sp && m_read_sp->IsValid() && - m_read_sp->GetFdType() == IOObject::eFDTypeSocket) - static_cast(*m_read_sp).PreDisconnect(); + if (m_io_sp->GetFdType() == IOObject::eFDTypeSocket) + static_cast(*m_io_sp).PreDisconnect(); // Try to get the ConnectionFileDescriptor's mutex. If we fail, that is // quite likely because somebody is doing a blocking read on our file @@ -222,12 +219,11 @@ ConnectionStatus ConnectionFileDescriptor::Disconnect(Status *error_ptr) { // Prevents reads and writes during shutdown. m_shutting_down = true; - Status error = m_read_sp->Close(); - Status error2 = m_write_sp->Close(); - if (error.Fail() || error2.Fail()) + Status error = m_io_sp->Close(); + if (error.Fail()) status = eConnectionStatusError; if (error_ptr) - *error_ptr = error.Fail() ? error : error2; + *error_ptr = error; // Close any pipes we were using for async interrupts m_pipe.Close(); @@ -269,14 +265,14 @@ size_t ConnectionFileDescriptor::Read(void *dst, size_t dst_len, Status error; size_t bytes_read = dst_len; - error = m_read_sp->Read(dst, bytes_read); + error = m_io_sp->Read(dst, bytes_read); if (log) { LLDB_LOGF(log, "%p ConnectionFileDescriptor::Read() fd = %" PRIu64 ", dst = %p, dst_len = %" PRIu64 ") => %" PRIu64 ", error = %s", static_cast(this), - static_cast(m_read_sp->GetWaitableHandle()), + static_cast(m_io_sp->GetWaitableHandle()), static_cast(dst), static_cast(dst_len), static_cast(bytes_read), error.AsCString()); } @@ -295,7 +291,7 @@ size_t ConnectionFileDescriptor::Read(void *dst, size_t dst_len, switch (error_value) { case EAGAIN: // The file was marked for non-blocking I/O, and no data were // ready to be read. - if (m_read_sp->GetFdType() == IOObject::eFDTypeSocket) + if (m_io_sp->GetFdType() == IOObject::eFDTypeSocket) status = eConnectionStatusTimedOut; else status = eConnectionStatusSuccess; @@ -373,14 +369,14 @@ size_t ConnectionFileDescriptor::Write(const void *src, size_t src_len, Status error; size_t bytes_sent = src_len; - error = m_write_sp->Write(src, bytes_sent); + error = m_io_sp->Write(src, bytes_sent); if (log) { LLDB_LOGF(log, "%p ConnectionFileDescriptor::Write(fd = %" PRIu64 ", src = %p, src_len = %" PRIu64 ") => %" PRIu64 " (error = %s)", static_cast(this), - static_cast(m_write_sp->GetWaitableHandle()), + static_cast(m_io_sp->GetWaitableHandle()), static_cast(src), static_cast(src_len), static_cast(bytes_sent), error.AsCString()); } @@ -443,7 +439,7 @@ ConnectionFileDescriptor::BytesAvailable(const Timeout &timeout, // Make a copy of the file descriptors to make sure we don't have another // thread change these values out from under us and cause problems in the // loop below where like in FS_SET() - const IOObject::WaitableHandle handle = m_read_sp->GetWaitableHandle(); + const IOObject::WaitableHandle handle = m_io_sp->GetWaitableHandle(); const int pipe_fd = m_pipe.GetReadFileDescriptor(); if (handle != IOObject::kInvalidHandleValue) { @@ -464,7 +460,7 @@ ConnectionFileDescriptor::BytesAvailable(const Timeout &timeout, if (have_pipe_fd) select_helper.FDSetRead(pipe_fd); - while (handle == m_read_sp->GetWaitableHandle()) { + while (handle == m_io_sp->GetWaitableHandle()) { Status error = select_helper.Select(); @@ -533,11 +529,9 @@ ConnectionFileDescriptor::NamedSocketAccept(llvm::StringRef socket_name, Socket::UnixDomainAccept(socket_name, m_child_processes_inherit, socket); if (error_ptr) *error_ptr = error; - m_write_sp.reset(socket); - m_read_sp = m_write_sp; - if (error.Fail()) { + m_io_sp.reset(socket); + if (error.Fail()) return eConnectionStatusError; - } m_uri.assign(std::string(socket_name)); return eConnectionStatusSuccess; } @@ -550,11 +544,9 @@ ConnectionFileDescriptor::NamedSocketConnect(llvm::StringRef socket_name, Socket::UnixDomainConnect(socket_name, m_child_processes_inherit, socket); if (error_ptr) *error_ptr = error; - m_write_sp.reset(socket); - m_read_sp = m_write_sp; - if (error.Fail()) { + m_io_sp.reset(socket); + if (error.Fail()) return eConnectionStatusError; - } m_uri.assign(std::string(socket_name)); return eConnectionStatusSuccess; } @@ -567,11 +559,9 @@ ConnectionFileDescriptor::UnixAbstractSocketConnect(llvm::StringRef socket_name, m_child_processes_inherit, socket); if (error_ptr) *error_ptr = error; - m_write_sp.reset(socket); - m_read_sp = m_write_sp; - if (error.Fail()) { + m_io_sp.reset(socket); + if (error.Fail()) return eConnectionStatusError; - } m_uri.assign(std::string(socket_name)); return eConnectionStatusSuccess; } @@ -622,8 +612,7 @@ ConnectionStatus ConnectionFileDescriptor::ConnectTCP(llvm::StringRef s, socket.takeError(), "tcp connect failed: {0}"); return eConnectionStatusError; } - m_write_sp = std::move(*socket); - m_read_sp = m_write_sp; + m_io_sp = std::move(*socket); m_uri.assign(std::string(s)); return eConnectionStatusSuccess; } @@ -642,8 +631,7 @@ ConnectionStatus ConnectionFileDescriptor::ConnectUDP(llvm::StringRef s, socket.takeError(), "tcp connect failed: {0}"); return eConnectionStatusError; } - m_write_sp = std::move(*socket); - m_read_sp = m_write_sp; + m_io_sp = std::move(*socket); m_uri.assign(std::string(s)); return eConnectionStatusSuccess; } @@ -665,8 +653,7 @@ ConnectionStatus ConnectionFileDescriptor::ConnectFD(llvm::StringRef s, if (error_ptr) error_ptr->SetErrorStringWithFormat("stale file descriptor: %s", s.str().c_str()); - m_read_sp.reset(); - m_write_sp.reset(); + m_io_sp.reset(); return eConnectionStatusError; } else { // Don't take ownership of a file descriptor that gets passed to us @@ -684,14 +671,11 @@ ConnectionStatus ConnectionFileDescriptor::ConnectFD(llvm::StringRef s, int resuse; bool is_socket = !!tcp_socket->GetOption(SOL_SOCKET, SO_REUSEADDR, resuse); - if (is_socket) { - m_read_sp = std::move(tcp_socket); - m_write_sp = m_read_sp; - } else { - m_read_sp = + if (is_socket) + m_io_sp = std::move(tcp_socket); + else + m_io_sp = std::make_shared(fd, File::eOpenOptionReadWrite, false); - m_write_sp = m_read_sp; - } m_uri = s.str(); return eConnectionStatusSuccess; } @@ -700,8 +684,7 @@ ConnectionStatus ConnectionFileDescriptor::ConnectFD(llvm::StringRef s, if (error_ptr) error_ptr->SetErrorStringWithFormat("invalid file descriptor: \"%s\"", s.str().c_str()); - m_read_sp.reset(); - m_write_sp.reset(); + m_io_sp.reset(); return eConnectionStatusError; #endif // LLDB_ENABLE_POSIX llvm_unreachable("this function should be only called w/ LLDB_ENABLE_POSIX"); @@ -745,9 +728,8 @@ ConnectionStatus ConnectionFileDescriptor::ConnectFile(llvm::StringRef s, ::fcntl(fd, F_SETFL, flags); } } - m_read_sp = + m_io_sp = std::make_shared(fd, File::eOpenOptionReadWrite, true); - m_write_sp = m_read_sp; return eConnectionStatusSuccess; #endif // LLDB_ENABLE_POSIX llvm_unreachable("this function should be only called w/ LLDB_ENABLE_POSIX"); @@ -769,7 +751,6 @@ void ConnectionFileDescriptor::SetChildProcessesInherit( } void ConnectionFileDescriptor::InitializeSocket(Socket *socket) { - m_write_sp.reset(socket); - m_read_sp = m_write_sp; + m_io_sp.reset(socket); m_uri = socket->GetRemoteConnectionURI(); } -- 2.7.4