[Lldb-commits] [PATCH] D13754: Split Socket class into Tcp/Udp/DomainSocket subclasses.

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Thu Oct 15 01:36:49 PDT 2015

labath accepted this revision.
labath added a comment.

Looks good, with some random comments you can ignore.

Comment at: include/lldb/Host/Socket.h:55
@@ -55,1 +54,3 @@
+    virtual Error Connect(llvm::StringRef host_and_port);
+    virtual Error Listen(llvm::StringRef host_and_port, int backlog);
I would rename these to something more neutral like `name`, `addr`, etc. as this will not be a host+port for the domain socket implementation.

Comment at: source/Host/common/Socket.cpp:357
@@ -740,3 +356,3 @@
-uint16_t Socket::GetLocalPortNumber(const NativeSocket& socket)
+Error Socket::Connect (llvm::StringRef host_and_port)
Should we make this and Accept() abstract, as all implementations override it anyway?

Comment at: source/Host/common/TcpSocket.cpp:254
@@ +253,3 @@
+            accept_connection = true;
+            // Since both sockets have the same descriptor, arbitrarily choose the send
+            // socket to be the owner.
This comment sounds obsolete. We should probably delete it.

Comment at: source/Host/posix/DomainSocket.cpp:73
@@ +72,3 @@
+    FileSystem::Unlink(FileSpec{name, true});
Not really in scope of this patch, but I must say I find blindly deleting a file like this dangerous.


More information about the lldb-commits mailing list