refactor Input/Output_Timeout: don't duplicate the same code in MSW/Unix code and also don't duplicate it for input and output, one function is enough for both

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@57558 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775

include/wx/msw/private/sockmsw.h

@@ -54,8 +54,6 @@ private:
         wxSocketManager::Get()->Install_Callback(this);
     }
 
-    wxSocketError Input_Timeout();
-    wxSocketError Output_Timeout();
     wxSocketError Connect_Timeout();
     int Recv_Stream(void *buffer, int size);
     int Recv_Dgram(void *buffer, int size);

include/wx/private/socket.h

@@ -297,6 +297,15 @@ public:
 protected:
     wxSocketImpl(wxSocketBase& wxsocket);
 
+    // wait until input/output becomes available or m_timeout expires
+    //
+    // returns true if we do have input/output or false on timeout or error
+    // (also sets m_error accordingly)
+    bool BlockForInputWithTimeout()
+        { return DoBlockWithTimeout(wxSOCKET_INPUT_FLAG); }
+    bool BlockForOutputWithTimeout()
+        { return DoBlockWithTimeout(wxSOCKET_OUTPUT_FLAG); }
+
 private:
     // handle the given connect() return value (which may be 0 or EWOULDBLOCK
     // or something else)
@@ -338,6 +347,12 @@ private:
     // update local address after binding/connecting
     wxSocketError UpdateLocalAddress();
 
+    // wait for IO on the socket or until timeout expires
+    //
+    // the parameter can be one of wxSOCKET_INPUT/OUTPUT_FLAG (but could be
+    // their combination in the future, hence we take wxSocketEventFlags)
+    bool DoBlockWithTimeout(wxSocketEventFlags flags);
+
 
     // set in ctor and never changed except that it's reset to NULL when the
     // socket is shut down

include/wx/unix/private/sockunix.h

@@ -100,9 +100,6 @@ private:
     void EnableEvent(wxSocketNotify event);
     void DisableEvent(wxSocketNotify event);
 
-
-    wxSocketError Input_Timeout();
-    wxSocketError Output_Timeout();
     int Recv_Stream(void *buffer, int size);
     int Recv_Dgram(void *buffer, int size);
     int Send_Stream(const void *buffer, int size);

src/common/socket.cpp

@@ -503,6 +503,38 @@ GAddress *wxSocketImpl::GetPeer()
   return NULL;
 }
 
+bool wxSocketImpl::DoBlockWithTimeout(wxSocketEventFlags flags)
+{
+    if ( !m_non_blocking )
+    {
+        fd_set fds;
+        wxFD_ZERO(&fds);
+        wxFD_SET(m_fd, &fds);
+
+        fd_set
+            *readfds = flags & wxSOCKET_INPUT_FLAG ? &fds : NULL,
+            *writefds = flags & wxSOCKET_OUTPUT_FLAG ? &fds : NULL;
+
+        // make a copy as it can be modified by select()
+        struct timeval tv = m_timeout;
+        int ret = select(m_fd + 1, readfds, writefds, NULL, &tv);
+
+        switch ( ret )
+        {
+            case 0:
+                m_error = wxSOCKET_TIMEDOUT;
+                return false;
+
+            case -1:
+                m_error = wxSOCKET_IOERR;
+                return false;
+        }
+    }
+    //else: we're non-blocking, never block
+
+    return true;
+}
+
 // ==========================================================================
 // wxSocketBase
 // ==========================================================================

src/msw/sockmsw.cpp

@@ -488,10 +488,9 @@ wxSocketImpl *wxSocketImplMSW::WaitConnection(wxSocketBase& wxsocket)
   }
 
   /* Wait for a connection (with timeout) */
-  if (Input_Timeout() == wxSOCKET_TIMEDOUT)
+  if ( !BlockForInputWithTimeout() )
   {
     delete connection;
-    /* m_error set by Input_Timeout */
     return NULL;
   }
 
@@ -601,9 +600,8 @@ int wxSocketImplMSW::Read(void *buffer, int size)
   }
 
   /* If the socket is blocking, wait for data (with a timeout) */
-  if (Input_Timeout() == wxSOCKET_TIMEDOUT)
+  if ( !BlockForInputWithTimeout() )
   {
-    m_error = wxSOCKET_TIMEDOUT;
     return -1;
   }
 
