DavidSpickett created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
DavidSpickett requested review of this revision.
Herald added a subscriber: JDevlieghere.

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)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92035

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

Index: lldb/tools/lldb-server/lldb-platform.cpp
===================================================================
--- lldb/tools/lldb-server/lldb-platform.cpp
+++ lldb/tools/lldb-server/lldb-platform.cpp
@@ -350,13 +350,13 @@
     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());
       }
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
@@ -17,6 +17,7 @@
 #include "lldb/Host/Socket.h"
 
 #include "llvm/Support/Error.h"
+#include "llvm/ADT/Optional.h"
 
 namespace lldb_private {
 namespace process_gdb_remote {
@@ -65,8 +66,10 @@
 
   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,
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
@@ -157,8 +157,8 @@
 
 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 @@
 
   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 @@
 #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 @@
                                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 @@
   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 @@
   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 @@
             __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);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to