extract and use common socketConnect method and apply timeout on device connect (potential fix for #594)

src/lib/ebus/device.cpp

@@ -24,16 +24,13 @@
 #include <fcntl.h>
 #include <sys/ioctl.h>
 #include <sys/file.h>
-#include <sys/socket.h>
 #include <netinet/in.h>
-#include <netinet/tcp.h>
 #ifdef HAVE_LINUX_SERIAL
 #  include <linux/serial.h>
 #endif
 #ifdef HAVE_FREEBSD_UFTDI
 #  include <dev/usb/uftdiio.h>
 #endif
-#include <errno.h>
 #ifdef HAVE_PPOLL
 #  include <poll.h>
 #endif
@@ -44,6 +41,7 @@
 #include <ios>
 #include <iomanip>
 #include "lib/ebus/data.h"
+#include "lib/utils/tcpsocket.h"
 
 namespace ebusd {
 
@@ -874,46 +872,10 @@ result_t NetworkDevice::open() {
   if (result != RESULT_OK) {
     return result;
   }
-  struct sockaddr_in address;
+  m_fd = socketConnect(m_hostOrIp, m_port, m_udp, nullptr, 5, 2);  // wait up to 5 seconds for established connection
-  memset(reinterpret_cast<char*>(&address), 0, sizeof(address));
-  if (inet_aton(m_hostOrIp, &address.sin_addr) == 0) {
-    struct hostent* h = gethostbyname(m_hostOrIp);
-    if (h == nullptr) {
-      return RESULT_ERR_GENERIC_IO;  // invalid host
-    }
-    memcpy(&address.sin_addr, h->h_addr_list[0], h->h_length);
-  }
-  address.sin_family = AF_INET;
-  address.sin_port = (in_port_t)htons(m_port);
-
-  m_fd = socket(AF_INET, m_udp ? SOCK_DGRAM : SOCK_STREAM, 0);
   if (m_fd < 0) {
     return RESULT_ERR_GENERIC_IO;
   }
-  int ret;
-  if (m_udp) {
-    struct sockaddr_in bindAddress = address;
-    bindAddress.sin_addr.s_addr = INADDR_ANY;
-    ret = bind(m_fd, (struct sockaddr*)&bindAddress, sizeof(address));
-  } else {
-    int value = 1;
-    ret = setsockopt(m_fd, IPPROTO_TCP, TCP_NODELAY, reinterpret_cast<void*>(&value), sizeof(value));
-    value = 1;
-    setsockopt(m_fd, SOL_SOCKET, SO_KEEPALIVE, reinterpret_cast<void*>(&value), sizeof(value));
-    value = 3;  // send keepalive after 3 seconds of silence
-    setsockopt(m_fd, IPPROTO_TCP, TCP_KEEPIDLE, reinterpret_cast<void*>(&value), sizeof(value));
-    value = 2;  // send keepalive in interval of 2 seconds
-    setsockopt(m_fd, IPPROTO_TCP, TCP_KEEPINTVL, reinterpret_cast<void*>(&value), sizeof(value));
-    value = 2;  // drop connection after 2 failed keep alive sends
-    setsockopt(m_fd, IPPROTO_TCP, TCP_KEEPCNT, reinterpret_cast<void*>(&value), sizeof(value));
-  }
-  if (ret >= 0) {
-    ret = connect(m_fd, (struct sockaddr*)&address, sizeof(address));
-  }
-  if (ret < 0) {
-    close();
-    return RESULT_ERR_GENERIC_IO;
-  }
   if (!m_udp) {
     usleep(25000);  // wait 25ms for potential initial garbage
   }

src/lib/utils/tcpsocket.cpp

@@ -24,12 +24,14 @@
 #include <fcntl.h>
 #include <arpa/inet.h>
 #include <netdb.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <netinet/tcp.h>
 #include <string.h>
 #include <errno.h>
 #ifdef HAVE_PPOLL
 #  include <poll.h>
 #endif
-#include <cstdlib>
 
 namespace ebusd {
 
@@ -45,52 +47,73 @@ bool TCPSocket::isValid() {
 }
 
 
-TCPSocket* TCPSocket::connect(const string& server, const uint16_t& port, int timeout) {
+int socketConnect(const char* server, uint16_t port, bool udp, socketaddress* storeAddress, int tcpConnectTimeout,
-  socketaddress address;
+int tcpKeepAliveInterval) {
-  int ret;
+  socketaddress localAddress;
+  socketaddress* address = storeAddress ? storeAddress : &localAddress;
+  memset(reinterpret_cast<char*>(address), 0, sizeof(*address));
 
-  memset(reinterpret_cast<char*>(&address), 0, sizeof(address));
+  if (inet_aton(server, &address->sin_addr) == 0) {
-
+    struct hostent* he = gethostbyname(server);
-  if (inet_addr(server.c_str()) == INADDR_NONE) {
-    struct hostent* he;
-
-    he = gethostbyname(server.c_str());
     if (he == nullptr) {
-      return nullptr;
+      return -1;
     }
-    memcpy(&address.sin_addr, he->h_addr_list[0], he->h_length);
+    memcpy(&address->sin_addr, he->h_addr_list[0], he->h_length);
-  } else {
+  }
-    ret = inet_aton(server.c_str(), &address.sin_addr);
+  address->sin_family = AF_INET;
-    if (ret == 0) {
+  address->sin_port = (in_port_t)htons(port);
-      return nullptr;
+
+  int sfd = socket(AF_INET, udp ? SOCK_DGRAM : SOCK_STREAM, 0);
+  if (sfd < 0) {
+    return -1;
+  }
+  int ret;
+  if (udp) {
+    struct sockaddr_in bindAddress = *address;
+    bindAddress.sin_addr.s_addr = INADDR_ANY;
+    ret = bind(sfd, (struct sockaddr*)&bindAddress, sizeof(bindAddress));
+    if (ret >= 0) {
+      ret = ::connect(sfd, (struct sockaddr*)address, sizeof(*address));
+    }
+    if (ret < 0) {
+      close(sfd);
+      return -1;
     }
   }
-
+  int value = 1;
-  address.sin_family = AF_INET;
+  ret = setsockopt(sfd, IPPROTO_TCP, TCP_NODELAY, reinterpret_cast<void*>(&value), sizeof(value));
-  address.sin_port = (in_port_t)htons(port);
+  if (ret < 0) {
-
+    close(sfd);
-  int sfd = socket(AF_INET, SOCK_STREAM, 0);
+    return -1;
-  if (sfd < 0) {
+  }
-    return nullptr;
+  if (tcpKeepAliveInterval > 0) {
+    value = 1;
+    setsockopt(sfd, SOL_SOCKET, SO_KEEPALIVE, reinterpret_cast<void*>(&value), sizeof(value));
+    value = tcpKeepAliveInterval+1;  // send keepalive after interval + 1 seconds of silence
+    setsockopt(sfd, IPPROTO_TCP, TCP_KEEPIDLE, reinterpret_cast<void*>(&value), sizeof(value));
+    value = tcpKeepAliveInterval;  // send keepalive in given interval
+    setsockopt(sfd, IPPROTO_TCP, TCP_KEEPINTVL, reinterpret_cast<void*>(&value), sizeof(value));
+    value = 2;  // drop connection after 2 failed keep alive sends
+    setsockopt(sfd, IPPROTO_TCP, TCP_KEEPCNT, reinterpret_cast<void*>(&value), sizeof(value));
   }
 #ifndef HAVE_PPOLL
 #ifndef HAVE_PSELECT
   timeout = 0;
 #endif
 #endif
-  if (timeout > 0 && fcntl(sfd, F_SETFL, O_NONBLOCK) < 0) {  // set non-blocking
+  if (tcpConnectTimeout > 0 && fcntl(sfd, F_SETFL, O_NONBLOCK) < 0) {  // set non-blocking
     close(sfd);
-    return nullptr;
+    return -1;
   }
-  ret = ::connect(sfd, (struct sockaddr *) &address, sizeof(address));
+  ret = ::connect(sfd, (struct sockaddr*)address, sizeof(*address));
   if (ret != 0) {
-    if (ret < 0 && (timeout <= 0 || errno != EINPROGRESS)) {
+    if (ret < 0 && (tcpConnectTimeout <= 0 || errno != EINPROGRESS)) {
       close(sfd);
-      return nullptr;
+      return -1;
     }
-    if (timeout > 0) {
+    if (tcpConnectTimeout > 0) {
       struct timespec tdiff;
-      tdiff.tv_sec = timeout;
+      tdiff.tv_sec = tcpConnectTimeout;
       tdiff.tv_nsec = 0;
 #ifdef HAVE_PPOLL
       nfds_t nfds = 1;
@@ -115,12 +138,22 @@ TCPSocket* TCPSocket::connect(const string& server, const uint16_t& port, int ti
 #endif
       if (ret == -1 || ret == 0) {
         close(sfd);
-        return nullptr;
+        return -1;
+      }
+      if (fcntl(sfd, F_SETFL, 0) < 0) {  // set blocking again
+        close(sfd);
+        return -1;
       }
     }
   }
-  if (timeout > 0 && fcntl(sfd, F_SETFL, 0) < 0) {  // set blocking again
+  return sfd;
-    close(sfd);
+}
+
+
+TCPSocket* TCPSocket::connect(const string& server, const uint16_t& port, int timeout) {
+  socketaddress address;
+  int sfd = socketConnect(server.c_str(), port, false, &address, timeout);
+  if (sfd < 0) {
     return nullptr;
   }
   TCPSocket* s = new TCPSocket(sfd, &address);
@@ -137,21 +170,18 @@ int TCPServer::start() {
   }
   m_lfd = socket(AF_INET, SOCK_STREAM, 0);
   socketaddress address;
-
   memset(&address, 0, sizeof(address));
 
   address.sin_family = AF_INET;
   address.sin_port = (in_port_t)htons(m_port);
 
-  if (m_address.size() > 0) {
+  if (!m_address.empty() && inet_pton(AF_INET, m_address.c_str(), &address.sin_addr) != 1) {
-    inet_pton(AF_INET, m_address.c_str(), &(address.sin_addr));
-  } else {
     address.sin_addr.s_addr = INADDR_ANY;
   }
-  int optval = 1;
+  int value = 1;
-  setsockopt(m_lfd, SOL_SOCKET, SO_REUSEADDR, &optval, sizeof(optval));
+  setsockopt(m_lfd, SOL_SOCKET, SO_REUSEADDR, &value, sizeof(value));
 
-  int result = bind(m_lfd, (struct sockaddr*) &address, sizeof(address));
+  int result = bind(m_lfd, (struct sockaddr*)&address, sizeof(address));
   if (result != 0) {
     return result;
   }
@@ -169,10 +199,9 @@ TCPSocket* TCPServer::newSocket() {
   }
   socketaddress address;
   socklen_t len = sizeof(address);
-
   memset(&address, 0, sizeof(address));
 
-  int sfd = accept(m_lfd, (struct sockaddr*) &address, &len);
+  int sfd = accept(m_lfd, (struct sockaddr*)&address, &len);
   if (sfd < 0) {
     return nullptr;
   }

src/lib/utils/tcpsocket.h

@@ -41,6 +41,20 @@ using std::string;
 #endif
 #endif
 
+/**
+ * Connect a TCP or UDP socket.
+ * @param server the server name or ip address to connect to.
+ * @param port the port number.
+ * @param udp true for UDP, false for TCP.
+ * @param storeAddress optional pointer to where the socket address will be stored.
+ * @param tcpConnectTimeout the TCP connect timeout in seconds, or 0.
+ * @param tcpKeepAliveInterval optional interval in seconds for sending TCP keepalive.
+ * @return the connected socket file descriptor on success, or -1 on error.
+ */
+int socketConnect(const char* server, uint16_t port, bool udp, socketaddress* storeAddress = nullptr,
+int tcpConnectTimeout = 0, int tcpKeepAliveInterval = 0);
+
+
 /**
  * Class for low level TCP socket operations (open, close, send, receive).
  */

src/tools/CMakeLists.txt

@@ -8,7 +8,7 @@ include_directories(intelhex)
 add_executable(ebusctl ${ebusctl_SOURCES})
 add_executable(ebuspicloader ${ebuspicloader_SOURCES})
 target_link_libraries(ebusctl utils ebus ${LIB_ARGP} ${ebusctl_LIBS})
-target_link_libraries(ebuspicloader ${LIB_ARGP})
+target_link_libraries(ebuspicloader utils ${LIB_ARGP} ${ebuspicloader_LIBS})
 
 if(WITH_EBUSFEED)
   set(ebusfeed_SOURCES ebusfeed.cpp)

src/tools/Makefile.am

@@ -8,6 +8,7 @@ ebusctl_SOURCES = ebusctl.cpp
 ebusctl_LDADD = ../lib/utils/libutils.a
 
 ebuspicloader_SOURCES = ebuspicloader.cpp intelhex/intelhexclass.cpp
+ebusctl_LDADD = ../lib/utils/libutils.a
 
 if WITH_EBUSFEED
 bin_PROGRAMS += ebusfeed

src/tools/ebuspicloader.cpp

@@ -37,7 +37,9 @@
 #include <string>
 #include <cstring>
 #include "intelhex/intelhexclass.h"
+#include "lib/utils/tcpsocket.h"
 
+using ebusd::socketConnect;
 
 /** the version string of the program. */
 const char *argp_program_version = "eBUS adapter PIC firmware loader";
@@ -661,32 +663,11 @@ int openSerial(std::string port) {
 
 int openNet(std::string host, uint16_t port) {
   // open network port
-  struct sockaddr_in address;
+  int fd = socketConnect(host.c_str(), port, false, nullptr, 5);
-  memset(reinterpret_cast<char*>(&address), 0, sizeof(address));
-  if (inet_addr(host.c_str()) == INADDR_NONE) {
-    struct hostent* he;
-    he = gethostbyname(host.c_str());
-    if (he == nullptr) {
-      std::cerr << "unable to resolve host " << host << std::endl;
-      return -1;
-    }
-    memcpy(&address.sin_addr, he->h_addr_list[0], he->h_length);
-  } else if (inet_aton(host.c_str(), &address.sin_addr) == 0) {
-    std::cerr << "unable to resolve IP " << host << std::endl;
-    return -1;
-  }
-  address.sin_family = AF_INET;
-  address.sin_port = (in_port_t)htons(port);
-  int fd = socket(AF_INET, SOCK_STREAM, 0);
   if (fd < 0) {
     std::cerr << "unable to open " << host << std::endl;
     return -1;
   }
-  if (connect(fd, (struct sockaddr *) &address, sizeof(address)) != 0) {
-    close(fd);
-    std::cerr << "unable to connect to " << host << std::endl;
-    return -1;
-  }
   fcntl(fd, F_SETFL, O_NONBLOCK);  // set non-blocking
   return fd;
 }