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