[LLDB][NFC] Reliability fixes to TCPSocket code

[fixathon]

Summary

Patch the following issues found by static code inspection:

• Unchecked return values from lib calls
• Passing potentially negative arg into a function that requires non-negative input
• Possible socket double-close

[hawkinsw]

Not that my opinion means much, but the changes look good to me.

[jasonmolenda]

LGTM.

[jingham]

We don't use the constant first so you don't accidentally use "=" rather than "==" convention anywhere else in lldb, so it looks weird here.

[fixathon]

In D131294#3707154, @jingham wrote:

We don't use the constant first so you don't accidentally use "=" rather than "==" convention anywhere else in lldb, so it looks weird here.

Good feedback. I'll switch that around.

On second thought, this very file has numerous instances of "constant first" style, ex: (-1 == expression), so this is consistent with the current style of this file. Would you still like the style adjusted?

[fixathon]

Commented on feedback.

lldb/source/Host/common/TCPSocket.cpp

for (SocketAddress &address : addresses) {
 
     if (-1 == llvm::sys::RetryAfterSignal(-1, ::connect, GetNativeSocket(),
                                           &address.sockaddr(),
                                           address.GetLength())) {
       Close();
       continue;
     }
 
-    SetOptionNoDelay();
+    if (-1 == SetOptionNoDelay()) {
+      Close();
+      continue;
+    }
 
     error.Clear();
     return error;
   }
 
   error.SetErrorString("Failed to connect port");
   return error;
 }
Status TCPSocket::Listen(llvm::StringRef name, int backlog) {
 
   if (host_port->hostname == "*")
     host_port->hostname = "0.0.0.0";
   std::vector<SocketAddress> addresses = SocketAddress::GetAddressInfo(
       host_port->hostname.c_str(), nullptr, AF_UNSPEC, SOCK_STREAM, IPPROTO_TCP);
   for (SocketAddress &address : addresses) {
     int fd = Socket::CreateSocket(address.GetFamily(), kType, IPPROTO_TCP,
                                   m_child_processes_inherit, error);
-    if (error.Fail())
+    if (error.Fail() || fd < 0)
       continue;
 
     // enable local address reuse
     int option_value = 1;
     set_socket_option_arg_type option_value_p =
         reinterpret_cast<set_socket_option_arg_type>(&option_value);
-    ::setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, option_value_p,
-                 sizeof(option_value));
+    if (-1 == ::setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, option_value_p,
+                           sizeof(option_value))) {
+      CLOSE_SOCKET(fd);
+      continue;
+    }
 
     SocketAddress listen_address = address;
     if(!listen_address.IsLocalhost())
       listen_address.SetToAnyAddress(address.GetFamily(), host_port->port);
     else
       listen_address.SetPort(host_port->port);
 
     int err =

 
 Status TCPSocket::Accept(Socket *&conn_socket) {
   Status error;
   if (m_listen_sockets.size() == 0) {
     error.SetErrorString("No open listening sockets!");
     return error;
   }
 
-  int sock = -1;
-  int listen_sock = -1;
+  int sock = kInvalidSocketValue;
+  int listen_sock = kInvalidSocketValue;
   lldb_private::SocketAddress AcceptAddr;
   MainLoop accept_loop;
   std::vector<MainLoopBase::ReadHandleUP> handles;
   for (auto socket : m_listen_sockets) {
     auto fd = socket.first;
     auto inherit = this->m_child_processes_inherit;
     auto io_sp = IOObjectSP(new TCPSocket(socket.first, false, inherit));
     handles.emplace_back(accept_loop.RegisterReadObject(
Status TCPSocket::Accept(Socket *&conn_socket) {
   while (!accept_connection) {
     accept_loop.Run();
 
     if (error.Fail())
         return error;
 
     lldb_private::SocketAddress &AddrIn = m_listen_sockets[listen_sock];
     if (!AddrIn.IsAnyAddr() && AcceptAddr != AddrIn) {
+      if (kInvalidSocketValue != sock) {
         CLOSE_SOCKET(sock);
+        sock = kInvalidSocketValue;
+      }
       llvm::errs() << llvm::formatv(
           "error: rejecting incoming connection from {0} (expecting {1})",
           AcceptAddr.GetIPAddress(), AddrIn.GetIPAddress());
       continue;
     }
     accept_connection = true;
     accepted_socket.reset(new TCPSocket(sock, *this));
   }