QPID-6435: Restore decent error messages to the SSL code

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1696715 13f79535-47bb-0310-9956-ffa450edef68

8 files changed, 73 insertions, 62 deletions

diff --git a/qpid/cpp/src/qpid/sys/Socket.h b/qpid/cpp/src/qpid/sys/Socket.h
index 38183bd5fd..7d84209390 100644
--- a/qpid/cpp/src/qpid/sys/Socket.h
+++ b/qpid/cpp/src/qpid/sys/Socket.h
@@ -62,6 +62,7 @@ public:
      * socket
      */
     virtual std::string getPeerAddress() const = 0;
+
     /**
      * Returns an address (host and port) for the local end of the
      * socket
@@ -79,12 +80,31 @@ public:
      */
     virtual int getError() const = 0;
 
-    /** Accept a connection from a socket that is already listening
+    /**
+     * Returns error text for last read/write error code
+     */
+    virtual std::string lastErrorCodeText() const = 0;
+
+    /**
+     * Accept a connection from a socket that is already listening
      * and has an incoming connection
      */
     virtual Socket* accept() const = 0;
 
+    /** Read up to count bytes into buffer
+     * If return is positive read that number of bytes;
+     * if 0 then end of input stream (or disconnected)
+     * if <0 then error, in this case an error text can be fetched
+     *    using lastErrorCodeText().
+     */
     virtual int read(void *buf, size_t count) const = 0;
+
+    /** Write up to count bytes into buffer
+     * If return is positive wrote that number of bytes;
+     * if 0 then stream closed (or disconnected)
+     * if <0 then error, in this case an error text can be fetched
+     *    using lastErrorCodeText().
+     */
     virtual int write(const void *buf, size_t count) const = 0;
 
     /* Transport security related: */
diff --git a/qpid/cpp/src/qpid/sys/posix/AsynchIO.cpp b/qpid/cpp/src/qpid/sys/posix/AsynchIO.cpp
index 7d04d2214d..870c22bcd3 100644
--- a/qpid/cpp/src/qpid/sys/posix/AsynchIO.cpp
+++ b/qpid/cpp/src/qpid/sys/posix/AsynchIO.cpp
@@ -481,7 +481,7 @@ void AsynchIO::readable(DispatchHandle& h) {
                     break;
                 } else {
                     // Report error then just treat as a socket disconnect
-                    QPID_LOG(error, "Error reading socket: " << qpid::sys::strError(errno) << "(" << errno << ")" );
+                    QPID_LOG(error, "Error reading socket: " << socket.lastErrorCodeText() );
                     eofCallback(*this);
                     h.unwatchRead();
                     break;
@@ -561,7 +561,7 @@ void AsynchIO::writeable(DispatchHandle& h) {
                     break;
                 } else {
                     // Report error then just treat as a socket disconnect
-                    QPID_LOG(error, "Error writing socket: " << qpid::sys::strError(errno) << "(" << errno << ")" );
+                    QPID_LOG(error, "Error writing socket: " << socket.lastErrorCodeText() );
                     h.unwatchWrite();
                     break;
                 }
diff --git a/qpid/cpp/src/qpid/sys/posix/BSDSocket.cpp b/qpid/cpp/src/qpid/sys/posix/BSDSocket.cpp
index 7c31b13ae9..415c32bfa5 100644
--- a/qpid/cpp/src/qpid/sys/posix/BSDSocket.cpp
+++ b/qpid/cpp/src/qpid/sys/posix/BSDSocket.cpp
@@ -34,6 +34,7 @@
 #include <netinet/tcp.h>
 #include <netdb.h>
 #include <cstdlib>
+#include <sstream>
 #include <string.h>
 
 namespace qpid {
@@ -68,8 +69,9 @@ uint16_t getLocalPort(int fd)
 }
 
 BSDSocket::BSDSocket() :
-    fd(-1),
     handle(new IOHandle),
+    fd(-1),
+    lastErrorCode(0),
     nonblocking(false),
     nodelay(false)
 {}
@@ -80,8 +82,9 @@ Socket* createSocket()
 }
 
 BSDSocket::BSDSocket(int fd0) :
+    handle(new IOHandle(fd0)),
     fd(fd0),
-    handle(new IOHandle(fd)),
+    lastErrorCode(0),
     nonblocking(false),
     nodelay(false)
 {}
