wsd: return status from writeOutgoingData

Without knowing whether the write succeeded
or failed, we cannot trust errno has our
error or some earlier and unrelated error.

This was caught when there were two sockets,
one of which disconnected. The write to the
disconnected one returned -1 and set errno
to ECONNRESET. We subsequently wrote to the
second socket, which succeeded. However,
because errno wasn't reset, and since
writeOutgoingData didn't return anything
to indicate the success, errno's ECONNRESET
value meant the second socket was also
disconnected, which was incorrect.

writeOutgoingData now returns the last return
value from writeData so we can make informed
decision as to whether to check errno or not.

Also, to avoid incorrecly propagating errno,
we now capture errno only when readData and
writeData return -1. This has the nice
side-effect that we reset errno to 0 when
no errors occur during our call.

Change-Id: I911b31390f37cc71938bc4a6ae75393dbf24bb9d
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
(cherry picked from commit dec21487859873c416ecd7191501973f36dc9cf4)

2 files changed, 22 insertions, 16 deletions

diff --git a/net/Socket.hpp b/net/Socket.hpp
index d7e6cfe557..c31e8c58ae 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -1106,7 +1106,7 @@ public:
 #if !MOBILEAPP
         // SSL decodes blocks of 16Kb, so for efficiency we use the same.
         char buf[16 * 1024];
-        ssize_t len;
+        ssize_t len = 0;
         int last_errno = 0;
         do
         {
@@ -1117,7 +1117,8 @@ public:
             do
             {
                 len = readData(buf, sizeof(buf));
-                last_errno = errno;
+                if (len < 0)
+                    last_errno = errno; // Save only on error.
 
                 if (len < 0 && last_errno != EAGAIN && last_errno != EWOULDBLOCK)
                     LOG_SYS_ERRNO(last_errno, '#' << getFD() << ": read failed, have "
@@ -1363,15 +1364,17 @@ protected:
             // Write if we can and have data to write.
             if ((events & POLLOUT) && !_outBuffer.empty())
             {
-                writeOutgoingData();
-                const int last_errno = errno;
-                if (last_errno == EPIPE || (EnableExperimental && last_errno == ECONNRESET))
+                if (writeOutgoingData() < 0)
                 {
-                    LOG_DBG('#' << getFD() << ": Disconnected while writing ("
-                                << Util::symbolicErrno(last_errno)
-                                << "): " << std::strerror(last_errno) << ')');
-                    closed = true;
-                    break;
+                    const int last_errno = errno;
+                    if (last_errno == EPIPE || (EnableExperimental && last_errno == ECONNRESET))
+                    {
+                        LOG_DBG('#' << getFD() << ": Disconnected while writing ("
+                                    << Util::symbolicErrno(last_errno)
+                                    << "): " << std::strerror(last_errno) << ')');
+                        closed = true;
+                        break;
+                    }
                 }
             }
         }
@@ -1390,14 +1393,15 @@ protected:
 
 public:
     /// Override to write data out to socket.
-    virtual void writeOutgoingData()
+    /// Returns the last return from writeData.
+    virtual int writeOutgoingData()
     {
         ASSERT_CORRECT_SOCKET_THREAD(this);
         assert(!_outBuffer.empty());
+        ssize_t len = 0;
         int last_errno = 0;
         do
         {
-            ssize_t len = 0;
             do
             {
                 // Writing much more than we can absorb in the kernel causes wastage.
@@ -1406,7 +1410,8 @@ public:
                     break;
 
                 len = writeData(_outBuffer.getBlock(), size);
-                last_errno = errno; // Save right after the syscall.
+                if (len < 0)
+                    last_errno = errno; // Save only on error.
 
                 // 0 len is unspecified result, according to man write(2).
                 if (len < 0 && last_errno != EAGAIN && last_errno != EWOULDBLOCK)
@@ -1441,6 +1446,7 @@ public:
 
         // Restore errno from the write call.
         errno = last_errno;
+        return len;
     }
 
     /// Does it look like we have some TLS / SSL where we don't expect it ?
diff --git a/net/SslSocket.hpp b/net/SslSocket.hpp
index 6985c67164..e546d1df77 100644
--- a/net/SslSocket.hpp
+++ b/net/SslSocket.hpp
@@ -125,18 +125,18 @@ public:
         return StreamSocket::readIncomingData();
     }
 
-    void writeOutgoingData() override
+    int writeOutgoingData() override
     {
         ASSERT_CORRECT_SOCKET_THREAD(this);
 
         const int rc = doHandshake();
         if (rc <= 0)
         {
-            return;
+            return rc;
         }
 
         // Default implementation.
-        StreamSocket::writeOutgoingData();
+        return StreamSocket::writeOutgoingData();
     }
 
     virtual int readData(char* buf, int len) override