[lldb] Clean up GDBRemoteCommunication::StartDebugserverProcess #145021
+274
−263
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It creates a pair of connected sockets using the simplest mechanism for the given platform (TCP on windows, socketpair(2) elsewhere). Main motivation is to remove the ugly platform-specific code in ProcessGDBRemote::LaunchAndConnectToDebugserver, but it can also be used in other places where we need to create a pair of connected sockets.
This lets get rid of platform-specific code in ProcessGDBRemote and use the same code path (module differences in socket types) everywhere. It also unlocks further cleanups in the debugserver launching code.
The function was extremely messy in that it, depending on the set of arguments, it could either modify the Connection object in `this` or not. It had a lot of arguments, with each call site passing a different combination of null values. This PR: - packs "url" and "comm_fd" arguments into a variant as they are mutually exclusive - removes the "null url *and* null comm_fd" which is not used as of llvm#145017 - marks the function as `static` to make it clear it (now) does not operate on the `this` object.
|
@llvm/pr-subscribers-lldb Author: Pavel Labath (labath) ChangesThe function was extremely messy in that it, depending on the set of
Depends on #145017 (This PR consists of three commits, the first two of which are equivalent to #145015 and #145017, respectively. For reviewing, I recommend only looking at the last commit. If you have comments on the first two, please put them on their respective PRs.) Patch is 32.80 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/145021.diff 12 Files Affected:
diff --git a/lldb/include/lldb/Host/Socket.h b/lldb/include/lldb/Host/Socket.h
index c313aa4f6d26b..14a9660ed30b7 100644
--- a/lldb/include/lldb/Host/Socket.h
+++ b/lldb/include/lldb/Host/Socket.h
@@ -106,6 +106,10 @@ class Socket : public IOObject {
static std::unique_ptr<Socket> Create(const SocketProtocol protocol,
Status &error);
+ static llvm::Expected<
+ std::pair<std::unique_ptr<Socket>, std::unique_ptr<Socket>>>
+ CreatePair(std::optional<SocketProtocol> protocol = std::nullopt);
+
virtual Status Connect(llvm::StringRef name) = 0;
virtual Status Listen(llvm::StringRef name, int backlog) = 0;
diff --git a/lldb/include/lldb/Host/common/TCPSocket.h b/lldb/include/lldb/Host/common/TCPSocket.h
index cb950c0015ea6..e81cb82dbcba1 100644
--- a/lldb/include/lldb/Host/common/TCPSocket.h
+++ b/lldb/include/lldb/Host/common/TCPSocket.h
@@ -23,6 +23,10 @@ class TCPSocket : public Socket {
TCPSocket(NativeSocket socket, bool should_close);
~TCPSocket() override;
+ static llvm::Expected<
+ std::pair<std::unique_ptr<TCPSocket>, std::unique_ptr<TCPSocket>>>
+ CreatePair();
+
// returns port number or 0 if error
uint16_t GetLocalPortNumber() const;
diff --git a/lldb/include/lldb/Host/posix/DomainSocket.h b/lldb/include/lldb/Host/posix/DomainSocket.h
index a840d474429ec..c3a6a64bbdef8 100644
--- a/lldb/include/lldb/Host/posix/DomainSocket.h
+++ b/lldb/include/lldb/Host/posix/DomainSocket.h
@@ -19,6 +19,10 @@ class DomainSocket : public Socket {
DomainSocket(NativeSocket socket, bool should_close);
explicit DomainSocket(bool should_close);
+ static llvm::Expected<
+ std::pair<std::unique_ptr<DomainSocket>, std::unique_ptr<DomainSocket>>>
+ CreatePair();
+
Status Connect(llvm::StringRef name) override;
Status Listen(llvm::StringRef name, int backlog) override;
diff --git a/lldb/source/Host/common/Socket.cpp b/lldb/source/Host/common/Socket.cpp
index 5c5cd653c3d9e..c9dec0f8ea22a 100644
--- a/lldb/source/Host/common/Socket.cpp
+++ b/lldb/source/Host/common/Socket.cpp
@@ -234,6 +234,23 @@ std::unique_ptr<Socket> Socket::Create(const SocketProtocol protocol,
return socket_up;
}
+llvm::Expected<std::pair<std::unique_ptr<Socket>, std::unique_ptr<Socket>>>
+Socket::CreatePair(std::optional<SocketProtocol> protocol) {
+ constexpr SocketProtocol kBestProtocol =
+ LLDB_ENABLE_POSIX ? ProtocolUnixDomain : ProtocolTcp;
+ switch (protocol.value_or(kBestProtocol)) {
+ case ProtocolTcp:
+ return TCPSocket::CreatePair();
+#if LLDB_ENABLE_POSIX
+ case ProtocolUnixDomain:
+ case ProtocolUnixAbstract:
+ return DomainSocket::CreatePair();
+#endif
+ default:
+ return llvm::createStringError("Unsupported protocol");
+ }
+}
+
llvm::Expected<std::unique_ptr<Socket>>
Socket::TcpConnect(llvm::StringRef host_and_port) {
Log *log = GetLog(LLDBLog::Connection);
diff --git a/lldb/source/Host/common/TCPSocket.cpp b/lldb/source/Host/common/TCPSocket.cpp
index 3d0dea1c61dd6..34f249746149e 100644
--- a/lldb/source/Host/common/TCPSocket.cpp
+++ b/lldb/source/Host/common/TCPSocket.cpp
@@ -52,6 +52,34 @@ TCPSocket::TCPSocket(NativeSocket socket, bool should_close)
TCPSocket::~TCPSocket() { CloseListenSockets(); }
+llvm::Expected<
+ std::pair<std::unique_ptr<TCPSocket>, std::unique_ptr<TCPSocket>>>
+TCPSocket::CreatePair() {
+ auto listen_socket_up = std::make_unique<TCPSocket>(true);
+ if (Status error = listen_socket_up->Listen("localhost:0", 5); error.Fail())
+ return error.takeError();
+
+ std::string connect_address =
+ llvm::StringRef(listen_socket_up->GetListeningConnectionURI()[0])
+ .split("://")
+ .second.str();
+
+ auto connect_socket_up = std::make_unique<TCPSocket>(true);
+ if (Status error = connect_socket_up->Connect(connect_address); error.Fail())
+ return error.takeError();
+
+ // Connection has already been made above, so a short timeout is sufficient.
+ Socket *accept_socket;
+ if (Status error =
+ listen_socket_up->Accept(std::chrono::seconds(1), accept_socket);
+ error.Fail())
+ return error.takeError();
+
+ return std::make_pair(
+ std::move(connect_socket_up),
+ std::unique_ptr<TCPSocket>(static_cast<TCPSocket *>(accept_socket)));
+}
+
bool TCPSocket::IsValid() const {
return m_socket != kInvalidSocketValue || m_listen_sockets.size() != 0;
}
diff --git a/lldb/source/Host/posix/DomainSocket.cpp b/lldb/source/Host/posix/DomainSocket.cpp
index 4f76e0c16d4c7..0202dea45a8e1 100644
--- a/lldb/source/Host/posix/DomainSocket.cpp
+++ b/lldb/source/Host/posix/DomainSocket.cpp
@@ -13,9 +13,11 @@
#endif
#include "llvm/Support/Errno.h"
+#include "llvm/Support/Error.h"
#include "llvm/Support/FileSystem.h"
#include <cstddef>
+#include <fcntl.h>
#include <memory>
#include <sys/socket.h>
#include <sys/un.h>
@@ -76,6 +78,33 @@ DomainSocket::DomainSocket(SocketProtocol protocol, NativeSocket socket,
m_socket = socket;
}
+llvm::Expected<
+ std::pair<std::unique_ptr<DomainSocket>, std::unique_ptr<DomainSocket>>>
+DomainSocket::CreatePair() {
+ int sockets[2];
+ int type = SOCK_STREAM;
+#ifdef SOCK_CLOEXEC
+ type |= SOCK_CLOEXEC;
+#endif
+ if (socketpair(AF_UNIX, type, 0, sockets) == -1)
+ return llvm::errorCodeToError(llvm::errnoAsErrorCode());
+
+#ifndef SOCK_CLOEXEC
+ for (int s : sockets) {
+ int r = fcntl(s, F_SETFD, FD_CLOEXEC | fcntl(s, F_GETFD));
+ assert(r == 0);
+ (void)r;
+ }
+#endif
+
+ return std::make_pair(std::unique_ptr<DomainSocket>(
+ new DomainSocket(ProtocolUnixDomain, sockets[0],
+ /*should_close=*/true)),
+ std::unique_ptr<DomainSocket>(
+ new DomainSocket(ProtocolUnixDomain, sockets[1],
+ /*should_close=*/true)));
+}
+
Status DomainSocket::Connect(llvm::StringRef name) {
sockaddr_un saddr_un;
socklen_t saddr_un_len;
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
index 2aea7c6b781d7..776b0c878a835 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -31,6 +31,7 @@
#include <climits>
#include <cstring>
#include <sys/stat.h>
+#include <variant>
#if defined(__APPLE__)
#define DEBUGSERVER_BASENAME "debugserver"
@@ -894,11 +895,9 @@ FileSpec GDBRemoteCommunication::GetDebugserverPath(Platform *platform) {
}
Status GDBRemoteCommunication::StartDebugserverProcess(
- const char *url, Platform *platform, ProcessLaunchInfo &launch_info,
- uint16_t *port, const Args *inferior_args, shared_fd_t pass_comm_fd) {
+ std::variant<llvm::StringRef, shared_fd_t> comm, Platform *platform,
+ ProcessLaunchInfo &launch_info, const Args *inferior_args) {
Log *log = GetLog(GDBRLog::Process);
- LLDB_LOG(log, "Starting debug server: url={0}, port={1}",
- url ? url : "<empty>", port ? *port : uint16_t(0));
FileSpec debugserver_file_spec = GetDebugserverPath(platform);
if (!debugserver_file_spec)
@@ -911,89 +910,58 @@ Status GDBRemoteCommunication::StartDebugserverProcess(
#if !defined(__APPLE__)
// First argument to lldb-server must be mode in which to run.
- debugserver_args.AppendArgument(llvm::StringRef("gdbserver"));
+ debugserver_args.AppendArgument("gdbserver");
#endif
- // If a url is supplied then use it
- if (url && url[0])
- debugserver_args.AppendArgument(llvm::StringRef(url));
-
- if (pass_comm_fd != SharedSocket::kInvalidFD) {
- StreamString fd_arg;
- fd_arg.Printf("--fd=%" PRIi64, (int64_t)pass_comm_fd);
- debugserver_args.AppendArgument(fd_arg.GetString());
- // Send "pass_comm_fd" down to the inferior so it can use it to
- // communicate back with this process. Ignored on Windows.
- launch_info.AppendDuplicateFileAction((int64_t)pass_comm_fd,
- (int64_t)pass_comm_fd);
- }
-
// use native registers, not the GDB registers
- debugserver_args.AppendArgument(llvm::StringRef("--native-regs"));
+ debugserver_args.AppendArgument("--native-regs");
if (launch_info.GetLaunchInSeparateProcessGroup())
- debugserver_args.AppendArgument(llvm::StringRef("--setsid"));
+ debugserver_args.AppendArgument("--setsid");
llvm::SmallString<128> named_pipe_path;
// socket_pipe is used by debug server to communicate back either
- // TCP port or domain socket name which it listens on.
- // The second purpose of the pipe to serve as a synchronization point -
+ // TCP port or domain socket name which it listens on. However, we're not
+ // interested in the actualy value here.
+ // The only reason for using the pipe is to serve as a synchronization point -
// once data is written to the pipe, debug server is up and running.
Pipe socket_pipe;
- std::unique_ptr<TCPSocket> sock_up;
+ // If a url is supplied then use it
+ if (shared_fd_t *comm_fd = std::get_if<shared_fd_t>(&comm)) {
+ LLDB_LOG(log, "debugserver communicates over fd {0}", comm_fd);
+ assert(*comm_fd != SharedSocket::kInvalidFD);
+ debugserver_args.AppendArgument(llvm::formatv("--fd={0}", *comm_fd).str());
+ // Send "comm_fd" down to the inferior so it can use it to communicate back
+ // with this process.
+ launch_info.AppendDuplicateFileAction((int64_t)*comm_fd, (int64_t)*comm_fd);
+ } else {
+ llvm::StringRef url = std::get<llvm::StringRef>(comm);
+ LLDB_LOG(log, "debugserver listens on: {0}", url);
+ debugserver_args.AppendArgument(url);
- // port is null when debug server should listen on domain socket - we're
- // not interested in port value but rather waiting for debug server to
- // become available.
- if (pass_comm_fd == SharedSocket::kInvalidFD) {
- if (url) {
-// Create a temporary file to get the stdout/stderr and redirect the output of
-// the command into this file. We will later read this file if all goes well
-// and fill the data into "command_output_ptr"
#if defined(__APPLE__)
- // Binding to port zero, we need to figure out what port it ends up
- // using using a named pipe...
- Status error = socket_pipe.CreateWithUniqueName("debugserver-named-pipe",
- false, named_pipe_path);
- if (error.Fail()) {
- LLDB_LOG(log, "named pipe creation failed: {0}", error);
- return error;
- }
- debugserver_args.AppendArgument(llvm::StringRef("--named-pipe"));
- debugserver_args.AppendArgument(named_pipe_path);
+ // Using a named pipe as debugserver does not support --pipe.
+ Status error = socket_pipe.CreateWithUniqueName("debugserver-named-pipe",
+ false, named_pipe_path);
+ if (error.Fail()) {
+ LLDB_LOG(log, "named pipe creation failed: {0}", error);
+ return error;
+ }
+ debugserver_args.AppendArgument(llvm::StringRef("--named-pipe"));
+ debugserver_args.AppendArgument(named_pipe_path);
#else
- // Binding to port zero, we need to figure out what port it ends up
- // using using an unnamed pipe...
- Status error = socket_pipe.CreateNew(true);
- if (error.Fail()) {
- LLDB_LOG(log, "unnamed pipe creation failed: {0}", error);
- return error;
- }
- pipe_t write = socket_pipe.GetWritePipe();
- debugserver_args.AppendArgument(llvm::StringRef("--pipe"));
- debugserver_args.AppendArgument(llvm::to_string(write));
- launch_info.AppendCloseFileAction(socket_pipe.GetReadFileDescriptor());
-#endif
- } else {
- // No host and port given, so lets listen on our end and make the
- // debugserver connect to us..
- if (llvm::Expected<std::unique_ptr<TCPSocket>> expected_sock =
- Socket::TcpListen("127.0.0.1:0"))
- sock_up = std::move(*expected_sock);
- else
- return Status::FromError(expected_sock.takeError());
-
- uint16_t port_ = sock_up->GetLocalPortNumber();
- // Send the host and port down that debugserver and specify an option
- // so that it connects back to the port we are listening to in this
- // process
- debugserver_args.AppendArgument(llvm::StringRef("--reverse-connect"));
- debugserver_args.AppendArgument(
- llvm::formatv("127.0.0.1:{0}", port_).str());
- if (port)
- *port = port_;
+ // Using an unnamed pipe as it's simpler.
+ Status error = socket_pipe.CreateNew(true);
+ if (error.Fail()) {
+ LLDB_LOG(log, "unnamed pipe creation failed: {0}", error);
+ return error;
}
+ pipe_t write = socket_pipe.GetWritePipe();
+ debugserver_args.AppendArgument(llvm::StringRef("--pipe"));
+ debugserver_args.AppendArgument(llvm::to_string(write));
+ launch_info.AppendCloseFileAction(socket_pipe.GetReadFileDescriptor());
+#endif
}
Environment host_env = Host::GetEnvironment();
@@ -1070,7 +1038,7 @@ Status GDBRemoteCommunication::StartDebugserverProcess(
return error;
}
- if (pass_comm_fd != SharedSocket::kInvalidFD)
+ if (std::holds_alternative<shared_fd_t>(comm))
return Status();
Status error;
@@ -1084,55 +1052,30 @@ Status GDBRemoteCommunication::StartDebugserverProcess(
if (socket_pipe.CanWrite())
socket_pipe.CloseWriteFileDescriptor();
- if (socket_pipe.CanRead()) {
- // Read port from pipe with 10 second timeout.
- std::string port_str;
- while (error.Success()) {
- char buf[10];
- if (llvm::Expected<size_t> num_bytes =
- socket_pipe.Read(buf, std::size(buf), std::chrono::seconds(10))) {
- if (*num_bytes == 0)
- break;
- port_str.append(buf, *num_bytes);
- } else {
- error = Status::FromError(num_bytes.takeError());
- }
- }
- if (error.Success() && (port != nullptr)) {
- // NB: Deliberately using .c_str() to stop at embedded '\0's
- llvm::StringRef port_ref = port_str.c_str();
- uint16_t child_port = 0;
- // FIXME: improve error handling
- llvm::to_integer(port_ref, child_port);
- if (*port == 0 || *port == child_port) {
- *port = child_port;
- LLDB_LOG(log, "debugserver listens on port {0}", *port);
- } else {
- LLDB_LOG(log,
- "debugserver listening on port {0} but requested port was {1}",
- child_port, (*port));
- }
+ assert(socket_pipe.CanRead());
+
+ // Read port from the pipe -- and ignore it (see comment above).
+ while (error.Success()) {
+ char buf[10];
+ if (llvm::Expected<size_t> num_bytes =
+ socket_pipe.Read(buf, std::size(buf), std::chrono::seconds(10))) {
+ if (*num_bytes == 0)
+ break;
} else {
- LLDB_LOG(log, "failed to read a port value from pipe {0}: {1}",
- named_pipe_path, error);
+ error = Status::FromError(num_bytes.takeError());
}
- socket_pipe.Close();
}
+ if (error.Fail()) {
+ LLDB_LOG(log, "failed to read a port value from pipe {0}: {1}",
+ named_pipe_path, error);
+ }
+ socket_pipe.Close();
if (named_pipe_path.size() > 0) {
if (Status err = socket_pipe.Delete(named_pipe_path); err.Fail())
LLDB_LOG(log, "failed to delete pipe {0}: {1}", named_pipe_path, err);
}
- if (error.Success() && sock_up) {
- Socket *accepted_socket = nullptr;
- error = sock_up->Accept(/*timeout=*/std::nullopt, accepted_socket);
- if (accepted_socket) {
- SetConnection(
- std::make_unique<ConnectionFileDescriptor>(accepted_socket));
- }
- }
-
return error;
}
@@ -1141,34 +1084,14 @@ void GDBRemoteCommunication::DumpHistory(Stream &strm) { m_history.Dump(strm); }
llvm::Error
GDBRemoteCommunication::ConnectLocally(GDBRemoteCommunication &client,
GDBRemoteCommunication &server) {
- const int backlog = 5;
- TCPSocket listen_socket(true);
- if (llvm::Error error =
- listen_socket.Listen("localhost:0", backlog).ToError())
- return error;
-
- llvm::SmallString<32> remote_addr;
- llvm::raw_svector_ostream(remote_addr)
- << "connect://localhost:" << listen_socket.GetLocalPortNumber();
-
- std::unique_ptr<ConnectionFileDescriptor> conn_up(
- new ConnectionFileDescriptor());
- Status status;
- if (conn_up->Connect(remote_addr, &status) != lldb::eConnectionStatusSuccess)
- return llvm::createStringError(llvm::inconvertibleErrorCode(),
- "Unable to connect: %s", status.AsCString());
-
- // The connection was already established above, so a short timeout is
- // sufficient.
- Socket *accept_socket = nullptr;
- if (Status accept_status =
- listen_socket.Accept(std::chrono::seconds(1), accept_socket);
- accept_status.Fail())
- return accept_status.takeError();
-
- client.SetConnection(std::move(conn_up));
- server.SetConnection(
- std::make_unique<ConnectionFileDescriptor>(accept_socket));
+ auto expected_socket_pair = Socket::CreatePair();
+ if (!expected_socket_pair)
+ return expected_socket_pair.takeError();
+
+ client.SetConnection(std::make_unique<ConnectionFileDescriptor>(
+ expected_socket_pair->first.release()));
+ server.SetConnection(std::make_unique<ConnectionFileDescriptor>(
+ expected_socket_pair->second.release()));
return llvm::Error::success();
}
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
index ee87629d9077b..4ee18412be6b6 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
@@ -135,17 +135,15 @@ class GDBRemoteCommunication : public Communication {
std::chrono::seconds GetPacketTimeout() const { return m_packet_timeout; }
// Get the debugserver path and check that it exist.
- FileSpec GetDebugserverPath(Platform *platform);
+ static FileSpec GetDebugserverPath(Platform *platform);
// Start a debugserver instance on the current host using the
// supplied connection URL.
- Status StartDebugserverProcess(
- const char *url,
+ static Status StartDebugserverProcess(
+ std::variant<llvm::StringRef, shared_fd_t> comm,
Platform *platform, // If non nullptr, then check with the platform for
// the GDB server binary if it can't be located
- ProcessLaunchInfo &launch_info, uint16_t *port, const Args *inferior_args,
- shared_fd_t pass_comm_fd); // Communication file descriptor to pass during
- // fork/exec to avoid having to connect/accept
+ ProcessLaunchInfo &launch_info, const Args *inferior_args);
void DumpHistory(Stream &strm);
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
index 89fdfa74bc025..7506cf64def38 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
@@ -94,7 +94,16 @@ GDBRemoteCommunicationServerPlatform::~GDBRemoteCommunicationServerPlatform() =
Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
const lldb_private::Args &args, lldb::pid_t &pid, std::string &socket_name,
shared_fd_t fd) {
- std::ostringstream url;
+ Log *log = GetLog(LLDBLog::Platform);
+
+ ProcessLaunchInfo debugserver_launch_info;
+ // Do not run in a new session so that it can not linger after the platform
+ // closes.
+ debugserver_launch_info.SetLaunchInSeparateProcessGroup(false);
+ debugserver_launch_info.SetMonitorProcessCallback(
+ [](lldb::pid_t, int, int) {});
+
+ Status error;
if (fd == SharedSocket::kInvalidFD) {
if (m_socket_protocol == Socket::ProtocolTcp) {
// Just check that GDBServer exists. GDBServer must be launched after
@@ -104,31 +113,22 @@ Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
return Status();
}
+ std::ostringstream url;
// debugserver does not accept the URL scheme prefix.
#if !defined(__APPLE__)
url << Socket::FindSchemeByProtocol(m_socket_protocol) << "://";
...
[truncated]
|
slydiman
reviewed
Jun 21, 2025
| } | ||
| assert(socket_pipe.CanRead()); | ||
|
|
||
| // Read port from the pipe -- and ignore it (see comment above). |
There was a problem hiding this comment.
Actually it is not always a port. It is a socket_id which equals the name in case of the named socket. I think it is necessary to update all comments about it.
slydiman
approved these changes
Jun 21, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The function was extremely messy in that it, depending on the set of
arguments, it could either modify the Connection object in
thisornot. It had a lot of arguments, with each call site passing a different
combination of null values. This PR:
mutually exclusive
staticto make it clear it (now) does notoperate on the
thisobject.Depends on #145017
(This PR consists of three commits, the first two of which are equivalent to #145015 and #145017, respectively. For reviewing, I recommend only looking at the last commit. If you have comments on the first two, please put them on their respective PRs.)