@@ -214,14 +217,26 @@ Socket* BSDSocket::accept() const
     else throw QPID_POSIX_ERROR(errno);
 }
 
+std::string BSDSocket::lastErrorCodeText() const
+{
+    std::stringstream s;
+    s << strError(lastErrorCode);
+    s << "(" << lastErrorCode << ")";
+    return s.str();
+}
+
 int BSDSocket::read(void *buf, size_t count) const
 {
-    return ::read(fd, buf, count);
+    int rc = ::read(fd, buf, count);
+    lastErrorCode = errno;
+    return rc;
 }
 
 int BSDSocket::write(const void *buf, size_t count) const
 {
-    return ::write(fd, buf, count);
+    int rc = ::write(fd, buf, count);
+    lastErrorCode = errno;
+    return rc;
 }
 
 std::string BSDSocket::getPeerAddress() const
diff --git a/qpid/cpp/src/qpid/sys/posix/BSDSocket.h b/qpid/cpp/src/qpid/sys/posix/BSDSocket.h
index ae73718d55..4fa2aef782 100644
--- a/qpid/cpp/src/qpid/sys/posix/BSDSocket.h
+++ b/qpid/cpp/src/qpid/sys/posix/BSDSocket.h
@@ -53,46 +53,22 @@ public:
 
     QPID_COMMON_EXTERN operator const IOHandle&() const;
 
-    /** Set socket non blocking */
     QPID_COMMON_EXTERN virtual void setNonblocking() const;
-
     QPID_COMMON_EXTERN virtual void setTcpNoDelay() const;
 
-    QPID_COMMON_EXTERN virtual void connect(const SocketAddress&) const;
-    QPID_COMMON_EXTERN virtual void finishConnect(const SocketAddress&) const;
-
-    QPID_COMMON_EXTERN virtual void close() const;
-
-    /** Bind to a port and start listening.
-     *@return The bound port number
-     */
-    QPID_COMMON_EXTERN virtual int listen(const SocketAddress&, int backlog = 10) const;
-
-    /**
-     * Returns an address (host and port) for the remote end of the
-     * socket
-     */
     QPID_COMMON_EXTERN std::string getPeerAddress() const;
-    /**
-     * Returns an address (host and port) for the local end of the
-     * socket
-     */
     QPID_COMMON_EXTERN std::string getLocalAddress() const;
 
-    /**
-     * Returns the error code stored in the socket.  This may be used
-     * to determine the result of a non-blocking connect.
-     */
     QPID_COMMON_EXTERN int getError() const;
+    QPID_COMMON_EXTERN virtual std::string lastErrorCodeText() const;
 
-    /** Accept a connection from a socket that is already listening
-     * and has an incoming connection
-     */
+    QPID_COMMON_EXTERN virtual void connect(const SocketAddress&) const;
+    QPID_COMMON_EXTERN virtual void finishConnect(const SocketAddress&) const;
+    QPID_COMMON_EXTERN virtual int listen(const SocketAddress&, int backlog = 10) const;
     QPID_COMMON_EXTERN virtual Socket* accept() const;
-
-    // TODO The following are raw operations, maybe they need better wrapping?
     QPID_COMMON_EXTERN virtual int read(void *buf, size_t count) const;
     QPID_COMMON_EXTERN virtual int write(const void *buf, size_t count) const;
+    QPID_COMMON_EXTERN virtual void close() const;
 
     QPID_COMMON_EXTERN int getKeyLen() const;
     QPID_COMMON_EXTERN std::string getClientAuthId() const;
@@ -101,10 +77,11 @@ protected:
     /** Create socket */
     void createSocket(const SocketAddress&) const;
 
-    mutable int fd;
     mutable boost::scoped_ptr<IOHandle> handle;
     mutable std::string localname;
     mutable std::string peername;
+    mutable int fd;
+    mutable int lastErrorCode;
     mutable bool nonblocking;
     mutable bool nodelay;
 };
diff --git a/qpid/cpp/src/qpid/sys/ssl/SslSocket.cpp b/qpid/cpp/src/qpid/sys/ssl/SslSocket.cpp
index 32bc78d22d..179d7ce1cd 100644
--- a/qpid/cpp/src/qpid/sys/ssl/SslSocket.cpp
+++ b/qpid/cpp/src/qpid/sys/ssl/SslSocket.cpp
@@ -47,8 +47,6 @@
 #include <key.h>
 #include <sslerr.h>
 
