Revert "[lldb] [Host] Refactor Socket::DecodeHostAndPort() to use LLVM API"
authorMichał Górny <mgorny@moritz.systems>
Fri, 24 Sep 2021 11:33:51 +0000 (13:33 +0200)
committerMichał Górny <mgorny@moritz.systems>
Fri, 24 Sep 2021 11:33:51 +0000 (13:33 +0200)
This reverts commit a6daf99228bc16fb7f2596d67a0d00fef327ace5.  It causes
buildbot regressions, I'll investigate.

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/tools/lldb-server/Acceptor.cpp
lldb/unittests/Host/SocketTest.cpp

index 12ddc11..36db0ec 100644 (file)
@@ -103,9 +103,9 @@ public:
   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 bool DecodeHostAndPort(llvm::StringRef host_and_port,
+                                std::string &host_str, std::string &port_str,
+                                int32_t &port, Status *error_ptr);
 
   // If this Socket is connected then return the URI used to connect.
   virtual std::string GetRemoteConnectionURI() const { return ""; };
index eb2495b..d1c327d 100644 (file)
 #include "lldb/Host/Config.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/SocketAddress.h"
+#include "lldb/Host/StringConvert.h"
 #include "lldb/Host/common/TCPSocket.h"
 #include "lldb/Host/common/UDPSocket.h"
 #include "lldb/Utility/Log.h"
+#include "lldb/Utility/RegularExpression.h"
 
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Errno.h"
@@ -170,17 +172,17 @@ 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);
 
+  Status error;
   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 decode_error;
+  int32_t port = INT32_MIN;
+  if (!DecodeHostAndPort(host_and_port, host_str, port_str, port, &error))
+    return error.ToError();
 
   std::unique_ptr<TCPSocket> listen_socket(
       new TCPSocket(true, child_processes_inherit));
 
-  Status error = listen_socket->Listen(host_and_port, backlog);
+  error = listen_socket->Listen(host_and_port, backlog);
   if (error.Fail())
     return error.ToError();
 
