[Lldb-commits] [lldb] ff569ed - [lldb] [Utility/UriParser] Replace port==-1 with llvm::None

Michał Górny via lldb-commits lldb-commits at lists.llvm.org
Fri Oct 22 05:39:25 PDT 2021


Author: Michał Górny
Date: 2021-10-22T14:39:18+02:00
New Revision: ff569ed03092dba39effcc45e81d64beff800bb5

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

LOG: [lldb] [Utility/UriParser] Replace port==-1 with llvm::None

Use llvm::Optional<uint16_t> instead of int for port number
in UriParser::Parse(), and use llvm::None to indicate missing port
instead of a magic value of -1.

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

Added: 
    

Modified: 
    lldb/include/lldb/Utility/UriParser.h
    lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
    lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
    lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
    lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
    lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
    lldb/source/Utility/UriParser.cpp
    lldb/tools/lldb-server/Acceptor.cpp
    lldb/unittests/Host/ConnectionFileDescriptorTest.cpp
    lldb/unittests/Host/SocketTest.cpp
    lldb/unittests/Utility/UriParserTest.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Utility/UriParser.h b/lldb/include/lldb/Utility/UriParser.h
index 6a64c3d747b55..f07df8fb24735 100644
--- a/lldb/include/lldb/Utility/UriParser.h
+++ b/lldb/include/lldb/Utility/UriParser.h
@@ -9,6 +9,7 @@
 #ifndef LLDB_UTILITY_URIPARSER_H
 #define LLDB_UTILITY_URIPARSER_H
 
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
 
 namespace lldb_private {
@@ -18,12 +19,12 @@ class UriParser {
   // RETURN VALUE
   //   if url is valid, function returns true and
   //   scheme/hostname/port/path are set to the parsed values
-  //   port it set to -1 if it is not included in the URL
+  //   port it set to llvm::None if it is not included in the URL
   //
   //   if the url is invalid, function returns false and
   //   output parameters remain unchanged
   static bool Parse(llvm::StringRef uri, llvm::StringRef &scheme,
-                    llvm::StringRef &hostname, int &port,
+                    llvm::StringRef &hostname, llvm::Optional<uint16_t> &port,
                     llvm::StringRef &path);
 };
 }