-#include <boost/format.hpp>
-
 namespace qpid {
 namespace sys {
 namespace ssl {
@@ -317,14 +315,23 @@ Socket* SslMuxSocket::accept() const
     }
 }
 
+std::string SslSocket::lastErrorCodeText() const
+{
+  return getErrorString(lastErrorCode);
+}
+
 int SslSocket::read(void *buf, size_t count) const
 {
-    return PR_Read(nssSocket, buf, count);
+    PRInt32 r = PR_Read(nssSocket, buf, count);
+    lastErrorCode = PR_GetError();
+    return r;
 }
 
 int SslSocket::write(const void *buf, size_t count) const
 {
-    return PR_Write(nssSocket, buf, count);
+    PRInt32 r = PR_Write(nssSocket, buf, count);
+    lastErrorCode = PR_GetError();
+    return r;
 }
 
 void SslSocket::setCertName(const std::string& name)
diff --git a/qpid/cpp/src/qpid/sys/ssl/SslSocket.h b/qpid/cpp/src/qpid/sys/ssl/SslSocket.h
index 2407a1bf4b..692a0d3967 100644
--- a/qpid/cpp/src/qpid/sys/ssl/SslSocket.h
+++ b/qpid/cpp/src/qpid/sys/ssl/SslSocket.h
@@ -48,38 +48,23 @@ public:
     /** Proceed with connect inspite of hostname verifcation failures*/
     void ignoreHostnameVerificationFailure();
 
-    /** Set socket non blocking */
-    void setNonblocking() const;
-
-    /** Set tcp-nodelay */
-    void setTcpNoDelay() const;
-
     /** Set SSL cert-name. Allows the cert-name to be set per
      * connection, overriding global cert-name settings from
      * NSSInit().*/
     void setCertName(const std::string& certName);
 
-    void connect(const SocketAddress&) const;
-    void finishConnect(const SocketAddress&) const;
+    void setNonblocking() const;
+    void setTcpNoDelay() const;
 
-    void close() const;
+    std::string lastErrorCodeText() const;
 
-    /** Bind to a port and start listening.
-     *@param port 0 means choose an available port.
-     *@param backlog maximum number of pending connections.
-     *@return The bound port.
-     */
+    void connect(const SocketAddress&) const;
+    void finishConnect(const SocketAddress&) const;
     int listen(const SocketAddress&, int backlog = 10) const;
-
-    /**
-     * Accept a connection from a socket that is already listening
-     * and has an incoming connection
-     */
     virtual Socket* accept() const;
-
-    // TODO The following are raw operations, maybe they need better wrapping?
     int read(void *buf, size_t count) const;
     int write(const void *buf, size_t count) const;
+    void close() const;
 
     int getKeyLen() const;
     std::string getClientAuthId() const;
diff --git a/qpid/cpp/src/qpid/sys/windows/WinSocket.cpp b/qpid/cpp/src/qpid/sys/windows/WinSocket.cpp
index 5637f6a9fb..331bd8f48c 100644
--- a/qpid/cpp/src/qpid/sys/windows/WinSocket.cpp
+++ b/qpid/cpp/src/qpid/sys/windows/WinSocket.cpp
@@ -248,6 +248,11 @@ int WinSocket::getError() const
     return result;
 }
 
+// TODO: I don't think this can ever be called!
+std::string WinSocket::lastErrorCodeText() const
+{
+    return strError(::WSAGetLastError());
+}
 void WinSocket::setTcpNoDelay() const
 {
     SOCKET& socket = handle->fd;
diff --git a/qpid/cpp/src/qpid/sys/windows/WinSocket.h b/qpid/cpp/src/qpid/sys/windows/WinSocket.h
index bee6a58e7a..435f8de4c6 100644
--- a/qpid/cpp/src/qpid/sys/windows/WinSocket.h
+++ b/qpid/cpp/src/qpid/sys/windows/WinSocket.h
@@ -87,6 +87,8 @@ public:
      */
     QPID_COMMON_EXTERN int getError() const;
 
+    QPID_COMMON_EXTERN std::string lastErrorCodeText() const;
+
     /** Accept a connection from a socket that is already listening
      * and has an incoming connection
      */