@@ -636,7 +634,7 @@ int wxSocketImplMSW::Write(const void *buffer, int size)
   }
 
   /* If the socket is blocking, wait for writability (with a timeout) */
-  if (Output_Timeout() == wxSOCKET_TIMEDOUT)
+  if ( BlockForOutputWithTimeout() )
     return -1;
 
   /* Write the data */
@@ -666,48 +664,6 @@ int wxSocketImplMSW::Write(const void *buffer, int size)
 
 /* Internals (IO) */
 
-/*
- *  For blocking sockets, wait until data is available or
- *  until timeout ellapses.
- */
-wxSocketError wxSocketImplMSW::Input_Timeout()
-{
-  fd_set readfds;
-
-  if (!m_non_blocking)
-  {
-    FD_ZERO(&readfds);
-    FD_SET(m_fd, &readfds);
-    if (select(0, &readfds, NULL, NULL, &m_timeout) == 0)
-    {
-      m_error = wxSOCKET_TIMEDOUT;
-      return wxSOCKET_TIMEDOUT;
-    }
-  }
-  return wxSOCKET_NOERROR;
-}
-
-/*
- *  For blocking sockets, wait until data can be sent without
- *  blocking or until timeout ellapses.
- */
-wxSocketError wxSocketImplMSW::Output_Timeout()
-{
-  fd_set writefds;
-
-  if (!m_non_blocking)
-  {
-    FD_ZERO(&writefds);
-    FD_SET(m_fd, &writefds);
-    if (select(0, NULL, &writefds, NULL, &m_timeout) == 0)
-    {
-      m_error = wxSOCKET_TIMEDOUT;
-      return wxSOCKET_TIMEDOUT;
-    }
-  }
-  return wxSOCKET_NOERROR;
-}
-
 /*
  *  For blocking sockets, wait until the connection is
  *  established or fails, or until timeout ellapses.

src/unix/sockunix.cpp

@@ -487,10 +487,9 @@ wxSocketImpl *wxSocketImplUnix::WaitConnection(wxSocketBase& wxsocket)
   }
 
   /* Wait for a connection (with timeout) */
-  if (Input_Timeout() == wxSOCKET_TIMEDOUT)
+  if ( !BlockForInputWithTimeout() )
   {
     delete connection;
-    /* m_error set by Input_Timeout */
     return NULL;
   }
 
@@ -588,23 +587,20 @@ wxSocketError wxSocketImplUnix::DoHandleConnect(int ret)
      */
     if ((err == EINPROGRESS) && (!m_non_blocking))
     {
-      if (Output_Timeout() == wxSOCKET_TIMEDOUT)
+      if ( !BlockForOutputWithTimeout() )
       {
         Close();
-        /* m_error is set in Output_Timeout */
         return wxSOCKET_TIMEDOUT;
       }
-      else
-      {
-        int error;
-        SOCKOPTLEN_T len = sizeof(error);
 
-        getsockopt(m_fd, SOL_SOCKET, SO_ERROR, (char*) &error, &len);
+      int error;
-        EnableEvents();
+      SOCKOPTLEN_T len = sizeof(error);
 
-        if (!error)
+      getsockopt(m_fd, SOL_SOCKET, SO_ERROR, (char*) &error, &len);
-          return wxSOCKET_NOERROR;
+      EnableEvents();
-      }
+
+      if (!error)
+        return wxSOCKET_NOERROR;
     }
 
     /* If connect failed with EINPROGRESS and the wxSocketImplUnix object
@@ -649,7 +645,8 @@ int wxSocketImplUnix::Read(void *buffer, int size)
   DisableEvent(wxSOCKET_INPUT);
 
   /* If the socket is blocking, wait for data (with a timeout) */
-  if (Input_Timeout() == wxSOCKET_TIMEDOUT) {
+  if ( !BlockForInputWithTimeout() )
+  {
     m_error = wxSOCKET_TIMEDOUT;
     /* Don't return here immediately, otherwise socket events would not be
      * re-enabled! */
@@ -712,7 +709,7 @@ int wxSocketImplUnix::Write(const void *buffer, int size)
   SOCKET_DEBUG(( "Write #2, size %d\n", size ));
 
   /* If the socket is blocking, wait for writability (with a timeout) */
-  if (Output_Timeout() == wxSOCKET_TIMEDOUT)
+  if ( !BlockForOutputWithTimeout() )
     return -1;
 
   SOCKET_DEBUG(( "Write #3, size %d\n", size ));
@@ -773,99 +770,6 @@ void wxSocketImplUnix::DisableEvent(wxSocketNotify event)
     }
 }
 
