mgorny created this revision.
mgorny added reviewers: labath, krytarowski, emaste, teemperor.
mgorny requested review of this revision.

Refactor ConnectToRemote() to improve readability and make future
changes easier:

1. Replace static buffers with std::string.
2. When handling errors, prefer reporting the actual error over dumb 
'connection status is not success'.
3. Move host/port parsing directly into reverse_connection condition that is 
its only user, and simplify it to make its purpose (verifying that a valid port 
is provided) clear.


https://reviews.llvm.org/D111963

Files:
  lldb/tools/lldb-server/lldb-gdbserver.cpp

Index: lldb/tools/lldb-server/lldb-gdbserver.cpp
===================================================================
--- lldb/tools/lldb-server/lldb-gdbserver.cpp
+++ lldb/tools/lldb-server/lldb-gdbserver.cpp
@@ -199,8 +199,7 @@
   std::unique_ptr<Connection> connection_up;
   if (connection_fd != -1) {
     // Build the connection string.
-    char connection_url[512];
-    snprintf(connection_url, sizeof(connection_url), "fd://%d", connection_fd);
+    std::string connection_url = llvm::formatv("fd://{0}", connection_fd).str();
 
     // Create the connection.
 #if LLDB_ENABLE_POSIX && !defined _WIN32
@@ -208,23 +207,20 @@
 #endif
     connection_up.reset(new ConnectionFileDescriptor);
     auto connection_result = connection_up->Connect(connection_url, &error);
+    if (error.Fail()) {
+      fprintf(stderr, "error: failed to connect to client at '%s': %s\n",
+              connection_url.c_str(), error.AsCString());
+      exit(-1);
+    }
     if (connection_result != eConnectionStatusSuccess) {
       fprintf(stderr, "error: failed to connect to client at '%s' "
                       "(connection status: %d)\n",
-              connection_url, static_cast<int>(connection_result));
-      exit(-1);
-    }
-    if (error.Fail()) {
-      fprintf(stderr, "error: failed to connect to client at '%s': %s\n",
-              connection_url, error.AsCString());
+              connection_url.c_str(), static_cast<int>(connection_result));
       exit(-1);
     }
   } else if (!host_and_port.empty()) {
     // Parse out host and port.
     std::string final_host_and_port;
-    std::string connection_host;
-    std::string connection_port;
-    uint32_t connection_portno = 0;
 
     // If host_and_port starts with ':', default the host to be "localhost" and
     // expect the remainder to be the port.
@@ -232,20 +228,15 @@
       final_host_and_port.append("localhost");
     final_host_and_port.append(host_and_port.str());
 
-    // Note: use rfind, because the host/port may look like "[::1]:12345".
-    const std::string::size_type colon_pos = final_host_and_port.rfind(':');
-    if (colon_pos != std::string::npos) {
-      connection_host = final_host_and_port.substr(0, colon_pos);
-      connection_port = final_host_and_port.substr(colon_pos + 1);
-      // FIXME: improve error handling
-      llvm::to_integer(connection_port, connection_portno);
-    }
-
-
     if (reverse_connect) {
       // llgs will connect to the gdb-remote client.
 
       // Ensure we have a port number for the connection.
+      // Note: use rfind, because the host/port may look like "[::1]:12345".
+      uint32_t connection_portno = 0;
+      const std::string::size_type colon_pos = final_host_and_port.rfind(':');
+      if (colon_pos != std::string::npos)
+        llvm::to_integer(final_host_and_port.substr(colon_pos + 1), connection_portno);
       if (connection_portno == 0) {
         fprintf(stderr, "error: port number must be specified on when using "
                         "reverse connect\n");
@@ -253,22 +244,21 @@
       }
 
       // Build the connection string.
-      char connection_url[512];
-      snprintf(connection_url, sizeof(connection_url), "connect://%s",
-               final_host_and_port.c_str());
+      final_host_and_port.insert(0, "connect://");
 
       // Create the connection.
       connection_up.reset(new ConnectionFileDescriptor);
-      auto connection_result = connection_up->Connect(connection_url, &error);
+      auto connection_result = connection_up->Connect(final_host_and_port,
+                                                      &error);
+      if (error.Fail()) {
+        fprintf(stderr, "error: failed to connect to client at '%s': %s\n",
+                final_host_and_port.c_str(), error.AsCString());
+        exit(-1);
+      }
       if (connection_result != eConnectionStatusSuccess) {
         fprintf(stderr, "error: failed to connect to client at '%s' "
                         "(connection status: %d)\n",
-                connection_url, static_cast<int>(connection_result));
-        exit(-1);
-      }
-      if (error.Fail()) {
-        fprintf(stderr, "error: failed to connect to client at '%s': %s\n",
-                connection_url, error.AsCString());
+                final_host_and_port.c_str(), static_cast<int>(connection_result));
         exit(-1);
       }
     } else {
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to