[Lldb-commits] [lldb] 073c5d0 - [lldb] [Host/Socket] Make DecodeHostAndPort() return a dedicated struct
Michał Górny via lldb-commits
lldb-commits at lists.llvm.org
Thu Oct 28 00:58:30 PDT 2021
Author: Michał Górny
Date: 2021-10-28T09:57:50+02:00
New Revision: 073c5d0e4706f14b599f62c7bb115b843d0e4962
URL: https://github.com/llvm/llvm-project/commit/073c5d0e4706f14b599f62c7bb115b843d0e4962
DIFF: https://github.com/llvm/llvm-project/commit/073c5d0e4706f14b599f62c7bb115b843d0e4962.diff
LOG: [lldb] [Host/Socket] Make DecodeHostAndPort() return a dedicated struct
Differential Revision: https://reviews.llvm.org/D112629
Added:
Modified:
lldb/include/lldb/Host/Socket.h
lldb/source/Host/common/Socket.cpp
lldb/source/Host/common/TCPSocket.cpp
lldb/source/Host/common/UDPSocket.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/tools/lldb-server/Acceptor.cpp
lldb/unittests/Host/SocketTest.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Host/Socket.h b/lldb/include/lldb/Host/Socket.h
index 780c6e6adddd1..9176433df7cea 100644
--- a/lldb/include/lldb/Host/Socket.h
+++ b/lldb/include/lldb/Host/Socket.h
@@ -47,6 +47,15 @@ class Socket : public IOObject {
ProtocolUnixAbstract
};
+ struct HostAndPort {
+ std::string hostname;
+ uint16_t port;
+
+ bool operator==(const HostAndPort &R) const {
+ return port == R.port && hostname == R.hostname;
+ }
+ };
+
static const NativeSocket kInvalidSocketValue;
~Socket() override;
@@ -102,9 +111,8 @@ class Socket : public IOObject {
bool IsValid() const override { return m_socket != kInvalidSocketValue; }
WaitableHandle GetWaitableHandle() override;
- static llvm::Error DecodeHostAndPort(llvm::StringRef host_and_port,
- std::string &host_str,
- std::string &port_str, uint16_t &port);
+ static llvm::Expected<HostAndPort>
+ DecodeHostAndPort(llvm::StringRef host_and_port);
// If this Socket is connected then return the URI used to connect.
virtual std::string GetRemoteConnectionURI() const { return ""; };
@@ -129,6 +137,9 @@ class Socket : public IOObject {
bool m_should_close_fd;
};
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
+ const Socket::HostAndPort &HP);
+
} // namespace lldb_private
#endif // LLDB_HOST_SOCKET_H
diff --git a/lldb/source/Host/common/Socket.cpp b/lldb/source/Host/common/Socket.cpp
index 1eaa69532592a..b3fdb933bf739 100644
--- a/lldb/source/Host/common/Socket.cpp
+++ b/lldb/source/Host/common/Socket.cpp
@@ -167,13 +167,6 @@ Socket::TcpListen(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);
- std::string host_str;
- std::string port_str;
- uint16_t port;
- if (llvm::Error decode_error =
- DecodeHostAndPort(host_and_port, host_str, port_str, port))
- return std::move(decode_error);
-
std::unique_ptr<TCPSocket> listen_socket(
new TCPSocket(true, child_processes_inherit));
@@ -181,12 +174,6 @@ Socket::TcpListen(llvm::StringRef host_and_port, bool child_processes_inherit,
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();
-
return std::move(listen_socket);
}
@@ -260,28 +247,22 @@ Status Socket::UnixAbstractAccept(llvm::StringRef name,
return error;
}
-llvm::Error Socket::DecodeHostAndPort(llvm::StringRef host_and_port,
- std::string &host_str,
- std::string &port_str, uint16_t &port) {
+llvm::Expected<Socket::HostAndPort> Socket::DecodeHostAndPort(llvm::StringRef host_and_port) {
static llvm::Regex g_regex("([^:]+|\\[[0-9a-fA-F:]+.*\\]):([0-9]+)");
+ HostAndPort ret;
llvm::SmallVector<llvm::StringRef, 3> matches;
if (g_regex.match(host_and_port, &matches)) {
- host_str = matches[1].str();
- port_str = matches[2].str();
+ ret.hostname = matches[1].str();
// IPv6 addresses are wrapped in [] when specified with ports
- if (host_str.front() == '[' && host_str.back() == ']')
- host_str = host_str.substr(1, host_str.size() - 2);
- if (to_integer(matches[2], port, 10))
- return llvm::Error::success();
+ if (ret.hostname.front() == '[' && ret.hostname.back() == ']')
+ ret.hostname = ret.hostname.substr(1, ret.hostname.size() - 2);
+ if (to_integer(matches[2], ret.port, 10))
+ return ret;
} else {
- // If this was unsuccessful, then check if it's simply a signed 32-bit
+ // If this was unsuccessful, then check if it's simply an unsigned 16-bit
// integer, representing a port with an empty host.
- host_str.clear();
- port_str.clear();
- if (to_integer(host_and_port, port, 10)) {
- port_str = host_and_port.str();
- return llvm::Error::success();
- }
+ if (to_integer(host_and_port, ret.port, 10))
+ return ret;
}
return llvm::createStringError(llvm::inconvertibleErrorCode(),
@@ -455,3 +436,8 @@ NativeSocket Socket::AcceptSocket(NativeSocket sockfd, struct sockaddr *addr,
SetLastError(error);
return fd;
}
+
+llvm::raw_ostream &lldb_private::operator<<(llvm::raw_ostream &OS,
+ const Socket::HostAndPort &HP) {
+ return OS << '[' << HP.hostname << ']' << ':' << HP.port;
+}
diff --git a/lldb/source/Host/common/TCPSocket.cpp b/lldb/source/Host/common/TCPSocket.cpp
index 4e5bd69b1520e..28c3fa1188c2a 100644
--- a/lldb/source/Host/common/TCPSocket.cpp
+++ b/lldb/source/Host/common/TCPSocket.cpp
@@ -152,24 +152,23 @@ Status TCPSocket::Connect(llvm::StringRef name) {
LLDB_LOGF(log, "TCPSocket::%s (host/port = %s)", __FUNCTION__, name.data());
Status error;
- std::string host_str;
- std::string port_str;
- uint16_t port;
- if (llvm::Error decode_error =
- DecodeHostAndPort(name, host_str, port_str, port))
- return Status(std::move(decode_error));
+ llvm::Expected<HostAndPort> host_port = DecodeHostAndPort(name);
+ if (!host_port)
+ return Status(host_port.takeError());
- std::vector<SocketAddress> addresses = SocketAddress::GetAddressInfo(
- host_str.c_str(), nullptr, AF_UNSPEC, SOCK_STREAM, IPPROTO_TCP);
+ std::vector<SocketAddress> addresses =
+ SocketAddress::GetAddressInfo(host_port->hostname.c_str(), nullptr,
+ AF_UNSPEC, SOCK_STREAM, IPPROTO_TCP);
for (SocketAddress &address : addresses) {
error = CreateSocket(address.GetFamily());
if (error.Fail())
continue;
- address.SetPort(port);
+ address.SetPort(host_port->port);
- if (-1 == llvm::sys::RetryAfterSignal(-1, ::connect,
- GetNativeSocket(), &address.sockaddr(), address.GetLength())) {
+ if (-1 == llvm::sys::RetryAfterSignal(-1, ::connect, GetNativeSocket(),
+ &address.sockaddr(),
+ address.GetLength())) {
CLOSE_SOCKET(GetNativeSocket());
continue;
}
@@ -189,17 +188,14 @@ Status TCPSocket::Listen(llvm::StringRef name, int backlog) {
LLDB_LOGF(log, "TCPSocket::%s (%s)", __FUNCTION__, name.data());
Status error;
- std::string host_str;
- std::string port_str;
- uint16_t port;
- if (llvm::Error decode_error =
- DecodeHostAndPort(name, host_str, port_str, port))
- return Status(std::move(decode_error));
-
- if (host_str == "*")
- host_str = "0.0.0.0";
+ llvm::Expected<HostAndPort> host_port = DecodeHostAndPort(name);
+ if (!host_port)
+ return Status(host_port.takeError());
+
+ if (host_port->hostname == "*")
+ host_port->hostname = "0.0.0.0";
std::vector<SocketAddress> addresses = SocketAddress::GetAddressInfo(
- host_str.c_str(), nullptr, AF_UNSPEC, SOCK_STREAM, IPPROTO_TCP);
+ host_port->hostname.c_str(), nullptr, AF_UNSPEC, SOCK_STREAM, IPPROTO_TCP);
for (SocketAddress &address : addresses) {
int fd = Socket::CreateSocket(address.GetFamily(), kType, IPPROTO_TCP,
m_child_processes_inherit, error);
@@ -215,9 +211,9 @@ Status TCPSocket::Listen(llvm::StringRef name, int backlog) {
SocketAddress listen_address = address;
if(!listen_address.IsLocalhost())
- listen_address.SetToAnyAddress(address.GetFamily(), port);
+ listen_address.SetToAnyAddress(address.GetFamily(), host_port->port);
else
- listen_address.SetPort(port);
+ listen_address.SetPort(host_port->port);
int err =
::bind(fd, &listen_address.sockaddr(), listen_address.GetLength());
@@ -230,10 +226,10 @@ Status TCPSocket::Listen(llvm::StringRef name, int backlog) {
continue;
}
- if (port == 0) {
+ if (host_port->port == 0) {
socklen_t sa_len = address.GetLength();
if (getsockname(fd, &address.sockaddr(), &sa_len) == 0)
- port = address.GetPort();
+ host_port->port = address.GetPort();
}
m_listen_sockets[fd] = address;
}
diff --git a/lldb/source/Host/common/UDPSocket.cpp b/lldb/source/Host/common/UDPSocket.cpp
index ceab6f781a074..31266c980e0ea 100644
--- a/lldb/source/Host/common/UDPSocket.cpp
+++ b/lldb/source/Host/common/UDPSocket.cpp
@@ -58,12 +58,9 @@ UDPSocket::Connect(llvm::StringRef name, bool child_processes_inherit) {
LLDB_LOG(log, "host/port = {0}", name);
Status error;
- std::string host_str;
- std::string port_str;
- uint16_t port;
- if (llvm::Error decode_error =
- DecodeHostAndPort(name, host_str, port_str, port))
- return std::move(decode_error);
+ llvm::Expected<HostAndPort> host_port = DecodeHostAndPort(name);
+ if (!host_port)
+ return host_port.takeError();
// At this point we have setup the receive port, now we need to setup the UDP
// send socket
@@ -74,16 +71,16 @@ UDPSocket::Connect(llvm::StringRef name, bool child_processes_inherit) {
::memset(&hints, 0, sizeof(hints));
hints.ai_family = kDomain;
hints.ai_socktype = kType;
- int err = ::getaddrinfo(host_str.c_str(), port_str.c_str(), &hints,
+ int err = ::getaddrinfo(host_port->hostname.c_str(), std::to_string(host_port->port).c_str(), &hints,
&service_info_list);
if (err != 0) {
error.SetErrorStringWithFormat(
#if defined(_WIN32) && defined(UNICODE)
- "getaddrinfo(%s, %s, &hints, &info) returned error %i (%S)",
+ "getaddrinfo(%s, %d, &hints, &info) returned error %i (%S)",
#else
- "getaddrinfo(%s, %s, &hints, &info) returned error %i (%s)",
+ "getaddrinfo(%s, %d, &hints, &info) returned error %i (%s)",
#endif
- host_str.c_str(), port_str.c_str(), err, gai_strerror(err));
+ host_port->hostname.c_str(), host_port->port, err, gai_strerror(err));
return error.ToError();
}
@@ -110,9 +107,9 @@ UDPSocket::Connect(llvm::StringRef name, bool child_processes_inherit) {
// Only bind to the loopback address if we are expecting a connection from
// localhost to avoid any firewall issues.
- const bool bind_addr_success = (host_str == "127.0.0.1" || host_str == "localhost")
- ? bind_addr.SetToLocalhost(kDomain, port)
- : bind_addr.SetToAnyAddress(kDomain, port);
+ const bool bind_addr_success = (host_port->hostname == "127.0.0.1" || host_port->hostname == "localhost")
+ ? bind_addr.SetToLocalhost(kDomain, host_port->port)
+ : bind_addr.SetToAnyAddress(kDomain, host_port->port);
if (!bind_addr_success) {
error.SetErrorString("Failed to get hostspec to bind for");
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 27acf7e204fc0..da645319fda63 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -3917,12 +3917,8 @@ lldb_private::process_gdb_remote::LLGSArgToURL(llvm::StringRef url_arg,
if (url_arg.startswith(":"))
host_port.insert(0, "localhost");
- std::string host_str;
- std::string port_str;
- uint16_t port;
// Try parsing the (preprocessed) argument as host:port pair.
- if (!llvm::errorToBool(
- Socket::DecodeHostAndPort(host_port, host_str, port_str, port)))
+ if (!llvm::errorToBool(Socket::DecodeHostAndPort(host_port).takeError()))
return (reverse_connect ? "connect://" : "listen://") + host_port;
// If none of the above applied, interpret the argument as UNIX socket path.
diff --git a/lldb/tools/lldb-server/Acceptor.cpp b/lldb/tools/lldb-server/Acceptor.cpp
index a031a32e8ec33..4714252011aab 100644
--- a/lldb/tools/lldb-server/Acceptor.cpp
+++ b/lldb/tools/lldb-server/Acceptor.cpp
@@ -92,12 +92,8 @@ std::unique_ptr<Acceptor> Acceptor::Create(StringRef name,
else
name = name.drop_front(res->scheme.size() + strlen("://"));
} else {
- std::string host_str;
- std::string port_str;
- uint16_t port;
// Try to match socket name as $host:port - e.g., localhost:5555
- if (!llvm::errorToBool(
- Socket::DecodeHostAndPort(name, host_str, port_str, port)))
+ if (!llvm::errorToBool(Socket::DecodeHostAndPort(name).takeError()))
socket_protocol = Socket::ProtocolTcp;
}
diff --git a/lldb/unittests/Host/SocketTest.cpp b/lldb/unittests/Host/SocketTest.cpp
index 15ccf647a2e09..6af0b3c92b3b9 100644
--- a/lldb/unittests/Host/SocketTest.cpp
+++ b/lldb/unittests/Host/SocketTest.cpp
@@ -33,65 +33,40 @@ class SocketTest : public testing::TestWithParam<SocketTestParams> {
};
TEST_P(SocketTest, DecodeHostAndPort) {
- std::string host_str;
- std::string port_str;
- uint16_t port;
-
- EXPECT_THAT_ERROR(
- Socket::DecodeHostAndPort("localhost:1138", host_str, port_str, port),
- llvm::Succeeded());
- EXPECT_STREQ("localhost", host_str.c_str());
- EXPECT_STREQ("1138", port_str.c_str());
- EXPECT_EQ(1138, port);
-
- EXPECT_THAT_ERROR(
- Socket::DecodeHostAndPort("google.com:65536", host_str, port_str, port),
+ EXPECT_THAT_EXPECTED(Socket::DecodeHostAndPort("localhost:1138"),
+ llvm::HasValue(Socket::HostAndPort{"localhost", 1138}));
+
+ EXPECT_THAT_EXPECTED(
+ Socket::DecodeHostAndPort("google.com:65536"),
llvm::FailedWithMessage(
"invalid host:port specification: 'google.com:65536'"));
- EXPECT_THAT_ERROR(
- Socket::DecodeHostAndPort("google.com:-1138", host_str, port_str, port),
+ EXPECT_THAT_EXPECTED(
+ Socket::DecodeHostAndPort("google.com:-1138"),
llvm::FailedWithMessage(
"invalid host:port specification: 'google.com:-1138'"));
- EXPECT_THAT_ERROR(
- Socket::DecodeHostAndPort("google.com:65536", host_str, port_str, port),
+ EXPECT_THAT_EXPECTED(
+ Socket::DecodeHostAndPort("google.com:65536"),
llvm::FailedWithMessage(
"invalid host:port specification: 'google.com:65536'"));
- EXPECT_THAT_ERROR(
- Socket::DecodeHostAndPort("12345", host_str, port_str, port),
- llvm::Succeeded());
- EXPECT_STREQ("", host_str.c_str());
- EXPECT_STREQ("12345", port_str.c_str());
- EXPECT_EQ(12345, port);
-
- EXPECT_THAT_ERROR(Socket::DecodeHostAndPort("*:0", host_str, port_str, port),
- llvm::Succeeded());
- EXPECT_STREQ("*", host_str.c_str());
- EXPECT_STREQ("0", port_str.c_str());
- EXPECT_EQ(0, port);
-
- EXPECT_THAT_ERROR(
- Socket::DecodeHostAndPort("*:65535", host_str, port_str, port),
- llvm::Succeeded());
- EXPECT_STREQ("*", host_str.c_str());
- EXPECT_STREQ("65535", port_str.c_str());
- EXPECT_EQ(65535, port);
-
- EXPECT_THAT_ERROR(
- Socket::DecodeHostAndPort("[::1]:12345", host_str, port_str, port),
- llvm::Succeeded());
- EXPECT_STREQ("::1", host_str.c_str());
- EXPECT_STREQ("12345", port_str.c_str());
- EXPECT_EQ(12345, port);
-
- EXPECT_THAT_ERROR(Socket::DecodeHostAndPort("[abcd:12fg:AF58::1]:12345",
- host_str, port_str, port),
- llvm::Succeeded());
- EXPECT_STREQ("abcd:12fg:AF58::1", host_str.c_str());
- EXPECT_STREQ("12345", port_str.c_str());
- EXPECT_EQ(12345, port);
+ EXPECT_THAT_EXPECTED(Socket::DecodeHostAndPort("12345"),
+ llvm::HasValue(Socket::HostAndPort{"", 12345}));
+
+ EXPECT_THAT_EXPECTED(Socket::DecodeHostAndPort("*:0"),
+ llvm::HasValue(Socket::HostAndPort{"*", 0}));
+
+ EXPECT_THAT_EXPECTED(Socket::DecodeHostAndPort("*:65535"),
+ llvm::HasValue(Socket::HostAndPort{"*", 65535}));
+
+ EXPECT_THAT_EXPECTED(
+ Socket::DecodeHostAndPort("[::1]:12345"),
+ llvm::HasValue(Socket::HostAndPort{"::1", 12345}));
+
+ EXPECT_THAT_EXPECTED(
+ Socket::DecodeHostAndPort("[abcd:12fg:AF58::1]:12345"),
+ llvm::HasValue(Socket::HostAndPort{"abcd:12fg:AF58::1", 12345}));
}
#if LLDB_ENABLE_POSIX
More information about the lldb-commits
mailing list