@@ -270,33 +272,47 @@ 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) {
-  static llvm::Regex g_regex("([^:]+|\\[[0-9a-fA-F:]+.*\\]):([0-9]+)");
+bool Socket::DecodeHostAndPort(llvm::StringRef host_and_port,
+                               std::string &host_str, std::string &port_str,
+                               int32_t &port, Status *error_ptr) {
+  static RegularExpression g_regex(
+      llvm::StringRef("([^:]+|\\[[0-9a-fA-F:]+.*\\]):([0-9]+)"));
   llvm::SmallVector<llvm::StringRef, 3> matches;
-  if (g_regex.match(host_and_port, &matches)) {
+  if (g_regex.Execute(host_and_port, &matches)) {
     host_str = matches[1].str();
     port_str = matches[2].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();
-  } else {
-    // If this was unsuccessful, then check if it's simply a signed 32-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();
+    bool ok = false;
+    port = StringConvert::ToUInt32(port_str.c_str(), UINT32_MAX, 10, &ok);
+    if (ok && port <= UINT16_MAX) {
+      if (error_ptr)
+        error_ptr->Clear();
+      return true;
     }
+    // port is too large
+    if (error_ptr)
+      error_ptr->SetErrorStringWithFormat(
+          "invalid host:port specification: '%s'", host_and_port.str().c_str());
+    return false;
+  }
+
+  // If this was unsuccessful, then check if it's simply a signed 32-bit
+  // integer, representing a port with an empty host.
+  host_str.clear();
+  port_str.clear();
+  if (to_integer(host_and_port, port, 10) && port < UINT16_MAX) {
+    port_str = std::string(host_and_port);
+    if (error_ptr)
+      error_ptr->Clear();
+    return true;
   }
 
-  return llvm::createStringError(llvm::inconvertibleErrorCode(),
-                                 "invalid host:port specification: '%s'",
-                                 host_and_port.str().c_str());
+  if (error_ptr)
+    error_ptr->SetErrorStringWithFormat("invalid host:port specification: '%s'",
+                                        host_and_port.str().c_str());
+  return false;
 }
 
 IOObject::WaitableHandle Socket::GetWaitableHandle() {
index d705171..ea7377e 100644 (file)
@@ -156,10 +156,9 @@ Status TCPSocket::Connect(llvm::StringRef 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 Status(std::move(decode_error));
+  int32_t port = INT32_MIN;
+  if (!DecodeHostAndPort(name, host_str, port_str, port, &error))
+    return error;
 
   std::vector<SocketAddress> addresses = SocketAddress::GetAddressInfo(
       host_str.c_str(), nullptr, AF_UNSPEC, SOCK_STREAM, IPPROTO_TCP);
@@ -193,10 +192,9 @@ Status TCPSocket::Listen(llvm::StringRef name, int backlog) {
   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));
+  int32_t port = INT32_MIN;
+  if (!DecodeHostAndPort(name, host_str, port_str, port, &error))
+    return error;
 
   if (host_str == "*")
     host_str = "0.0.0.0";
index de4f4f2..0b537b3 100644 (file)
@@ -63,10 +63,9 @@ UDPSocket::Connect(llvm::StringRef name, bool child_processes_inherit) {
   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 decode_error;
+  int32_t port = INT32_MIN;
+  if (!DecodeHostAndPort(name, host_str, port_str, port, &error))
+    return error.ToError();
 
   // At this point we have setup the receive port, now we need to setup the UDP
   // send socket
index 16ecc54..b8be9c5 100644 (file)
@@ -96,10 +96,9 @@ std::unique_ptr<Acceptor> Acceptor::Create(StringRef name,
   } else {
     std::string host_str;
     std::string port_str;
-    uint16_t port;
+    int32_t port = INT32_MIN;
     // Try to match socket name as $host:port - e.g., localhost:5555
-    if (!llvm::errorToBool(
-            Socket::DecodeHostAndPort(name, host_str, port_str, port)))
+    if (Socket::DecodeHostAndPort(name, host_str, port_str, port, nullptr))
       socket_protocol = Socket::ProtocolTcp;
   }
 
index 6b9fb42..5593b77 100644 (file)
@@ -10,7 +10,6 @@
 #include "TestingSupport/SubsystemRAII.h"
 #include "lldb/Host/Config.h"
 #include "lldb/Utility/UriParser.h"
-#include "llvm/Testing/Support/Error.h"
 #include "gtest/gtest.h"
 
 using namespace lldb_private;
@@ -35,63 +34,67 @@ protected:
 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());
+  int32_t port;
+  Status error;
+  EXPECT_TRUE(Socket::DecodeHostAndPort("localhost:1138", host_str, port_str,
+                                        port, &error));
   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),
-      llvm::FailedWithMessage(
-          "invalid host:port specification: 'google.com:65536'"));
-
-  EXPECT_THAT_ERROR(
-      Socket::DecodeHostAndPort("google.com:-1138", host_str, port_str, port),
-      llvm::FailedWithMessage(
-          "invalid host:port specification: 'google.com:-1138'"));
-
-  EXPECT_THAT_ERROR(
-      Socket::DecodeHostAndPort("google.com:65536", host_str, port_str, port),
-      llvm::FailedWithMessage(
-          "invalid host:port specification: 'google.com:65536'"));
-
-  EXPECT_THAT_ERROR(
-      Socket::DecodeHostAndPort("12345", host_str, port_str, port),
-      llvm::Succeeded());
+  EXPECT_TRUE(error.Success());
+
+  EXPECT_FALSE(Socket::DecodeHostAndPort("google.com:65536", host_str, port_str,
+                                         port, &error));
+  EXPECT_TRUE(error.Fail());
+  EXPECT_STREQ("invalid host:port specification: 'google.com:65536'",
+               error.AsCString());
+
+  EXPECT_FALSE(Socket::DecodeHostAndPort("google.com:-1138", host_str, port_str,
+                                         port, &error));
+  EXPECT_TRUE(error.Fail());
+  EXPECT_STREQ("invalid host:port specification: 'google.com:-1138'",
+               error.AsCString());
+
+  EXPECT_FALSE(Socket::DecodeHostAndPort("google.com:65536", host_str, port_str,
+                                         port, &error));
+  EXPECT_TRUE(error.Fail());
+  EXPECT_STREQ("invalid host:port specification: 'google.com:65536'",
+               error.AsCString());
+
+  EXPECT_TRUE(
+      Socket::DecodeHostAndPort("12345", host_str, port_str, port, &error));
   EXPECT_STREQ("", host_str.c_str());
   EXPECT_STREQ("12345", port_str.c_str());
   EXPECT_EQ(12345, port);
+  EXPECT_TRUE(error.Success());
 
-  EXPECT_THAT_ERROR(Socket::DecodeHostAndPort("*:0", host_str, port_str, port),
-                    llvm::Succeeded());
+  EXPECT_TRUE(
+      Socket::DecodeHostAndPort("*:0", host_str, port_str, port, &error));
   EXPECT_STREQ("*", host_str.c_str());
   EXPECT_STREQ("0", port_str.c_str());
   EXPECT_EQ(0, port);
+  EXPECT_TRUE(error.Success());
 
-  EXPECT_THAT_ERROR(
-      Socket::DecodeHostAndPort("*:65535", host_str, port_str, port),
-      llvm::Succeeded());
+  EXPECT_TRUE(
+      Socket::DecodeHostAndPort("*:65535", host_str, port_str, port, &error));
   EXPECT_STREQ("*", host_str.c_str());
   EXPECT_STREQ("65535", port_str.c_str());
   EXPECT_EQ(65535, port);
+  EXPECT_TRUE(error.Success());
 
-  EXPECT_THAT_ERROR(
-      Socket::DecodeHostAndPort("[::1]:12345", host_str, port_str, port),
-      llvm::Succeeded());
+  EXPECT_TRUE(
+      Socket::DecodeHostAndPort("[::1]:12345", host_str, port_str, port, &error));
   EXPECT_STREQ("::1", host_str.c_str());
   EXPECT_STREQ("12345", port_str.c_str());
   EXPECT_EQ(12345, port);
+  EXPECT_TRUE(error.Success());
 
-  EXPECT_THAT_ERROR(Socket::DecodeHostAndPort("[abcd:12fg:AF58::1]:12345",
-                                              host_str, port_str, port),
-                    llvm::Succeeded());
+  EXPECT_TRUE(
+      Socket::DecodeHostAndPort("[abcd:12fg:AF58::1]:12345", host_str, port_str, port, &error));
   EXPECT_STREQ("abcd:12fg:AF58::1", host_str.c_str());
   EXPECT_STREQ("12345", port_str.c_str());
   EXPECT_EQ(12345, port);
+  EXPECT_TRUE(error.Success());
 }
 
 #if LLDB_ENABLE_POSIX