[Lldb-commits] [lldb] 4b46a41 - [lldb] [ConnectionFileDescriptorPosix] Refactor scheme matching
Michał Górny via lldb-commits
lldb-commits at lists.llvm.org
Fri Oct 8 02:48:06 PDT 2021
Author: Michał Górny
Date: 2021-10-08T11:47:57+02:00
New Revision: 4b46a4134385eda6449e93ad124f9b2685b25bb2
URL: https://github.com/llvm/llvm-project/commit/4b46a4134385eda6449e93ad124f9b2685b25bb2
DIFF: https://github.com/llvm/llvm-project/commit/4b46a4134385eda6449e93ad124f9b2685b25bb2.diff
LOG: [lldb] [ConnectionFileDescriptorPosix] Refactor scheme matching
Move the POSIX-specific fd:// and file:// scheme handling into
separate methods. Replace the custom GetURLAddress() matching with
splitting into scheme and path, and matching scheme via
llvm::StringSwitch. Use early returns.
Differential Revision: https://reviews.llvm.org/D111321
Added:
Modified:
lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
index 42be989dfa7b6..57bb01d42f232 100644
--- a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
+++ b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
@@ -28,17 +28,6 @@ class SocketAddress;
class ConnectionFileDescriptor : public Connection {
public:
- static const char *LISTEN_SCHEME;
- static const char *ACCEPT_SCHEME;
- static const char *UNIX_ACCEPT_SCHEME;
- static const char *CONNECT_SCHEME;
- static const char *TCP_CONNECT_SCHEME;
- static const char *UDP_SCHEME;
- static const char *UNIX_CONNECT_SCHEME;
- static const char *UNIX_ABSTRACT_CONNECT_SCHEME;
- static const char *FD_SCHEME;
- static const char *FILE_SCHEME;
-
ConnectionFileDescriptor(bool child_processes_inherit = false);
ConnectionFileDescriptor(int fd, bool owns_fd);
@@ -95,6 +84,10 @@ class ConnectionFileDescriptor : public Connection {
lldb::ConnectionStatus UnixAbstractSocketConnect(llvm::StringRef socket_name,
Status *error_ptr);
+ lldb::ConnectionStatus ConnectFD(llvm::StringRef args, Status *error_ptr);
+
+ lldb::ConnectionStatus ConnectFile(llvm::StringRef args, Status *error_ptr);
+
lldb::IOObjectSP m_read_sp;
lldb::IOObjectSP m_write_sp;
diff --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
index 9a6a5aa835f55..b192c65b619d9 100644
--- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -50,27 +50,6 @@
using namespace lldb;
using namespace lldb_private;
-const char *ConnectionFileDescriptor::LISTEN_SCHEME = "listen";
-const char *ConnectionFileDescriptor::ACCEPT_SCHEME = "accept";
-const char *ConnectionFileDescriptor::UNIX_ACCEPT_SCHEME = "unix-accept";
-const char *ConnectionFileDescriptor::CONNECT_SCHEME = "connect";
-const char *ConnectionFileDescriptor::TCP_CONNECT_SCHEME = "tcp-connect";
-const char *ConnectionFileDescriptor::UDP_SCHEME = "udp";
-const char *ConnectionFileDescriptor::UNIX_CONNECT_SCHEME = "unix-connect";
-const char *ConnectionFileDescriptor::UNIX_ABSTRACT_CONNECT_SCHEME =
- "unix-abstract-connect";
-const char *ConnectionFileDescriptor::FD_SCHEME = "fd";
-const char *ConnectionFileDescriptor::FILE_SCHEME = "file";
-
-static llvm::Optional<llvm::StringRef> GetURLAddress(llvm::StringRef url,
- llvm::StringRef scheme) {
- if (!url.consume_front(scheme))
- return llvm::None;
- if (!url.consume_front("://"))
- return llvm::None;
- return url;
-}
-
ConnectionFileDescriptor::ConnectionFileDescriptor(bool child_processes_inherit)
: Connection(), m_pipe(), m_mutex(), m_shutting_down(false),
@@ -154,133 +133,41 @@ ConnectionStatus ConnectionFileDescriptor::Connect(llvm::StringRef path,
OpenCommandPipe();
- if (!path.empty()) {
- llvm::Optional<llvm::StringRef> addr;
- if ((addr = GetURLAddress(path, LISTEN_SCHEME))) {
- // listen://HOST:PORT
- return SocketListenAndAccept(*addr, error_ptr);
- } else if ((addr = GetURLAddress(path, ACCEPT_SCHEME))) {
- // unix://SOCKNAME
- return NamedSocketAccept(*addr, error_ptr);
- } else if ((addr = GetURLAddress(path, UNIX_ACCEPT_SCHEME))) {
- // unix://SOCKNAME
- return NamedSocketAccept(*addr, error_ptr);
- } else if ((addr = GetURLAddress(path, CONNECT_SCHEME))) {
- return ConnectTCP(*addr, error_ptr);
- } else if ((addr = GetURLAddress(path, TCP_CONNECT_SCHEME))) {
- return ConnectTCP(*addr, error_ptr);
- } else if ((addr = GetURLAddress(path, UDP_SCHEME))) {
- return ConnectUDP(*addr, error_ptr);
- } else if ((addr = GetURLAddress(path, UNIX_CONNECT_SCHEME))) {
- // unix-connect://SOCKNAME
- return NamedSocketConnect(*addr, error_ptr);
- } else if ((addr = GetURLAddress(path, UNIX_ABSTRACT_CONNECT_SCHEME))) {
- // unix-abstract-connect://SOCKNAME
- return UnixAbstractSocketConnect(*addr, error_ptr);
- }
-#if LLDB_ENABLE_POSIX
- else if ((addr = GetURLAddress(path, FD_SCHEME))) {
- // Just passing a native file descriptor within this current process that
- // is already opened (possibly from a service or other source).
- int fd = -1;
-
- if (!addr->getAsInteger(0, fd)) {
- // We have what looks to be a valid file descriptor, but we should make
- // sure it is. We currently are doing this by trying to get the flags
- // from the file descriptor and making sure it isn't a bad fd.
- errno = 0;
- int flags = ::fcntl(fd, F_GETFL, 0);
- if (flags == -1 || errno == EBADF) {
- if (error_ptr)
- error_ptr->SetErrorStringWithFormat("stale file descriptor: %s",
- path.str().c_str());
- m_read_sp.reset();
- m_write_sp.reset();
- return eConnectionStatusError;
- } else {
- // Don't take ownership of a file descriptor that gets passed to us
- // since someone else opened the file descriptor and handed it to us.
- // TODO: Since are using a URL to open connection we should
- // eventually parse options using the web standard where we have
- // "fd://123?opt1=value;opt2=value" and we can have an option be
- // "owns=1" or "owns=0" or something like this to allow us to specify
- // this. For now, we assume we must assume we don't own it.
-
- std::unique_ptr<TCPSocket> tcp_socket;
- tcp_socket = std::make_unique<TCPSocket>(fd, false, false);
- // Try and get a socket option from this file descriptor to see if
- // this is a socket and set m_is_socket accordingly.
- int resuse;
- bool is_socket =
- !!tcp_socket->GetOption(SOL_SOCKET, SO_REUSEADDR, resuse);
- if (is_socket) {
- m_read_sp = std::move(tcp_socket);
- m_write_sp = m_read_sp;
- } else {
- m_read_sp = std::make_shared<NativeFile>(
- fd, File::eOpenOptionReadWrite, false);
- m_write_sp = m_read_sp;
- }
- m_uri = std::string(*addr);
- return eConnectionStatusSuccess;
- }
- }
-
- if (error_ptr)
- error_ptr->SetErrorStringWithFormat("invalid file descriptor: \"%s\"",
- path.str().c_str());
- m_read_sp.reset();
- m_write_sp.reset();
- return eConnectionStatusError;
- } else if ((addr = GetURLAddress(path, FILE_SCHEME))) {
- std::string addr_str = addr->str();
- // file:///PATH
- int fd = llvm::sys::RetryAfterSignal(-1, ::open, addr_str.c_str(), O_RDWR);
- if (fd == -1) {
- if (error_ptr)
- error_ptr->SetErrorToErrno();
- return eConnectionStatusError;
- }
-
- if (::isatty(fd)) {
- // Set up serial terminal emulation
- struct termios options;
- ::tcgetattr(fd, &options);
-
- // Set port speed to maximum
- ::cfsetospeed(&options, B115200);
- ::cfsetispeed(&options, B115200);
-
- // Raw input, disable echo and signals
- options.c_lflag &= ~(ICANON | ECHO | ECHOE | ISIG);
-
- // Make sure only one character is needed to return from a read
- options.c_cc[VMIN] = 1;
- options.c_cc[VTIME] = 0;
+ if (path.empty()) {
+ if (error_ptr)
+ error_ptr->SetErrorString("invalid connect arguments");
+ return eConnectionStatusError;
+ }
- llvm::sys::RetryAfterSignal(-1, ::tcsetattr, fd, TCSANOW, &options);
- }
+ llvm::StringRef scheme;
+ std::tie(scheme, path) = path.split("://");
- int flags = ::fcntl(fd, F_GETFL, 0);
- if (flags >= 0) {
- if ((flags & O_NONBLOCK) == 0) {
- flags |= O_NONBLOCK;
- ::fcntl(fd, F_SETFL, flags);
- }
- }
- m_read_sp =
- std::make_shared<NativeFile>(fd, File::eOpenOptionReadWrite, true);
- m_write_sp = m_read_sp;
- return eConnectionStatusSuccess;
- }
+ if (!path.empty()) {
+ auto method =
+ llvm::StringSwitch<ConnectionStatus (ConnectionFileDescriptor::*)(
+ llvm::StringRef, Status *)>(scheme)
+ .Case("listen", &ConnectionFileDescriptor::SocketListenAndAccept)
+ .Cases("accept", "unix-accept",
+ &ConnectionFileDescriptor::NamedSocketAccept)
+ .Cases("connect", "tcp-connect",
+ &ConnectionFileDescriptor::ConnectTCP)
+ .Case("udp", &ConnectionFileDescriptor::ConnectUDP)
+ .Case("unix-connect", &ConnectionFileDescriptor::NamedSocketConnect)
+ .Case("unix-abstract-connect",
+ &ConnectionFileDescriptor::UnixAbstractSocketConnect)
+#if LLDB_ENABLE_POSIX
+ .Case("fd", &ConnectionFileDescriptor::ConnectFD)
+ .Case("file", &ConnectionFileDescriptor::ConnectFile)
#endif
- if (error_ptr)
- error_ptr->SetErrorStringWithFormat("unsupported connection URL: '%s'",
- path.str().c_str());
- return eConnectionStatusError;
+ .Default(nullptr);
+
+ if (method)
+ return (this->*method)(path, error_ptr);
}
+
if (error_ptr)
- error_ptr->SetErrorString("invalid connect arguments");
+ error_ptr->SetErrorStringWithFormat("unsupported connection URL: '%s'",
+ path.str().c_str());
return eConnectionStatusError;
}
@@ -761,6 +648,111 @@ ConnectionStatus ConnectionFileDescriptor::ConnectUDP(llvm::StringRef s,
return eConnectionStatusSuccess;
}
+ConnectionStatus ConnectionFileDescriptor::ConnectFD(llvm::StringRef s,
+ Status *error_ptr) {
+#if LLDB_ENABLE_POSIX
+ // Just passing a native file descriptor within this current process that
+ // is already opened (possibly from a service or other source).
+ int fd = -1;
+
+ if (!s.getAsInteger(0, fd)) {
+ // We have what looks to be a valid file descriptor, but we should make
+ // sure it is. We currently are doing this by trying to get the flags
+ // from the file descriptor and making sure it isn't a bad fd.
+ errno = 0;
+ int flags = ::fcntl(fd, F_GETFL, 0);
+ if (flags == -1 || errno == EBADF) {
+ if (error_ptr)
+ error_ptr->SetErrorStringWithFormat("stale file descriptor: %s",
+ s.str().c_str());
+ m_read_sp.reset();
+ m_write_sp.reset();
+ return eConnectionStatusError;
+ } else {
+ // Don't take ownership of a file descriptor that gets passed to us
+ // since someone else opened the file descriptor and handed it to us.
+ // TODO: Since are using a URL to open connection we should
+ // eventually parse options using the web standard where we have
+ // "fd://123?opt1=value;opt2=value" and we can have an option be
+ // "owns=1" or "owns=0" or something like this to allow us to specify
+ // this. For now, we assume we must assume we don't own it.
+
+ std::unique_ptr<TCPSocket> tcp_socket;
+ tcp_socket = std::make_unique<TCPSocket>(fd, false, false);
+ // Try and get a socket option from this file descriptor to see if
+ // this is a socket and set m_is_socket accordingly.
+ int resuse;
+ bool is_socket =
+ !!tcp_socket->GetOption(SOL_SOCKET, SO_REUSEADDR, resuse);
+ if (is_socket) {
+ m_read_sp = std::move(tcp_socket);
+ m_write_sp = m_read_sp;
+ } else {
+ m_read_sp =
+ std::make_shared<NativeFile>(fd, File::eOpenOptionReadWrite, false);
+ m_write_sp = m_read_sp;
+ }
+ m_uri = s.str();
+ return eConnectionStatusSuccess;
+ }
+ }
+
+ if (error_ptr)
+ error_ptr->SetErrorStringWithFormat("invalid file descriptor: \"%s\"",
+ s.str().c_str());
+ m_read_sp.reset();
+ m_write_sp.reset();
+ return eConnectionStatusError;
+#endif // LLDB_ENABLE_POSIX
+ llvm_unreachable("this function should be only called w/ LLDB_ENABLE_POSIX");
+}
+
+ConnectionStatus ConnectionFileDescriptor::ConnectFile(llvm::StringRef s,
+ Status *error_ptr) {
+#if LLDB_ENABLE_POSIX
+ std::string addr_str = s.str();
+ // file:///PATH
+ int fd = llvm::sys::RetryAfterSignal(-1, ::open, addr_str.c_str(), O_RDWR);
+ if (fd == -1) {
+ if (error_ptr)
+ error_ptr->SetErrorToErrno();
+ return eConnectionStatusError;
+ }
+
+ if (::isatty(fd)) {
+ // Set up serial terminal emulation
+ struct termios options;
+ ::tcgetattr(fd, &options);
+
+ // Set port speed to maximum
+ ::cfsetospeed(&options, B115200);
+ ::cfsetispeed(&options, B115200);
+
+ // Raw input, disable echo and signals
+ options.c_lflag &= ~(ICANON | ECHO | ECHOE | ISIG);
+
+ // Make sure only one character is needed to return from a read
+ options.c_cc[VMIN] = 1;
+ options.c_cc[VTIME] = 0;
+
+ llvm::sys::RetryAfterSignal(-1, ::tcsetattr, fd, TCSANOW, &options);
+ }
+
+ int flags = ::fcntl(fd, F_GETFL, 0);
+ if (flags >= 0) {
+ if ((flags & O_NONBLOCK) == 0) {
+ flags |= O_NONBLOCK;
+ ::fcntl(fd, F_SETFL, flags);
+ }
+ }
+ m_read_sp =
+ std::make_shared<NativeFile>(fd, File::eOpenOptionReadWrite, true);
+ m_write_sp = m_read_sp;
+ return eConnectionStatusSuccess;
+#endif // LLDB_ENABLE_POSIX
+ llvm_unreachable("this function should be only called w/ LLDB_ENABLE_POSIX");
+}
+
uint16_t
ConnectionFileDescriptor::GetListeningPort(const Timeout<std::micro> &timeout) {
auto Result = m_port_predicate.WaitForValueNotEqualTo(0, timeout);
diff --git a/lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp b/lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
index 77690b4efabd8..2847bcfaa9260 100644
--- a/lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
+++ b/lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
@@ -120,9 +120,9 @@ Status PlatformAndroidRemoteGDBServer::ConnectRemote(Args &args) {
m_device_id = std::string(host);
m_socket_namespace.reset();
- if (scheme == ConnectionFileDescriptor::UNIX_CONNECT_SCHEME)
+ if (scheme == "unix-connect")
m_socket_namespace = AdbClient::UnixSocketNamespaceFileSystem;
- else if (scheme == ConnectionFileDescriptor::UNIX_ABSTRACT_CONNECT_SCHEME)
+ else if (scheme == "unix-abstract-connect")
m_socket_namespace = AdbClient::UnixSocketNamespaceAbstract;
std::string connect_url;
More information about the lldb-commits
mailing list