Author: Pavel Labath Date: 2020-04-23T14:20:26+02:00 New Revision: c9e6b7010c6998b6df50ff376425d1d4e4937bea
URL: https://github.com/llvm/llvm-project/commit/c9e6b7010c6998b6df50ff376425d1d4e4937bea DIFF: https://github.com/llvm/llvm-project/commit/c9e6b7010c6998b6df50ff376425d1d4e4937bea.diff LOG: [lldb/Host] Modernize some socket functions return Expected<Socket> instead of a Status object plus a Socket*& argument. Added: Modified: lldb/include/lldb/Host/Socket.h lldb/include/lldb/Host/common/UDPSocket.h lldb/source/Host/common/Socket.cpp lldb/source/Host/common/UDPSocket.cpp lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp lldb/unittests/Host/SocketTest.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Host/Socket.h b/lldb/include/lldb/Host/Socket.h index 77562276d352..36db0ec63e9d 100644 --- a/lldb/include/lldb/Host/Socket.h +++ b/lldb/include/lldb/Host/Socket.h @@ -36,6 +36,8 @@ typedef SOCKET NativeSocket; #else typedef int NativeSocket; #endif +class TCPSocket; +class UDPSocket; class Socket : public IOObject { public: @@ -64,13 +66,16 @@ class Socket : public IOObject { // Initialize a Tcp Socket object in listening mode. listen and accept are // implemented separately because the caller may wish to manipulate or query // the socket after it is initialized, but before entering a blocking accept. - static Status TcpListen(llvm::StringRef host_and_port, - bool child_processes_inherit, Socket *&socket, - Predicate<uint16_t> *predicate, int backlog = 5); - static Status TcpConnect(llvm::StringRef host_and_port, - bool child_processes_inherit, Socket *&socket); - static Status UdpConnect(llvm::StringRef host_and_port, - bool child_processes_inherit, Socket *&socket); + static llvm::Expected<std::unique_ptr<TCPSocket>> + TcpListen(llvm::StringRef host_and_port, bool child_processes_inherit, + Predicate<uint16_t> *predicate, int backlog = 5); + + static llvm::Expected<std::unique_ptr<Socket>> + TcpConnect(llvm::StringRef host_and_port, bool child_processes_inherit); + + static llvm::Expected<std::unique_ptr<UDPSocket>> + UdpConnect(llvm::StringRef host_and_port, bool child_processes_inherit); + static Status UnixDomainConnect(llvm::StringRef host_and_port, bool child_processes_inherit, Socket *&socket); diff --git a/lldb/include/lldb/Host/common/UDPSocket.h b/lldb/include/lldb/Host/common/UDPSocket.h index b3db4fa29f6e..bae707e345d8 100644 --- a/lldb/include/lldb/Host/common/UDPSocket.h +++ b/lldb/include/lldb/Host/common/UDPSocket.h @@ -16,8 +16,8 @@ class UDPSocket : public Socket { public: UDPSocket(bool should_close, bool child_processes_inherit); - static Status Connect(llvm::StringRef name, bool child_processes_inherit, - Socket *&socket); + static llvm::Expected<std::unique_ptr<UDPSocket>> + Connect(llvm::StringRef name, bool child_processes_inherit); std::string GetRemoteConnectionURI() const override; diff --git a/lldb/source/Host/common/Socket.cpp b/lldb/source/Host/common/Socket.cpp index 45c2f9eb1e41..4bcf34a6b456 100644 --- a/lldb/source/Host/common/Socket.cpp +++ b/lldb/source/Host/common/Socket.cpp @@ -147,71 +147,65 @@ std::unique_ptr<Socket> Socket::Create(const SocketProtocol protocol, return socket_up; } -Status Socket::TcpConnect(llvm::StringRef host_and_port, - bool child_processes_inherit, Socket *&socket) { - Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_COMMUNICATION)); - LLDB_LOGF(log, "Socket::%s (host/port = %s)", __FUNCTION__, - host_and_port.str().c_str()); +llvm::Expected<std::unique_ptr<Socket>> +Socket::TcpConnect(llvm::StringRef host_and_port, + bool child_processes_inherit) { + Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION)); + LLDB_LOG(log, "host_and_port = {0}", host_and_port); Status error; std::unique_ptr<Socket> connect_socket( Create(ProtocolTcp, child_processes_inherit, error)); if (error.Fail()) - return error; + return error.ToError(); error = connect_socket->Connect(host_and_port); if (error.Success()) - socket = connect_socket.release(); + return std::move(connect_socket); - return error; + return error.ToError(); } -Status Socket::TcpListen(llvm::StringRef host_and_port, - bool child_processes_inherit, Socket *&socket, - Predicate<uint16_t> *predicate, int backlog) { +llvm::Expected<std::unique_ptr<TCPSocket>> +Socket::TcpListen(llvm::StringRef host_and_port, bool child_processes_inherit, + Predicate<uint16_t> *predicate, int backlog) { Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION)); - LLDB_LOGF(log, "Socket::%s (%s)", __FUNCTION__, host_and_port.str().c_str()); + LLDB_LOG(log, "host_and_port = {0}", host_and_port); Status error; std::string host_str; std::string port_str; int32_t port = INT32_MIN; if (!DecodeHostAndPort(host_and_port, host_str, port_str, port, &error)) - return error; + return error.ToError(); std::unique_ptr<TCPSocket> listen_socket( new TCPSocket(true, child_processes_inherit)); - if (error.Fail()) - return error; error = listen_socket->Listen(host_and_port, backlog); - if (error.Success()) { - // We were asked to listen on port zero which means we must now read the - // actual port that was given to us as port zero is a special code for - // "find an open port for me". - if (port == 0) - port = listen_socket->GetLocalPortNumber(); - - // Set the port predicate since when doing a listen://<host>:<port> it - // often needs to accept the incoming connection which is a blocking system - // call. Allowing access to the bound port using a predicate allows us to - // wait for the port predicate to be set to a non-zero value from another - // thread in an efficient manor. - if (predicate) - predicate->SetValue(port, eBroadcastAlways); - socket = listen_socket.release(); - } - - return error; + if (error.Fail()) + return error.ToError(); + + // We were asked to listen on port zero which means we must now read the + // actual port that was given to us as port zero is a special code for + // "find an open port for me". + if (port == 0) + port = listen_socket->GetLocalPortNumber(); + + // Set the port predicate since when doing a listen://<host>:<port> it + // often needs to accept the incoming connection which is a blocking system + // call. Allowing access to the bound port using a predicate allows us to + // wait for the port predicate to be set to a non-zero value from another + // thread in an efficient manor. + if (predicate) + predicate->SetValue(port, eBroadcastAlways); + return std::move(listen_socket); } -Status Socket::UdpConnect(llvm::StringRef host_and_port, - bool child_processes_inherit, Socket *&socket) { - Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION)); - LLDB_LOGF(log, "Socket::%s (host/port = %s)", __FUNCTION__, - host_and_port.str().c_str()); - - return UDPSocket::Connect(host_and_port, child_processes_inherit, socket); +llvm::Expected<std::unique_ptr<UDPSocket>> +Socket::UdpConnect(llvm::StringRef host_and_port, + bool child_processes_inherit) { + return UDPSocket::Connect(host_and_port, child_processes_inherit); } Status Socket::UnixDomainConnect(llvm::StringRef name, diff --git a/lldb/source/Host/common/UDPSocket.cpp b/lldb/source/Host/common/UDPSocket.cpp index 75fb483087a9..0b537b3a9b13 100644 --- a/lldb/source/Host/common/UDPSocket.cpp +++ b/lldb/source/Host/common/UDPSocket.cpp @@ -53,19 +53,19 @@ Status UDPSocket::Accept(Socket *&socket) { return Status("%s", g_not_supported_error); } -Status UDPSocket::Connect(llvm::StringRef name, bool child_processes_inherit, - Socket *&socket) { - std::unique_ptr<UDPSocket> final_socket; +llvm::Expected<std::unique_ptr<UDPSocket>> +UDPSocket::Connect(llvm::StringRef name, bool child_processes_inherit) { + std::unique_ptr<UDPSocket> socket; Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION)); - LLDB_LOGF(log, "UDPSocket::%s (host/port = %s)", __FUNCTION__, name.data()); + LLDB_LOG(log, "host/port = {0}", name); Status error; std::string host_str; std::string port_str; int32_t port = INT32_MIN; if (!DecodeHostAndPort(name, host_str, port_str, port, &error)) - return error; + return error.ToError(); // At this point we have setup the receive port, now we need to setup the UDP // send socket @@ -86,7 +86,7 @@ Status UDPSocket::Connect(llvm::StringRef name, bool child_processes_inherit, "getaddrinfo(%s, %s, &hints, &info) returned error %i (%s)", #endif host_str.c_str(), port_str.c_str(), err, gai_strerror(err)); - return error; + return error.ToError(); } for (struct addrinfo *service_info_ptr = service_info_list; @@ -96,8 +96,8 @@ Status UDPSocket::Connect(llvm::StringRef name, bool child_processes_inherit, service_info_ptr->ai_family, service_info_ptr->ai_socktype, service_info_ptr->ai_protocol, child_processes_inherit, error); if (error.Success()) { - final_socket.reset(new UDPSocket(send_fd)); - final_socket->m_sockaddr = service_info_ptr; + socket.reset(new UDPSocket(send_fd)); + socket->m_sockaddr = service_info_ptr; break; } else continue; @@ -105,8 +105,8 @@ Status UDPSocket::Connect(llvm::StringRef name, bool child_processes_inherit, ::freeaddrinfo(service_info_list); - if (!final_socket) - return error; + if (!socket) + return error.ToError(); SocketAddress bind_addr; @@ -118,20 +118,19 @@ Status UDPSocket::Connect(llvm::StringRef name, bool child_processes_inherit, if (!bind_addr_success) { error.SetErrorString("Failed to get hostspec to bind for"); - return error; + return error.ToError(); } bind_addr.SetPort(0); // Let the source port # be determined dynamically - err = ::bind(final_socket->GetNativeSocket(), bind_addr, bind_addr.GetLength()); + err = ::bind(socket->GetNativeSocket(), bind_addr, bind_addr.GetLength()); struct sockaddr_in source_info; socklen_t address_len = sizeof (struct sockaddr_in); - err = ::getsockname(final_socket->GetNativeSocket(), (struct sockaddr *) &source_info, &address_len); + err = ::getsockname(socket->GetNativeSocket(), + (struct sockaddr *)&source_info, &address_len); - socket = final_socket.release(); - error.Clear(); - return error; + return std::move(socket); } std::string UDPSocket::GetRemoteConnectionURI() const { diff --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp index 011e1fb63e15..5683330a75c8 100644 --- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp +++ b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp @@ -42,6 +42,7 @@ #include "lldb/Host/Host.h" #include "lldb/Host/Socket.h" #include "lldb/Host/common/TCPSocket.h" +#include "lldb/Host/common/UDPSocket.h" #include "lldb/Utility/Log.h" #include "lldb/Utility/StreamString.h" #include "lldb/Utility/Timer.h" @@ -693,58 +694,71 @@ ConnectionFileDescriptor::UnixAbstractSocketConnect(llvm::StringRef socket_name, ConnectionStatus ConnectionFileDescriptor::SocketListenAndAccept(llvm::StringRef s, Status *error_ptr) { + if (error_ptr) + *error_ptr = Status(); m_port_predicate.SetValue(0, eBroadcastNever); - Socket *socket = nullptr; m_waiting_for_accept = true; - Status error = Socket::TcpListen(s, m_child_processes_inherit, socket, - &m_port_predicate); - if (error_ptr) - *error_ptr = error; - if (error.Fail()) + llvm::Expected<std::unique_ptr<TCPSocket>> listening_socket = + Socket::TcpListen(s, m_child_processes_inherit, &m_port_predicate); + if (!listening_socket) { + if (error_ptr) + *error_ptr = listening_socket.takeError(); + else + LLDB_LOG_ERROR(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION), + listening_socket.takeError(), "tcp listen failed: {0}"); return eConnectionStatusError; + } - std::unique_ptr<Socket> listening_socket_up; - listening_socket_up.reset(socket); - socket = nullptr; - error = listening_socket_up->Accept(socket); - listening_socket_up.reset(); + Socket *accepted_socket; + Status error = listening_socket.get()->Accept(accepted_socket); if (error_ptr) *error_ptr = error; if (error.Fail()) return eConnectionStatusError; - InitializeSocket(socket); + InitializeSocket(accepted_socket); return eConnectionStatusSuccess; } ConnectionStatus ConnectionFileDescriptor::ConnectTCP(llvm::StringRef s, Status *error_ptr) { - Socket *socket = nullptr; - Status error = Socket::TcpConnect(s, 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()) { + *error_ptr = Status(); + + llvm::Expected<std::unique_ptr<Socket>> socket = + Socket::TcpConnect(s, m_child_processes_inherit); + if (!socket) { + if (error_ptr) + *error_ptr = socket.takeError(); + else + LLDB_LOG_ERROR(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION), + socket.takeError(), "tcp connect failed: {0}"); return eConnectionStatusError; } + m_write_sp = std::move(*socket); + m_read_sp = m_write_sp; m_uri.assign(std::string(s)); return eConnectionStatusSuccess; } ConnectionStatus ConnectionFileDescriptor::ConnectUDP(llvm::StringRef s, Status *error_ptr) { - Socket *socket = nullptr; - Status error = Socket::UdpConnect(s, 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()) { + *error_ptr = Status(); + llvm::Expected<std::unique_ptr<UDPSocket>> socket = + Socket::UdpConnect(s, m_child_processes_inherit); + if (!socket) { + if (error_ptr) + *error_ptr = socket.takeError(); + else + LLDB_LOG_ERROR(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION), + socket.takeError(), "tcp connect failed: {0}"); return eConnectionStatusError; } + m_write_sp = std::move(*socket); + m_read_sp = m_write_sp; m_uri.assign(std::string(s)); return eConnectionStatusSuccess; } diff --git a/lldb/unittests/Host/SocketTest.cpp b/lldb/unittests/Host/SocketTest.cpp index a4fa78ff011a..54548d36956c 100644 --- a/lldb/unittests/Host/SocketTest.cpp +++ b/lldb/unittests/Host/SocketTest.cpp @@ -128,27 +128,21 @@ TEST_F(SocketTest, TCPGetAddress) { } TEST_F(SocketTest, UDPConnect) { - Socket *socket; + llvm::Expected<std::unique_ptr<UDPSocket>> socket = + UDPSocket::Connect("127.0.0.1:0", /*child_processes_inherit=*/false); - bool child_processes_inherit = false; - auto error = UDPSocket::Connect("127.0.0.1:0", child_processes_inherit, - socket); - - std::unique_ptr<Socket> socket_up(socket); - - EXPECT_TRUE(error.Success()); - EXPECT_TRUE(socket_up->IsValid()); + ASSERT_THAT_EXPECTED(socket, llvm::Succeeded()); + EXPECT_TRUE(socket.get()->IsValid()); } TEST_F(SocketTest, TCPListen0GetPort) { - Socket *server_socket; Predicate<uint16_t> port_predicate; port_predicate.SetValue(0, eBroadcastNever); - Status err = - Socket::TcpListen("10.10.12.3:0", false, server_socket, &port_predicate); - std::unique_ptr<TCPSocket> socket_up((TCPSocket*)server_socket); - EXPECT_TRUE(socket_up->IsValid()); - EXPECT_NE(socket_up->GetLocalPortNumber(), 0); + llvm::Expected<std::unique_ptr<TCPSocket>> sock = + Socket::TcpListen("10.10.12.3:0", false, &port_predicate); + ASSERT_THAT_EXPECTED(sock, llvm::Succeeded()); + ASSERT_TRUE(sock.get()->IsValid()); + EXPECT_NE(sock.get()->GetLocalPortNumber(), 0); } TEST_F(SocketTest, TCPGetConnectURI) { @@ -175,16 +169,15 @@ TEST_F(SocketTest, UDPGetConnectURI) { GTEST_LOG_(WARNING) << "Skipping test due to missing IPv4 support."; return; } - Socket *socket; - bool child_processes_inherit = false; - auto error = - UDPSocket::Connect("127.0.0.1:0", child_processes_inherit, socket); + llvm::Expected<std::unique_ptr<UDPSocket>> socket = + UDPSocket::Connect("127.0.0.1:0", /*child_processes_inherit=*/false); + ASSERT_THAT_EXPECTED(socket, llvm::Succeeded()); llvm::StringRef scheme; llvm::StringRef hostname; int port; llvm::StringRef path; - std::string uri(socket->GetRemoteConnectionURI()); + std::string uri = socket.get()->GetRemoteConnectionURI(); EXPECT_TRUE(UriParser::Parse(uri, scheme, hostname, port, path)); EXPECT_EQ(scheme, "udp"); } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
