[llvm-branch-commits] [lldb] a7f8d96 - [lldb] Use llvm::Optional for port in LaunchGDBServer

[David Spickett via llvm-branch-commits]

Author: David Spickett
Date: 2020-11-30T11:20:39Z
New Revision: a7f8d96b16a394a87230d592c727906f67a4ba07

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

LOG: [lldb] Use llvm::Optional for port in LaunchGDBServer

Previously we used UINT16_MAX to mean no port/no specifc
port. This leads to confusion because 65535 is a valid
port number.

Instead use an optional. If you want a specific port call
LaunchGDBServer as normal, otherwise pass an empty optional
and it will be set to the port that gets chosen.
(or left empty in the case where we fail to find a port)

Reviewed By: labath

Differential Revision: https://reviews.llvm.org/D92035

Added: 
    

Modified: 
    lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
    lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
    lldb/tools/lldb-server/lldb-platform.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
index ae1260221e56..4aa153460941 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
@@ -157,8 +157,8 @@ GDBRemoteCommunicationServerPlatform::~GDBRemoteCommunicationServerPlatform() {}
 
 Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
     const lldb_private::Args &args, std::string hostname, lldb::pid_t &pid,
-    uint16_t &port, std::string &socket_name) {
-  if (port == UINT16_MAX) {
+    llvm::Optional<uint16_t> &port, std::string &socket_name) {
+  if (!port) {
     llvm::Expected<uint16_t> available_port = m_port_map.GetNextAvailablePort();
     if (available_port)
       port = *available_port;
@@ -179,7 +179,7 @@ Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
 
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PLATFORM));
   LLDB_LOGF(log, "Launching debugserver with: %s:%u...", hostname.c_str(),
-            port);
+            *port);
 
   // Do not run in a new session so that it can not linger after the platform
   // closes.
@@ -194,7 +194,7 @@ Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
 #if !defined(__APPLE__)
   url << m_socket_scheme << "://";
 #endif
-  uint16_t *port_ptr = &port;
+  uint16_t *port_ptr = port.getPointer();
   if (m_socket_protocol == Socket::ProtocolTcp) {
     llvm::StringRef platform_scheme;
     llvm::StringRef platform_ip;
@@ -205,7 +205,7 @@ Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
                                platform_port, platform_path);
     UNUSED_IF_ASSERT_DISABLED(ok);
     assert(ok);
-    url << platform_ip.str() << ":" << port;
+    url << platform_ip.str() << ":" << *port;
   } else {
     socket_name = GetDomainSocketPath("gdbserver").GetPath();
     url << socket_name;
@@ -219,11 +219,11 @@ Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
   if (pid != LLDB_INVALID_PROCESS_ID) {
     std::lock_guard<std::recursive_mutex> guard(m_spawned_pids_mutex);
     m_spawned_pids.insert(pid);
-    if (port > 0)
-      m_port_map.AssociatePortWithProcess(port, pid);
+    if (*port > 0)
+      m_port_map.AssociatePortWithProcess(*port, pid);
   } else {
-    if (port > 0)
-      m_port_map.FreePort(port);
+    if (*port > 0)
+      m_port_map.FreePort(*port);
   }
   return error;
 }
@@ -243,12 +243,15 @@ GDBRemoteCommunicationServerPlatform::Handle_qLaunchGDBServer(
   packet.SetFilePos(::strlen("qLaunchGDBServer;"));
   llvm::StringRef name;
   llvm::StringRef value;
-  uint16_t port = UINT16_MAX;
+  llvm::Optional<uint16_t> port;
   while (packet.GetNameColonValue(name, value)) {
     if (name.equals("host"))
       hostname = std::string(value);
-    else if (name.equals("port"))
-      value.getAsInteger(0, port);
+    else if (name.equals("port")) {
+      // Make the Optional valid so we can use its value
+      port = 0;
+      value.getAsInteger(0, port.getValue());
+    }
   }
 
   lldb::pid_t debugserver_pid = LLDB_INVALID_PROCESS_ID;
@@ -269,8 +272,9 @@ GDBRemoteCommunicationServerPlatform::Handle_qLaunchGDBServer(
             __FUNCTION__, debugserver_pid);
 
   StreamGDBRemote response;
+  assert(port);
   response.Printf("pid:%" PRIu64 ";port:%u;", debugserver_pid,
-                  port + m_port_offset);
+                  *port + m_port_offset);
   if (!socket_name.empty()) {
     response.PutCString("socket_name:");
     response.PutStringAsRawHex8(socket_name);

diff  --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
index 69261dd2ad64..6b964da4a279 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
@@ -16,6 +16,7 @@
 #include "GDBRemoteCommunicationServerCommon.h"
 #include "lldb/Host/Socket.h"
 
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/Error.h"
 
 namespace lldb_private {
@@ -86,8 +87,10 @@ class GDBRemoteCommunicationServerPlatform
 
   void SetInferiorArguments(const lldb_private::Args &args);
 
+  // Set port if you want to use a specific port number.
+  // Otherwise port will be set to the port that was chosen for you.
   Status LaunchGDBServer(const lldb_private::Args &args, std::string hostname,
-                         lldb::pid_t &pid, uint16_t &port,
+                         lldb::pid_t &pid, llvm::Optional<uint16_t> &port,
                          std::string &socket_name);
 
   void SetPendingGdbServer(lldb::pid_t pid, uint16_t port,

diff  --git a/lldb/tools/lldb-server/lldb-platform.cpp b/lldb/tools/lldb-server/lldb-platform.cpp
index 05bfe71117c6..a5d4ecfa3f29 100644
--- a/lldb/tools/lldb-server/lldb-platform.cpp
+++ b/lldb/tools/lldb-server/lldb-platform.cpp
@@ -349,13 +349,13 @@ int main_platform(int argc, char *argv[]) {
     if (platform.IsConnected()) {
       if (inferior_arguments.GetArgumentCount() > 0) {
         lldb::pid_t pid = LLDB_INVALID_PROCESS_ID;
-        uint16_t port = 0;
+        llvm::Optional<uint16_t> port = 0;
         std::string socket_name;
         Status error = platform.LaunchGDBServer(inferior_arguments,
                                                 "", // hostname
                                                 pid, port, socket_name);
         if (error.Success())
-          platform.SetPendingGdbServer(pid, port, socket_name);
+          platform.SetPendingGdbServer(pid, *port, socket_name);
         else
           fprintf(stderr, "failed to start gdbserver: %s\n", error.AsCString());
       }