[Lldb-commits] [lldb] 8dbbe33 - Revert "[lldb] [Host/ConnectionFileDescriptor] Refactor to improve code reuse"

Med Ismail Bennani via lldb-commits lldb-commits at lists.llvm.org
Wed Oct 27 14:57:43 PDT 2021


Author: Med Ismail Bennani
Date: 2021-10-27T23:57:33+02:00
New Revision: 8dbbe3356b0bda56c7d21e5a3b07d7bfe601bc22

URL: https://github.com/llvm/llvm-project/commit/8dbbe3356b0bda56c7d21e5a3b07d7bfe601bc22
DIFF: https://github.com/llvm/llvm-project/commit/8dbbe3356b0bda56c7d21e5a3b07d7bfe601bc22.diff

LOG: Revert "[lldb] [Host/ConnectionFileDescriptor] Refactor to improve code reuse"

This reverts commit e1acadb61dfc0810656219c6314019d5132f2c61.

Added: 
    

Modified: 
    lldb/include/lldb/Host/Socket.h
    lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
    lldb/source/Host/common/Socket.cpp
    lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Host/Socket.h b/lldb/include/lldb/Host/Socket.h
index 306763025d4bc..780c6e6adddd1 100644
--- a/lldb/include/lldb/Host/Socket.h
+++ b/lldb/include/lldb/Host/Socket.h
@@ -75,6 +75,18 @@ class Socket : public IOObject {
   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);
+  static Status UnixDomainAccept(llvm::StringRef host_and_port,
+                                 bool child_processes_inherit, Socket *&socket);
+  static Status UnixAbstractConnect(llvm::StringRef host_and_port,
+                                    bool child_processes_inherit,
+                                    Socket *&socket);
+  static Status UnixAbstractAccept(llvm::StringRef host_and_port,
+                                   bool child_processes_inherit,
+                                   Socket *&socket);
+
   int GetOption(int level, int option_name, int &option_value);
   int SetOption(int level, int option_name, int option_value);
 

diff  --git a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
index 35773d5907e91..12eab5fc8ae83 100644
--- a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
+++ b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
@@ -16,7 +16,6 @@
 #include "lldb/lldb-forward.h"
 
 #include "lldb/Host/Pipe.h"
-#include "lldb/Host/Socket.h"
 #include "lldb/Utility/Connection.h"
 #include "lldb/Utility/IOObject.h"
 