diff  --git a/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp b/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
index 1e66b86e26615..f9b5cfd758d89 100644
--- a/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
+++ b/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
@@ -155,7 +155,7 @@ Status PlatformAndroid::ConnectRemote(Args &args) {
   if (!m_remote_platform_sp)
     m_remote_platform_sp = PlatformSP(new PlatformAndroidRemoteGDBServer());
 
-  int port;
+  llvm::Optional<uint16_t> port;
   llvm::StringRef scheme, host, path;
   const char *url = args.GetArgumentAtIndex(0);
   if (!url)

diff  --git a/lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp b/lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
index 2847bcfaa9260..3c9a33c93d27b 100644
--- a/lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
+++ b/lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
@@ -109,7 +109,7 @@ Status PlatformAndroidRemoteGDBServer::ConnectRemote(Args &args) {
     return Status(
         "\"platform connect\" takes a single argument: <connect-url>");
 
-  int remote_port;
+  llvm::Optional<uint16_t> remote_port;
   llvm::StringRef scheme, host, path;
   const char *url = args.GetArgumentAtIndex(0);
   if (!url)
@@ -126,9 +126,8 @@ Status PlatformAndroidRemoteGDBServer::ConnectRemote(Args &args) {
     m_socket_namespace = AdbClient::UnixSocketNamespaceAbstract;
 
   std::string connect_url;
-  auto error =
-      MakeConnectURL(g_remote_platform_pid, (remote_port < 0) ? 0 : remote_port,
-                     path, connect_url);
+  auto error = MakeConnectURL(g_remote_platform_pid, remote_port.getValueOr(0),
+                              path, connect_url);
 
   if (error.Fail())
     return error;
@@ -207,7 +206,7 @@ lldb::ProcessSP PlatformAndroidRemoteGDBServer::ConnectProcess(
   // any other valid pid on android.
   static lldb::pid_t s_remote_gdbserver_fake_pid = 0xffffffffffffffffULL;
 
-  int remote_port;
+  llvm::Optional<uint16_t> remote_port;
   llvm::StringRef scheme, host, path;
   if (!UriParser::Parse(connect_url, scheme, host, remote_port, path)) {
     error.SetErrorStringWithFormat("Invalid URL: %s",
@@ -217,8 +216,7 @@ lldb::ProcessSP PlatformAndroidRemoteGDBServer::ConnectProcess(
 
   std::string new_connect_url;
   error = MakeConnectURL(s_remote_gdbserver_fake_pid--,
-                         (remote_port < 0) ? 0 : remote_port, path,
-                         new_connect_url);
+                         remote_port.getValueOr(0), path, new_connect_url);
   if (error.Fail())
     return nullptr;
 

diff  --git a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
index 667618b26882f..c1ee1c7a220ca 100644
--- a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
+++ b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
@@ -305,7 +305,7 @@ Status PlatformRemoteGDBServer::ConnectRemote(Args &args) {
   if (!url)
     return Status("URL is null.");
 
-  int port;
+  llvm::Optional<uint16_t> port;
   llvm::StringRef scheme, hostname, pathname;
   if (!UriParser::Parse(url, scheme, hostname, port, pathname))
     return Status("Invalid URL: %s", url);

diff  --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 2080e09da3c2a..b1e9e3cb44157 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -39,7 +39,6 @@
 #include "lldb/Utility/State.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/UnimplementedError.h"
-#include "lldb/Utility/UriParser.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/ScopedPrinter.h"

diff  --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
index 7c2f80dc76b87..fc8a2a583213f 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
@@ -199,7 +199,7 @@ Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
   if (m_socket_protocol == Socket::ProtocolTcp) {
     llvm::StringRef platform_scheme;
     llvm::StringRef platform_ip;
-    int platform_port;
+    llvm::Optional<uint16_t> platform_port;
     llvm::StringRef platform_path;
     std::string platform_uri = GetConnection()->GetURI();
     bool ok = UriParser::Parse(platform_uri, platform_scheme, platform_ip,

diff  --git a/lldb/source/Utility/UriParser.cpp b/lldb/source/Utility/UriParser.cpp
index c6ed249858965..2c6b1182ca47a 100644
--- a/lldb/source/Utility/UriParser.cpp
+++ b/lldb/source/Utility/UriParser.cpp
@@ -17,7 +17,7 @@ using namespace lldb_private;
 
 // UriParser::Parse
 bool UriParser::Parse(llvm::StringRef uri, llvm::StringRef &scheme,
-                      llvm::StringRef &hostname, int &port,
+                      llvm::StringRef &hostname, llvm::Optional<uint16_t> &port,
                       llvm::StringRef &path) {
   llvm::StringRef tmp_scheme, tmp_hostname, tmp_path;
 
@@ -61,7 +61,7 @@ bool UriParser::Parse(llvm::StringRef uri, llvm::StringRef &scheme,
       return false;
     port = port_value;
   } else
-    port = -1;
+    port = llvm::None;
 
   scheme = tmp_scheme;
   hostname = tmp_hostname;

diff  --git a/lldb/tools/lldb-server/Acceptor.cpp b/lldb/tools/lldb-server/Acceptor.cpp
index 16ecc542cd47f..285b64fc9d3e2 100644
--- a/lldb/tools/lldb-server/Acceptor.cpp
+++ b/lldb/tools/lldb-server/Acceptor.cpp
@@ -84,7 +84,7 @@ std::unique_ptr<Acceptor> Acceptor::Create(StringRef name,
   error.Clear();
 
   Socket::SocketProtocol socket_protocol = Socket::ProtocolUnixDomain;
-  int port;
+  llvm::Optional<uint16_t> port;
   StringRef scheme, host, path;
   // Try to match socket name as URL - e.g., tcp://localhost:5555
   if (UriParser::Parse(name, scheme, host, port, path)) {

diff  --git a/lldb/unittests/Host/ConnectionFileDescriptorTest.cpp b/lldb/unittests/Host/ConnectionFileDescriptorTest.cpp
index 76c54a96b22e7..89ab08053949f 100644
--- a/lldb/unittests/Host/ConnectionFileDescriptorTest.cpp
+++ b/lldb/unittests/Host/ConnectionFileDescriptorTest.cpp
@@ -28,12 +28,12 @@ class ConnectionFileDescriptorTest : public testing::Test {
 
     llvm::StringRef scheme;
     llvm::StringRef hostname;
-    int port;
+    llvm::Optional<uint16_t> port;
     llvm::StringRef path;
     std::string uri(connection_file_descriptor.GetURI());
     EXPECT_TRUE(UriParser::Parse(uri, scheme, hostname, port, path));
     EXPECT_EQ(ip, hostname);
-    EXPECT_EQ(socket->GetRemotePortNumber(), port);
+    EXPECT_EQ(socket->GetRemotePortNumber(), port.getValue());
   }
 };
 

diff  --git a/lldb/unittests/Host/SocketTest.cpp b/lldb/unittests/Host/SocketTest.cpp
index 6b9fb42b4b166..cbb9e783c145c 100644
--- a/lldb/unittests/Host/SocketTest.cpp
+++ b/lldb/unittests/Host/SocketTest.cpp
@@ -173,7 +173,7 @@ TEST_P(SocketTest, TCPGetConnectURI) {
 
   llvm::StringRef scheme;
   llvm::StringRef hostname;
-  int port;
+  llvm::Optional<uint16_t> port;
   llvm::StringRef path;
   std::string uri(socket_a_up->GetRemoteConnectionURI());
   EXPECT_TRUE(UriParser::Parse(uri, scheme, hostname, port, path));
@@ -191,7 +191,7 @@ TEST_P(SocketTest, UDPGetConnectURI) {
 
   llvm::StringRef scheme;
   llvm::StringRef hostname;
-  int port;
+  llvm::Optional<uint16_t> port;
   llvm::StringRef path;
   std::string uri = socket.get()->GetRemoteConnectionURI();
   EXPECT_TRUE(UriParser::Parse(uri, scheme, hostname, port, path));
@@ -216,7 +216,7 @@ TEST_P(SocketTest, DomainGetConnectURI) {
 
   llvm::StringRef scheme;
   llvm::StringRef hostname;
-  int port;
+  llvm::Optional<uint16_t> port;
   llvm::StringRef path;
   std::string uri(socket_a_up->GetRemoteConnectionURI());
   EXPECT_TRUE(UriParser::Parse(uri, scheme, hostname, port, path));

diff  --git a/lldb/unittests/Utility/UriParserTest.cpp b/lldb/unittests/Utility/UriParserTest.cpp
index c88a647ef9378..90109150d63ed 100644
--- a/lldb/unittests/Utility/UriParserTest.cpp
+++ b/lldb/unittests/Utility/UriParserTest.cpp
@@ -11,7 +11,7 @@ static const char *kAsdf = "asdf";
 class UriTestCase {
 public:
   UriTestCase(const char *uri, const char *scheme, const char *hostname,
-              int port, const char *path)
+              llvm::Optional<uint16_t> port, const char *path)
       : m_uri(uri), m_result(true), m_scheme(scheme), m_hostname(hostname),
         m_port(port), m_path(path) {}
 
@@ -23,14 +23,14 @@ class UriTestCase {
   bool m_result;
   const char *m_scheme;
   const char *m_hostname;
-  int m_port;
+  llvm::Optional<uint16_t> m_port;
   const char *m_path;
 };
 
 #define VALIDATE                                                               \
   llvm::StringRef scheme(kAsdf);                                               \
   llvm::StringRef hostname(kAsdf);                                             \
-  int port(1138);                                                              \
+  llvm::Optional<uint16_t> port(1138);                                         \
   llvm::StringRef path(kAsdf);                                                 \
   EXPECT_EQ(testCase.m_result,                                                 \
             UriParser::Parse(testCase.m_uri, scheme, hostname, port, path));   \
@@ -40,7 +40,7 @@ class UriTestCase {
   EXPECT_STREQ(testCase.m_path, path.str().c_str());
 
 TEST(UriParserTest, Minimal) {
-  const UriTestCase testCase("x://y", "x", "y", -1, "/");
+  const UriTestCase testCase("x://y", "x", "y", llvm::None, "/");
   VALIDATE
 }
 
@@ -48,7 +48,7 @@ TEST(UriParserTest, MinimalPort) {
   const UriTestCase testCase("x://y:1", "x", "y", 1, "/");
   llvm::StringRef scheme(kAsdf);
   llvm::StringRef hostname(kAsdf);
-  int port(1138);
+  llvm::Optional<uint16_t> port(1138);
   llvm::StringRef path(kAsdf);
   bool result = UriParser::Parse(testCase.m_uri, scheme, hostname, port, path);
   EXPECT_EQ(testCase.m_result, result);
@@ -60,7 +60,7 @@ TEST(UriParserTest, MinimalPort) {
 }
 
 TEST(UriParserTest, MinimalPath) {
-  const UriTestCase testCase("x://y/", "x", "y", -1, "/");
+  const UriTestCase testCase("x://y/", "x", "y", llvm::None, "/");
   VALIDATE
 }
 
@@ -70,7 +70,8 @@ TEST(UriParserTest, MinimalPortPath) {
 }
 
 TEST(UriParserTest, LongPath) {
-  const UriTestCase testCase("x://y/abc/def/xyz", "x", "y", -1, "/abc/def/xyz");
+  const UriTestCase testCase("x://y/abc/def/xyz", "x", "y", llvm::None,
+                             "/abc/def/xyz");
   VALIDATE
 }
 
@@ -92,7 +93,7 @@ TEST(UriParserTest, BracketedHostnamePort) {
                              "192.168.100.132", 5432, "/");
   llvm::StringRef scheme(kAsdf);
   llvm::StringRef hostname(kAsdf);
-  int port(1138);
+  llvm::Optional<uint16_t> port(1138);
   llvm::StringRef path(kAsdf);
   bool result = UriParser::Parse(testCase.m_uri, scheme, hostname, port, path);
   EXPECT_EQ(testCase.m_result, result);
@@ -105,14 +106,14 @@ TEST(UriParserTest, BracketedHostnamePort) {
 
 TEST(UriParserTest, BracketedHostname) {
   const UriTestCase testCase("connect://[192.168.100.132]", "connect",
-                             "192.168.100.132", -1, "/");
+                             "192.168.100.132", llvm::None, "/");
   VALIDATE
 }
 
 TEST(UriParserTest, BracketedHostnameWithPortIPv4) {
   // Android device over IPv4: port is a part of the hostname.
   const UriTestCase testCase("connect://[192.168.100.132:1234]", "connect",
-                             "192.168.100.132:1234", -1, "/");
+                             "192.168.100.132:1234", llvm::None, "/");
   VALIDATE
 }
 
@@ -120,7 +121,7 @@ TEST(UriParserTest, BracketedHostnameWithPortIPv6) {
   // Android device over IPv6: port is a part of the hostname.
   const UriTestCase testCase(
       "connect://[[2601:600:107f:db64:a42b:4faa:284]:1234]", "connect",
-      "[2601:600:107f:db64:a42b:4faa:284]:1234", -1, "/");
+      "[2601:600:107f:db64:a42b:4faa:284]:1234", llvm::None, "/");
   VALIDATE
 }
 


        


More information about the lldb-commits mailing list