-/*
- *  For blocking sockets, wait until data is available or
- *  until timeout ellapses.
- */
-wxSocketError wxSocketImplUnix::Input_Timeout()
-{
-  fd_set readfds;
-  int ret;
-
-  // Linux select() will overwrite the struct on return so make a copy
-  struct timeval tv = m_timeout;
-
-  if (!m_non_blocking)
-  {
-    wxFD_ZERO(&readfds);
-    wxFD_SET(m_fd, &readfds);
-    ret = select(m_fd + 1, &readfds, NULL, NULL, &tv);
-    if (ret == 0)
-    {
-      SOCKET_DEBUG(( "Input_Timeout, select returned 0\n" ));
-      m_error = wxSOCKET_TIMEDOUT;
-      return wxSOCKET_TIMEDOUT;
-    }
-
-    if (ret == -1)
-    {
-      SOCKET_DEBUG(( "Input_Timeout, select returned -1\n" ));
-      if (errno == EBADF) { SOCKET_DEBUG(( "Invalid file descriptor\n" )); }
-      if (errno == EINTR) { SOCKET_DEBUG(( "A non blocked signal was caught\n" )); }
-      if (errno == EINVAL) { SOCKET_DEBUG(( "The highest number descriptor is negative\n" )); }
-      if (errno == ENOMEM) { SOCKET_DEBUG(( "Not enough memory\n" )); }
-      m_error = wxSOCKET_TIMEDOUT;
-      return wxSOCKET_TIMEDOUT;
-    }
-  }
-
-  return wxSOCKET_NOERROR;
-}
-
-/*
- *  For blocking sockets, wait until data can be sent without
- *  blocking or until timeout ellapses.
- */
-wxSocketError wxSocketImplUnix::Output_Timeout()
-{
-  fd_set writefds;
-  int ret;
-
-  // Linux select() will overwrite the struct on return so make a copy
-  struct timeval tv = m_timeout;
-
-  SOCKET_DEBUG( ("m_non_blocking has: %d\n", (int)m_non_blocking) );
-
-  if (!m_non_blocking)
-  {
-    wxFD_ZERO(&writefds);
-    wxFD_SET(m_fd, &writefds);
-    ret = select(m_fd + 1, NULL, &writefds, NULL, &tv);
-    if (ret == 0)
-    {
-      SOCKET_DEBUG(( "Output_Timeout, select returned 0\n" ));
-      m_error = wxSOCKET_TIMEDOUT;
-      return wxSOCKET_TIMEDOUT;
-    }
-
-    if (ret == -1)
-    {
-      SOCKET_DEBUG(( "Output_Timeout, select returned -1\n" ));
-      if (errno == EBADF) { SOCKET_DEBUG(( "Invalid file descriptor\n" )); }
-      if (errno == EINTR) { SOCKET_DEBUG(( "A non blocked signal was caught\n" )); }
-      if (errno == EINVAL) { SOCKET_DEBUG(( "The highest number descriptor is negative\n" )); }
-      if (errno == ENOMEM) { SOCKET_DEBUG(( "Not enough memory\n" )); }
-      m_error = wxSOCKET_TIMEDOUT;
-      return wxSOCKET_TIMEDOUT;
-    }
-
-    if ( ! wxFD_ISSET(m_fd, &writefds) )
-    {
-        SOCKET_DEBUG(( "Output_Timeout is buggy!\n" ));
-    }
-    else
-    {
-        SOCKET_DEBUG(( "Output_Timeout seems correct\n" ));
-    }
-  }
-  else
-  {
-    SOCKET_DEBUG(( "Output_Timeout, didn't try select!\n" ));
-  }
-
-  return wxSOCKET_NOERROR;
-}
-
 int wxSocketImplUnix::Recv_Stream(void *buffer, int size)
 {
   int ret;