@@ -74,18 +73,9 @@ class ConnectionFileDescriptor : public Connection {
   void CloseCommandPipe();
 
   lldb::ConnectionStatus
-  AcceptSocket(Socket::SocketProtocol socket_protocol,
-               llvm::StringRef socket_name,
-               llvm::function_ref<void(Socket &)> post_listen_callback,
-               Status *error_ptr);
-
-  lldb::ConnectionStatus ConnectSocket(Socket::SocketProtocol socket_protocol,
-                                       llvm::StringRef socket_name,
-                                       Status *error_ptr);
-
-  lldb::ConnectionStatus AcceptTCP(llvm::StringRef host_and_port,
-                                   socket_id_callback_type socket_id_callback,
-                                   Status *error_ptr);
+  SocketListenAndAccept(llvm::StringRef host_and_port,
+                        socket_id_callback_type socket_id_callback,
+                        Status *error_ptr);
 
   lldb::ConnectionStatus ConnectTCP(llvm::StringRef host_and_port,
                                     socket_id_callback_type socket_id_callback,
@@ -96,24 +86,24 @@ class ConnectionFileDescriptor : public Connection {
                                     Status *error_ptr);
 
   lldb::ConnectionStatus
-  ConnectNamedSocket(llvm::StringRef socket_name,
+  NamedSocketConnect(llvm::StringRef socket_name,
                      socket_id_callback_type socket_id_callback,
                      Status *error_ptr);
 
   lldb::ConnectionStatus
-  AcceptNamedSocket(llvm::StringRef socket_name,
+  NamedSocketAccept(llvm::StringRef socket_name,
                     socket_id_callback_type socket_id_callback,
                     Status *error_ptr);
 
   lldb::ConnectionStatus
-  AcceptAbstractSocket(llvm::StringRef socket_name,
-                       socket_id_callback_type socket_id_callback,
-                       Status *error_ptr);
+  UnixAbstractSocketAccept(llvm::StringRef socket_name,
+                           socket_id_callback_type socket_id_callback,
+                           Status *error_ptr);
 
   lldb::ConnectionStatus
-  ConnectAbstractSocket(llvm::StringRef socket_name,
-                        socket_id_callback_type socket_id_callback,
-                        Status *error_ptr);
+  UnixAbstractSocketConnect(llvm::StringRef socket_name,
+                            socket_id_callback_type socket_id_callback,
+                            Status *error_ptr);
 
   lldb::ConnectionStatus ConnectFD(llvm::StringRef args,
                                    socket_id_callback_type socket_id_callback,

diff  --git a/lldb/source/Host/common/Socket.cpp b/lldb/source/Host/common/Socket.cpp
index 0894eee2522ab..1eaa69532592a 100644
--- a/lldb/source/Host/common/Socket.cpp
+++ b/lldb/source/Host/common/Socket.cpp
@@ -167,6 +167,13 @@ 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));
 
@@ -174,6 +181,12 @@ 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);
 }
 
@@ -183,6 +196,70 @@ Socket::UdpConnect(llvm::StringRef host_and_port,
   return UDPSocket::Connect(host_and_port, child_processes_inherit);
 }
 
+Status Socket::UnixDomainConnect(llvm::StringRef name,
+                                 bool child_processes_inherit,
+                                 Socket *&socket) {
+  Status error;
+  std::unique_ptr<Socket> connect_socket(
+      Create(ProtocolUnixDomain, child_processes_inherit, error));
+  if (error.Fail())
+    return error;
+
+  error = connect_socket->Connect(name);
+  if (error.Success())
+    socket = connect_socket.release();
+
+  return error;
+}
+
+Status Socket::UnixDomainAccept(llvm::StringRef name,
+                                bool child_processes_inherit, Socket *&socket) {
+  Status error;
+  std::unique_ptr<Socket> listen_socket(
+      Create(ProtocolUnixDomain, child_processes_inherit, error));
+  if (error.Fail())
+    return error;
+
+  error = listen_socket->Listen(name, 5);
+  if (error.Fail())
+    return error;
+
+  error = listen_socket->Accept(socket);
+  return error;
+}
+
+Status Socket::UnixAbstractConnect(llvm::StringRef name,
+                                   bool child_processes_inherit,
+                                   Socket *&socket) {
+  Status error;
+  std::unique_ptr<Socket> connect_socket(
+      Create(ProtocolUnixAbstract, child_processes_inherit, error));
+  if (error.Fail())
+    return error;
+
+  error = connect_socket->Connect(name);
+  if (error.Success())
+    socket = connect_socket.release();
+  return error;
+}
+
+Status Socket::UnixAbstractAccept(llvm::StringRef name,
+                                  bool child_processes_inherit,
+                                  Socket *&socket) {
+  Status error;
+  std::unique_ptr<Socket> listen_socket(
+      Create(ProtocolUnixAbstract, child_processes_inherit, error));
+  if (error.Fail())
+    return error;
+
+  error = listen_socket->Listen(name, 5);
+  if (error.Fail())
+    return error;
+
+  error = listen_socket->Accept(socket);
+  return error;
+}
+
 llvm::Error Socket::DecodeHostAndPort(llvm::StringRef host_and_port,
                                       std::string &host_str,
                                       std::string &port_str, uint16_t &port) {

diff  --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
index 0fc52071bfb6a..1c3de966916ad 100644
--- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -151,17 +151,17 @@ ConnectionFileDescriptor::Connect(llvm::StringRef path,
     auto method =
         llvm::StringSwitch<ConnectionStatus (ConnectionFileDescriptor::*)(
             llvm::StringRef, socket_id_callback_type, Status *)>(scheme)
-            .Case("listen", &ConnectionFileDescriptor::AcceptTCP)
+            .Case("listen", &ConnectionFileDescriptor::SocketListenAndAccept)
             .Cases("accept", "unix-accept",
-                   &ConnectionFileDescriptor::AcceptNamedSocket)
+                   &ConnectionFileDescriptor::NamedSocketAccept)
             .Case("unix-abstract-accept",
-                  &ConnectionFileDescriptor::AcceptAbstractSocket)
+                  &ConnectionFileDescriptor::UnixAbstractSocketAccept)
             .Cases("connect", "tcp-connect",
                    &ConnectionFileDescriptor::ConnectTCP)
             .Case("udp", &ConnectionFileDescriptor::ConnectUDP)
-            .Case("unix-connect", &ConnectionFileDescriptor::ConnectNamedSocket)
+            .Case("unix-connect", &ConnectionFileDescriptor::NamedSocketConnect)
             .Case("unix-abstract-connect",
-                  &ConnectionFileDescriptor::ConnectAbstractSocket)
+                  &ConnectionFileDescriptor::UnixAbstractSocketConnect)
 #if LLDB_ENABLE_POSIX
             .Case("fd", &ConnectionFileDescriptor::ConnectFD)
             .Case("file", &ConnectionFileDescriptor::ConnectFile)
