[Lldb-commits] [lldb] c9e6b70 - [lldb/Host] Modernize some socket functions
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Thu Apr 23 05:20:39 PDT 2020
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");
}
More information about the lldb-commits
mailing list