From ff569ed03092dba39effcc45e81d64beff800bb5 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Micha=C5=82=20G=C3=B3rny?= Date: Fri, 22 Oct 2021 14:00:07 +0200 Subject: [PATCH] [lldb] [Utility/UriParser] Replace port==-1 with llvm::None Use llvm::Optional 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 --- lldb/include/lldb/Utility/UriParser.h | 5 +++-- .../Plugins/Platform/Android/PlatformAndroid.cpp | 2 +- .../Android/PlatformAndroidRemoteGDBServer.cpp | 12 +++++------ .../gdb-server/PlatformRemoteGDBServer.cpp | 2 +- .../GDBRemoteCommunicationServerLLGS.cpp | 1 - .../GDBRemoteCommunicationServerPlatform.cpp | 2 +- lldb/source/Utility/UriParser.cpp | 4 ++-- lldb/tools/lldb-server/Acceptor.cpp | 2 +- .../Host/ConnectionFileDescriptorTest.cpp | 4 ++-- lldb/unittests/Host/SocketTest.cpp | 6 +++--- lldb/unittests/Utility/UriParserTest.cpp | 23 +++++++++++----------- 11 files changed, 31 insertions(+), 32 deletions(-) diff --git a/lldb/include/lldb/Utility/UriParser.h b/lldb/include/lldb/Utility/UriParser.h index 6a64c3d..f07df8f 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 @@ public: // 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 &port, llvm::StringRef &path); }; } diff --git a/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp b/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp index 1e66b86..f9b5cfd 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 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 2847bcf..3c9a33c 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: "); - int remote_port; + llvm::Optional 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 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 667618b..c1ee1c7 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 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 2080e09..b1e9e3c 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 7c2f80d..fc8a2a5 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 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 c6ed249..2c6b118 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 &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 16ecc54..285b64f 100644 --- a/lldb/tools/lldb-server/Acceptor.cpp +++ b/lldb/tools/lldb-server/Acceptor.cpp @@ -84,7 +84,7 @@ std::unique_ptr Acceptor::Create(StringRef name, error.Clear(); Socket::SocketProtocol socket_protocol = Socket::ProtocolUnixDomain; - int port; + llvm::Optional 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 76c54a9..89ab080 100644 --- a/lldb/unittests/Host/ConnectionFileDescriptorTest.cpp +++ b/lldb/unittests/Host/ConnectionFileDescriptorTest.cpp @@ -28,12 +28,12 @@ public: llvm::StringRef scheme; llvm::StringRef hostname; - int port; + llvm::Optional 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 6b9fb42..cbb9e78 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 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 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 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 c88a647..9010915 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 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 @@ public: bool m_result; const char *m_scheme; const char *m_hostname; - int m_port; + llvm::Optional m_port; const char *m_path; }; #define VALIDATE \ llvm::StringRef scheme(kAsdf); \ llvm::StringRef hostname(kAsdf); \ - int port(1138); \ + llvm::Optional port(1138); \ llvm::StringRef path(kAsdf); \ EXPECT_EQ(testCase.m_result, \ UriParser::Parse(testCase.m_uri, scheme, hostname, port, path)); \ @@ -40,7 +40,7 @@ public: 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 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 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 } -- 2.7.4