@@ -532,25 +532,24 @@ ConnectionFileDescriptor::BytesAvailable(const Timeout<std::micro> &timeout,
   return eConnectionStatusLostConnection;
 }
 
-lldb::ConnectionStatus ConnectionFileDescriptor::AcceptSocket(
-    Socket::SocketProtocol socket_protocol, llvm::StringRef socket_name,
-    llvm::function_ref<void(Socket &)> post_listen_callback,
+ConnectionStatus ConnectionFileDescriptor::NamedSocketAccept(
+    llvm::StringRef socket_name, socket_id_callback_type socket_id_callback,
     Status *error_ptr) {
   Status error;
-  std::unique_ptr<Socket> listening_socket =
-      Socket::Create(socket_protocol, m_child_processes_inherit, error);
-  Socket *accepted_socket;
+  std::unique_ptr<Socket> listen_socket = Socket::Create(
+      Socket::ProtocolUnixDomain, m_child_processes_inherit, error);
+  Socket *socket = nullptr;
 
   if (!error.Fail())
-    error = listening_socket->Listen(socket_name, 5);
+    error = listen_socket->Listen(socket_name, 5);
 
   if (!error.Fail()) {
-    post_listen_callback(*listening_socket);
-    error = listening_socket->Accept(accepted_socket);
+    socket_id_callback(socket_name);
+    error = listen_socket->Accept(socket);
   }
 
   if (!error.Fail()) {
-    m_io_sp.reset(accepted_socket);
+    m_io_sp.reset(socket);
     m_uri.assign(socket_name.str());
     return eConnectionStatusSuccess;
   }
@@ -560,19 +559,40 @@ lldb::ConnectionStatus ConnectionFileDescriptor::AcceptSocket(
   return eConnectionStatusError;
 }
 
-lldb::ConnectionStatus
-ConnectionFileDescriptor::ConnectSocket(Socket::SocketProtocol socket_protocol,
-                                        llvm::StringRef socket_name,
-                                        Status *error_ptr) {
+ConnectionStatus ConnectionFileDescriptor::NamedSocketConnect(
+    llvm::StringRef socket_name, socket_id_callback_type socket_id_callback,
+    Status *error_ptr) {
+  Socket *socket = nullptr;
+  Status error =
+      Socket::UnixDomainConnect(socket_name, m_child_processes_inherit, socket);
+  if (error_ptr)
+    *error_ptr = error;
+  m_io_sp.reset(socket);
+  if (error.Fail())
+    return eConnectionStatusError;
+  m_uri.assign(std::string(socket_name));
+  return eConnectionStatusSuccess;
+}
+
+ConnectionStatus ConnectionFileDescriptor::UnixAbstractSocketAccept(
+    llvm::StringRef socket_name, socket_id_callback_type socket_id_callback,
+    Status *error_ptr) {
   Status error;
-  std::unique_ptr<Socket> socket =
-      Socket::Create(socket_protocol, m_child_processes_inherit, error);
+  std::unique_ptr<Socket> listen_socket = Socket::Create(
+      Socket::ProtocolUnixAbstract, m_child_processes_inherit, error);
+  Socket *socket = nullptr;
+
+  if (!error.Fail())
+    error = listen_socket->Listen(socket_name, 5);
 
   if (!error.Fail())
-    error = socket->Connect(socket_name);
+    socket_id_callback(socket_name);
+
+  if (!error.Fail())
+    error = listen_socket->Accept(socket);
 
   if (!error.Fail()) {
-    m_io_sp = std::move(socket);
+    m_io_sp.reset(socket);
     m_uri.assign(socket_name.str());
     return eConnectionStatusSuccess;
   }
@@ -582,63 +602,72 @@ ConnectionFileDescriptor::ConnectSocket(Socket::SocketProtocol socket_protocol,
   return eConnectionStatusError;
 }
 
-ConnectionStatus ConnectionFileDescriptor::AcceptNamedSocket(
+lldb::ConnectionStatus ConnectionFileDescriptor::UnixAbstractSocketConnect(
     llvm::StringRef socket_name, socket_id_callback_type socket_id_callback,
     Status *error_ptr) {
-  return AcceptSocket(
-      Socket::ProtocolUnixDomain, socket_name,
-      [socket_id_callback, socket_name](Socket &listening_socket) {
-        socket_id_callback(socket_name);
-      },
-      error_ptr);
+  Socket *socket = nullptr;
+  Status error = Socket::UnixAbstractConnect(socket_name,
+                                             m_child_processes_inherit, socket);
+  if (error_ptr)
+    *error_ptr = error;
+  m_io_sp.reset(socket);
+  if (error.Fail())
+    return eConnectionStatusError;
+  m_uri.assign(std::string(socket_name));
+  return eConnectionStatusSuccess;
 }
 
-ConnectionStatus ConnectionFileDescriptor::ConnectNamedSocket(
-    llvm::StringRef socket_name, socket_id_callback_type socket_id_callback,
+ConnectionStatus ConnectionFileDescriptor::SocketListenAndAccept(
+    llvm::StringRef s, socket_id_callback_type socket_id_callback,
     Status *error_ptr) {
-  return ConnectSocket(Socket::ProtocolUnixDomain, socket_name, error_ptr);
-}
+  if (error_ptr)
+    *error_ptr = Status();
 
-ConnectionStatus ConnectionFileDescriptor::AcceptAbstractSocket(
-    llvm::StringRef socket_name, socket_id_callback_type socket_id_callback,
-    Status *error_ptr) {
-  return AcceptSocket(
-      Socket::ProtocolUnixAbstract, socket_name,
-      [socket_id_callback, socket_name](Socket &listening_socket) {
-        socket_id_callback(socket_name);
-      },
-      error_ptr);
-}
+  llvm::Expected<std::unique_ptr<TCPSocket>> listening_socket =
+      Socket::TcpListen(s, m_child_processes_inherit);
+  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;
+  }
 
-lldb::ConnectionStatus ConnectionFileDescriptor::ConnectAbstractSocket(
-    llvm::StringRef socket_name, socket_id_callback_type socket_id_callback,
-    Status *error_ptr) {
-  return ConnectSocket(Socket::ProtocolUnixAbstract, socket_name, error_ptr);
-}
+  uint16_t port = listening_socket.get()->GetLocalPortNumber();
+  socket_id_callback(std::to_string(port));
 
-ConnectionStatus
-ConnectionFileDescriptor::AcceptTCP(llvm::StringRef socket_name,
-                                    socket_id_callback_type socket_id_callback,
-                                    Status *error_ptr) {
-  ConnectionStatus ret = AcceptSocket(
-      Socket::ProtocolTcp, socket_name,
-      [socket_id_callback](Socket &listening_socket) {
-        uint16_t port =
-            static_cast<TCPSocket &>(listening_socket).GetLocalPortNumber();
-        socket_id_callback(std::to_string(port));
-      },
-      error_ptr);
-  if (ret == eConnectionStatusSuccess)
-    m_uri.assign(
-        static_cast<TCPSocket *>(m_io_sp.get())->GetRemoteConnectionURI());
-  return ret;
+  Socket *accepted_socket;
+  Status error = listening_socket.get()->Accept(accepted_socket);
+  if (error_ptr)
+    *error_ptr = error;
+  if (error.Fail())
+    return eConnectionStatusError;
+
+  InitializeSocket(accepted_socket);
+  return eConnectionStatusSuccess;
 }
 
 ConnectionStatus
-ConnectionFileDescriptor::ConnectTCP(llvm::StringRef socket_name,
+ConnectionFileDescriptor::ConnectTCP(llvm::StringRef s,
                                      socket_id_callback_type socket_id_callback,
                                      Status *error_ptr) {
-  return ConnectSocket(Socket::ProtocolTcp, socket_name, error_ptr);
+  if (error_ptr)
+    *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_io_sp = std::move(*socket);
+  m_uri.assign(std::string(s));
+  return eConnectionStatusSuccess;
 }
 
 ConnectionStatus


        


More information about the lldb-commits mailing list