Skip to content

Commit e50f02b

Browse files
mgornymgorny
committed
[lldb] [Host/ConnectionFileDescriptor] Refactor to improve code reuse
Refactor ConnectionFileDescriptor to improve code reuse for different types of sockets. Unify method naming. While at it, remove some (now-)dead code from Socket. Differential Revision: https://reviews.llvm.org/D112495
1 parent c45045b commit e50f02b

File tree

4 files changed

+92
-184
lines changed

4 files changed

+92
-184
lines changed

‎lldb/include/lldb/Host/Socket.h

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -84,18 +84,6 @@ class Socket : public IOObject {
8484
static llvm::Expected<std::unique_ptr<UDPSocket>>
8585
UdpConnect(llvm::StringRef host_and_port, bool child_processes_inherit);
8686

87-
static Status UnixDomainConnect(llvm::StringRef host_and_port,
88-
bool child_processes_inherit,
89-
Socket *&socket);
90-
static Status UnixDomainAccept(llvm::StringRef host_and_port,
91-
bool child_processes_inherit, Socket *&socket);
92-
static Status UnixAbstractConnect(llvm::StringRef host_and_port,
93-
bool child_processes_inherit,
94-
Socket *&socket);
95-
static Status UnixAbstractAccept(llvm::StringRef host_and_port,
96-
bool child_processes_inherit,
97-
Socket *&socket);
98-
9987
int GetOption(int level, int option_name, int &option_value);
10088
int SetOption(int level, int option_name, int option_value);
10189

‎lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include "lldb/lldb-forward.h"
1717

1818
#include "lldb/Host/Pipe.h"
19+
#include "lldb/Host/Socket.h"
1920
#include "lldb/Utility/Connection.h"
2021
#include "lldb/Utility/IOObject.h"
2122

@@ -73,9 +74,18 @@ class ConnectionFileDescriptor : public Connection {
7374
void CloseCommandPipe();
7475

7576
lldb::ConnectionStatus
76-
SocketListenAndAccept(llvm::StringRef host_and_port,
77-
socket_id_callback_type socket_id_callback,
78-
Status *error_ptr);
77+
AcceptSocket(Socket::SocketProtocol socket_protocol,
78+
llvm::StringRef socket_name,
79+
llvm::function_ref<void(Socket &)> post_listen_callback,
80+
Status *error_ptr);
81+
82+
lldb::ConnectionStatus ConnectSocket(Socket::SocketProtocol socket_protocol,
83+
llvm::StringRef socket_name,
84+
Status *error_ptr);
85+
86+
lldb::ConnectionStatus AcceptTCP(llvm::StringRef host_and_port,
87+
socket_id_callback_type socket_id_callback,
88+
Status *error_ptr);
7989

8090
lldb::ConnectionStatus ConnectTCP(llvm::StringRef host_and_port,
8191
socket_id_callback_type socket_id_callback,
@@ -86,24 +96,24 @@ class ConnectionFileDescriptor : public Connection {
8696
Status *error_ptr);
8797

8898
lldb::ConnectionStatus
89-
NamedSocketConnect(llvm::StringRef socket_name,
99+
ConnectNamedSocket(llvm::StringRef socket_name,
90100
socket_id_callback_type socket_id_callback,
91101
Status *error_ptr);
92102

93103
lldb::ConnectionStatus
94-
NamedSocketAccept(llvm::StringRef socket_name,
104+
AcceptNamedSocket(llvm::StringRef socket_name,
95105
socket_id_callback_type socket_id_callback,
96106
Status *error_ptr);
97107

98108
lldb::ConnectionStatus
99-
UnixAbstractSocketAccept(llvm::StringRef socket_name,
100-
socket_id_callback_type socket_id_callback,
101-
Status *error_ptr);
109+
AcceptAbstractSocket(llvm::StringRef socket_name,
110+
socket_id_callback_type socket_id_callback,
111+
Status *error_ptr);
102112

103113
lldb::ConnectionStatus
104-
UnixAbstractSocketConnect(llvm::StringRef socket_name,
105-
socket_id_callback_type socket_id_callback,
106-
Status *error_ptr);
114+
ConnectAbstractSocket(llvm::StringRef socket_name,
115+
socket_id_callback_type socket_id_callback,
116+
Status *error_ptr);
107117

108118
lldb::ConnectionStatus ConnectFD(llvm::StringRef args,
109119
socket_id_callback_type socket_id_callback,

‎lldb/source/Host/common/Socket.cpp

Lines changed: 0 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -183,70 +183,6 @@ Socket::UdpConnect(llvm::StringRef host_and_port,
183183
return UDPSocket::Connect(host_and_port, child_processes_inherit);
184184
}
185185

186-
Status Socket::UnixDomainConnect(llvm::StringRef name,
187-
bool child_processes_inherit,
188-
Socket *&socket) {
189-
Status error;
190-
std::unique_ptr<Socket> connect_socket(
191-
Create(ProtocolUnixDomain, child_processes_inherit, error));
192-
if (error.Fail())
193-
return error;
194-
195-
error = connect_socket->Connect(name);
196-
if (error.Success())
197-
socket = connect_socket.release();
198-
199-
return error;
200-
}
201-
202-
Status Socket::UnixDomainAccept(llvm::StringRef name,
203-
bool child_processes_inherit, Socket *&socket) {
204-
Status error;
205-
std::unique_ptr<Socket> listen_socket(
206-
Create(ProtocolUnixDomain, child_processes_inherit, error));
207-
if (error.Fail())
208-
return error;
209-
210-
error = listen_socket->Listen(name, 5);
211-
if (error.Fail())
212-
return error;
213-
214-
error = listen_socket->Accept(socket);
215-
return error;
216-
}
217-
218-
Status Socket::UnixAbstractConnect(llvm::StringRef name,
219-
bool child_processes_inherit,
220-
Socket *&socket) {
221-
Status error;
222-
std::unique_ptr<Socket> connect_socket(
223-
Create(ProtocolUnixAbstract, child_processes_inherit, error));
224-
if (error.Fail())
225-
return error;
226-
227-
error = connect_socket->Connect(name);
228-
if (error.Success())
229-
socket = connect_socket.release();
230-
return error;
231-
}
232-
233-
Status Socket::UnixAbstractAccept(llvm::StringRef name,
234-
bool child_processes_inherit,
235-
Socket *&socket) {
236-
Status error;
237-
std::unique_ptr<Socket> listen_socket(
238-
Create(ProtocolUnixAbstract, child_processes_inherit, error));
239-
if (error.Fail())
240-
return error;
241-
242-
error = listen_socket->Listen(name, 5);
243-
if (error.Fail())
244-
return error;
245-
246-
error = listen_socket->Accept(socket);
247-
return error;
248-
}
249-
250186
llvm::Expected<Socket::HostAndPort> Socket::DecodeHostAndPort(llvm::StringRef host_and_port) {
251187
static llvm::Regex g_regex("([^:]+|\\[[0-9a-fA-F:]+.*\\]):([0-9]+)");
252188
HostAndPort ret;

‎lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp

Lines changed: 71 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -151,26 +151,29 @@ ConnectionFileDescriptor::Connect(llvm::StringRef path,
151151
auto method =
152152
llvm::StringSwitch<ConnectionStatus (ConnectionFileDescriptor::*)(
153153
llvm::StringRef, socket_id_callback_type, Status *)>(scheme)
154-
.Case("listen", &ConnectionFileDescriptor::SocketListenAndAccept)
154+
.Case("listen", &ConnectionFileDescriptor::AcceptTCP)
155155
.Cases("accept", "unix-accept",
156-
&ConnectionFileDescriptor::NamedSocketAccept)
156+
&ConnectionFileDescriptor::AcceptNamedSocket)
157157
.Case("unix-abstract-accept",
158-
&ConnectionFileDescriptor::UnixAbstractSocketAccept)
158+
&ConnectionFileDescriptor::AcceptAbstractSocket)
159159
.Cases("connect", "tcp-connect",
160160
&ConnectionFileDescriptor::ConnectTCP)
161161
.Case("udp", &ConnectionFileDescriptor::ConnectUDP)
162-
.Case("unix-connect", &ConnectionFileDescriptor::NamedSocketConnect)
162+
.Case("unix-connect", &ConnectionFileDescriptor::ConnectNamedSocket)
163163
.Case("unix-abstract-connect",
164-
&ConnectionFileDescriptor::UnixAbstractSocketConnect)
164+
&ConnectionFileDescriptor::ConnectAbstractSocket)
165165
#if LLDB_ENABLE_POSIX
166166
.Case("fd", &ConnectionFileDescriptor::ConnectFD)
167167
.Case("file", &ConnectionFileDescriptor::ConnectFile)
168168
.Case("serial", &ConnectionFileDescriptor::ConnectSerialPort)
169169
#endif
170170
.Default(nullptr);
171171

172-
if (method)
172+
if (method) {
173+
if (error_ptr)
174+
*error_ptr = Status();
173175
return (this->*method)(path, socket_id_callback, error_ptr);
176+
}
174177
}
175178

176179
if (error_ptr)
@@ -532,24 +535,25 @@ ConnectionFileDescriptor::BytesAvailable(const Timeout<std::micro> &timeout,
532535
return eConnectionStatusLostConnection;
533536
}
534537

535-
ConnectionStatus ConnectionFileDescriptor::NamedSocketAccept(
536-
llvm::StringRef socket_name, socket_id_callback_type socket_id_callback,
538+
lldb::ConnectionStatus ConnectionFileDescriptor::AcceptSocket(
539+
Socket::SocketProtocol socket_protocol, llvm::StringRef socket_name,
540+
llvm::function_ref<void(Socket &)> post_listen_callback,
537541
Status *error_ptr) {
538542
Status error;
539-
std::unique_ptr<Socket> listen_socket = Socket::Create(
540-
Socket::ProtocolUnixDomain, m_child_processes_inherit, error);
541-
Socket *socket = nullptr;
543+
std::unique_ptr<Socket> listening_socket =
544+
Socket::Create(socket_protocol, m_child_processes_inherit, error);
545+
Socket *accepted_socket;
542546

543547
if (!error.Fail())
544-
error = listen_socket->Listen(socket_name, 5);
548+
error = listening_socket->Listen(socket_name, 5);
545549

546550
if (!error.Fail()) {
547-
socket_id_callback(socket_name);
548-
error = listen_socket->Accept(socket);
551+
post_listen_callback(*listening_socket);
552+
error = listening_socket->Accept(accepted_socket);
549553
}
550554

551555
if (!error.Fail()) {
552-
m_io_sp.reset(socket);
556+
m_io_sp.reset(accepted_socket);
553557
m_uri.assign(socket_name.str());
554558
return eConnectionStatusSuccess;
555559
}
@@ -559,40 +563,19 @@ ConnectionStatus ConnectionFileDescriptor::NamedSocketAccept(
559563
return eConnectionStatusError;
560564
}
561565

562-
ConnectionStatus ConnectionFileDescriptor::NamedSocketConnect(
563-
llvm::StringRef socket_name, socket_id_callback_type socket_id_callback,
564-
Status *error_ptr) {
565-
Socket *socket = nullptr;
566-
Status error =
567-
Socket::UnixDomainConnect(socket_name, m_child_processes_inherit, socket);
568-
if (error_ptr)
569-
*error_ptr = error;
570-
m_io_sp.reset(socket);
571-
if (error.Fail())
572-
return eConnectionStatusError;
573-
m_uri.assign(std::string(socket_name));
574-
return eConnectionStatusSuccess;
575-
}
576-
577-
ConnectionStatus ConnectionFileDescriptor::UnixAbstractSocketAccept(
578-
llvm::StringRef socket_name, socket_id_callback_type socket_id_callback,
579-
Status *error_ptr) {
566+
lldb::ConnectionStatus
567+
ConnectionFileDescriptor::ConnectSocket(Socket::SocketProtocol socket_protocol,
568+
llvm::StringRef socket_name,
569+
Status *error_ptr) {
580570
Status error;
581-
std::unique_ptr<Socket> listen_socket = Socket::Create(
582-
Socket::ProtocolUnixAbstract, m_child_processes_inherit, error);
583-
Socket *socket = nullptr;
571+
std::unique_ptr<Socket> socket =
572+
Socket::Create(socket_protocol, m_child_processes_inherit, error);
584573

585574
if (!error.Fail())
586-
error = listen_socket->Listen(socket_name, 5);
587-
588-
if (!error.Fail())
589-
socket_id_callback(socket_name);
590-
591-
if (!error.Fail())
592-
error = listen_socket->Accept(socket);
575+
error = socket->Connect(socket_name);
593576

594577
if (!error.Fail()) {
595-
m_io_sp.reset(socket);
578+
m_io_sp = std::move(socket);
596579
m_uri.assign(socket_name.str());
597580
return eConnectionStatusSuccess;
598581
}
@@ -602,72 +585,63 @@ ConnectionStatus ConnectionFileDescriptor::UnixAbstractSocketAccept(
602585
return eConnectionStatusError;
603586
}
604587

605-
lldb::ConnectionStatus ConnectionFileDescriptor::UnixAbstractSocketConnect(
588+
ConnectionStatus ConnectionFileDescriptor::AcceptNamedSocket(
606589
llvm::StringRef socket_name, socket_id_callback_type socket_id_callback,
607590
Status *error_ptr) {
608-
Socket *socket = nullptr;
609-
Status error = Socket::UnixAbstractConnect(socket_name,
610-
m_child_processes_inherit, socket);
611-
if (error_ptr)
612-
*error_ptr = error;
613-
m_io_sp.reset(socket);
614-
if (error.Fail())
615-
return eConnectionStatusError;
616-
m_uri.assign(std::string(socket_name));
617-
return eConnectionStatusSuccess;
591+
return AcceptSocket(
592+
Socket::ProtocolUnixDomain, socket_name,
593+
[socket_id_callback, socket_name](Socket &listening_socket) {
594+
socket_id_callback(socket_name);
595+
},
596+
error_ptr);
618597
}
619598

620-
ConnectionStatus ConnectionFileDescriptor::SocketListenAndAccept(
621-
llvm::StringRef s, socket_id_callback_type socket_id_callback,
599+
ConnectionStatus ConnectionFileDescriptor::ConnectNamedSocket(
600+
llvm::StringRef socket_name, socket_id_callback_type socket_id_callback,
622601
Status *error_ptr) {
623-
if (error_ptr)
624-
*error_ptr = Status();
625-
626-
llvm::Expected<std::unique_ptr<TCPSocket>> listening_socket =
627-
Socket::TcpListen(s, m_child_processes_inherit);
628-
if (!listening_socket) {
629-
if (error_ptr)
630-
*error_ptr = listening_socket.takeError();
631-
else
632-
LLDB_LOG_ERROR(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION),
633-
listening_socket.takeError(), "tcp listen failed: {0}");
634-
return eConnectionStatusError;
635-
}
602+
return ConnectSocket(Socket::ProtocolUnixDomain, socket_name, error_ptr);
603+
}
636604

637-
uint16_t port = listening_socket.get()->GetLocalPortNumber();
638-
socket_id_callback(std::to_string(port));
605+
ConnectionStatus ConnectionFileDescriptor::AcceptAbstractSocket(
606+
llvm::StringRef socket_name, socket_id_callback_type socket_id_callback,
607+
Status *error_ptr) {
608+
return AcceptSocket(
609+
Socket::ProtocolUnixAbstract, socket_name,
610+
[socket_id_callback, socket_name](Socket &listening_socket) {
611+
socket_id_callback(socket_name);
612+
},
613+
error_ptr);
614+
}
639615

640-
Socket *accepted_socket;
641-
Status error = listening_socket.get()->Accept(accepted_socket);
642-
if (error_ptr)
643-
*error_ptr = error;
644-
if (error.Fail())
645-
return eConnectionStatusError;
616+
lldb::ConnectionStatus ConnectionFileDescriptor::ConnectAbstractSocket(
617+
llvm::StringRef socket_name, socket_id_callback_type socket_id_callback,
618+
Status *error_ptr) {
619+
return ConnectSocket(Socket::ProtocolUnixAbstract, socket_name, error_ptr);
620+
}
646621

647-
InitializeSocket(accepted_socket);
648-
return eConnectionStatusSuccess;
622+
ConnectionStatus
623+
ConnectionFileDescriptor::AcceptTCP(llvm::StringRef socket_name,
624+
socket_id_callback_type socket_id_callback,
625+
Status *error_ptr) {
626+
ConnectionStatus ret = AcceptSocket(
627+
Socket::ProtocolTcp, socket_name,
628+
[socket_id_callback](Socket &listening_socket) {
629+
uint16_t port =
630+
static_cast<TCPSocket &>(listening_socket).GetLocalPortNumber();
631+
socket_id_callback(std::to_string(port));
632+
},
633+
error_ptr);
634+
if (ret == eConnectionStatusSuccess)
635+
m_uri.assign(
636+
static_cast<TCPSocket *>(m_io_sp.get())->GetRemoteConnectionURI());
637+
return ret;
649638
}
650639

651640
ConnectionStatus
652-
ConnectionFileDescriptor::ConnectTCP(llvm::StringRef s,
641+
ConnectionFileDescriptor::ConnectTCP(llvm::StringRef socket_name,
653642
socket_id_callback_type socket_id_callback,
654643
Status *error_ptr) {
655-
if (error_ptr)
656-
*error_ptr = Status();
657-
658-
llvm::Expected<std::unique_ptr<Socket>> socket =
659-
Socket::TcpConnect(s, m_child_processes_inherit);
660-
if (!socket) {
661-
if (error_ptr)
662-
*error_ptr = socket.takeError();
663-
else
664-
LLDB_LOG_ERROR(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION),
665-
socket.takeError(), "tcp connect failed: {0}");
666-
return eConnectionStatusError;
667-
}
668-
m_io_sp = std::move(*socket);
669-
m_uri.assign(std::string(s));
670-
return eConnectionStatusSuccess;
644+
return ConnectSocket(Socket::ProtocolTcp, socket_name, error_ptr);
671645
}
672646

673647
ConnectionStatus

0 commit comments

Comments
 (0)