Refactoring SockRAII

Change-Id: Ib264bb049ed52d644f2d5873dabcd7be532738a3

diff --git a/src/manager/client-async/connection-thread.cpp b/src/manager/client-async/connection-thread.cpp
index 8cd76b4..ffb6d8d 100644 (file)
--- a/src/manager/client-async/connection-thread.cpp
+++ b/src/manager/client-async/connection-thread.cpp
@@ -30,10 +30,6 @@
 
 namespace CKM {
 
-namespace {
-const int POLL_TIMEOUT = 8000;
-} // namespace anonymous
-
 ConnectionThread::Pipe::Pipe()
 {
     if (-1 == pipe(m_pipe))

diff --git a/src/manager/client-async/service.cpp b/src/manager/client-async/service.cpp
index 03ce4da..55c3be2 100644 (file)
--- a/src/manager/client-async/service.cpp
+++ b/src/manager/client-async/service.cpp
@@ -45,7 +45,7 @@ void Service::addRequest(AsyncRequest&& req)
     if(!m_socket) {
         m_socket.reset(new SockRAII());
         int ret;
-        if (CKM_API_SUCCESS != (ret = m_socket->Connect(m_interface.c_str()))) {
+        if (CKM_API_SUCCESS != (ret = m_socket->connect(m_interface.c_str()))) {
             LogError("Socket connection failed: " << ret);
             m_socket.reset();
             req.observer->ReceivedError(ret);
@@ -64,7 +64,7 @@ void Service::serviceError(int error)
     if (m_socket)
     {
         // stop listening on socket
-        m_descriptors.remove(m_socket->Get(), false);
+        m_descriptors.remove(m_socket->get(), false);
         // close the socket
         m_socket.reset();
     }
@@ -87,8 +87,8 @@ void Service::serviceError(int error)
 
 void Service::socketReady(int sock, short revents)
 {
-    if (sock != m_socket->Get()) {
-        LogError("Unexpected socket: " << sock << "!=" << m_socket->Get());
+    if (sock != m_socket->get()) {
+        LogError("Unexpected socket: " << sock << "!=" << m_socket->get());
         serviceError(CKM_API_ERROR_SOCKET);
         return;
     }
@@ -124,7 +124,7 @@ void Service::sendData()
     while (!m_sendQueue.empty()) {
         AsyncRequest& req = m_sendQueue.front();
 
-        ssize_t temp = TEMP_FAILURE_RETRY(write(m_socket->Get(),
+        ssize_t temp = TEMP_FAILURE_RETRY(write(m_socket->get(),
                                                 &req.buffer[req.written],
                                                 req.buffer.size() - req.written));
         if (-1 == temp) {
@@ -158,7 +158,7 @@ void Service::receiveData()
 {
     char buffer[RECV_BUFFER_SIZE];
 
-    ssize_t temp = TEMP_FAILURE_RETRY(read(m_socket->Get(), buffer, RECV_BUFFER_SIZE));
+    ssize_t temp = TEMP_FAILURE_RETRY(read(m_socket->get(), buffer, RECV_BUFFER_SIZE));
     if (-1 == temp) {
         int err = errno;
         LogError("Error in read: " << GetErrnoString(err));
@@ -201,9 +201,9 @@ void Service::receiveData()
 void Service::watch(short events)
 {
     if (0 == events)
-        m_descriptors.remove(m_socket->Get(), false);
+        m_descriptors.remove(m_socket->get(), false);
     else
-        m_descriptors.add(m_socket->Get(),
+        m_descriptors.add(m_socket->get(),
                           events,
                           [this](int sock, short revents){ socketReady(sock, revents); });
 }

diff --git a/src/manager/client/client-common.cpp b/src/manager/client/client-common.cpp
index 08d3492..c92e529 100644 (file)
--- a/src/manager/client/client-common.cpp
+++ b/src/manager/client/client-common.cpp
@@ -59,115 +59,92 @@ SockRAII::SockRAII() : m_sock(-1) {}
 
 SockRAII::~SockRAII()
 {
-    Disconnect();
+    disconnect();
 }
 
-int SockRAII::Connect(char const * const interface)
+int SockRAII::connect(const char * interface)
 {
-    int sock = -1;
-    try
-    {
-        if(!interface) {
-            LogError("No valid interface address given.");
-            throw CKM_API_ERROR_INPUT_PARAM;
-        }
+    if (!interface) {
+        LogError("No valid interface address given.");
+        return CKM_API_ERROR_INPUT_PARAM;
+    }
 
-        sock = socket(AF_UNIX, SOCK_STREAM, 0);
-        if(sock < 0) {
-            LogError("Error creating socket: " << CKM::GetErrnoString(errno));
-            throw CKM_API_ERROR_SOCKET;
-        }
+    int localSock = socket(AF_UNIX, SOCK_STREAM, 0);
 
-        // configure non-blocking mode
-        int flags;
-        if((flags = fcntl(sock, F_GETFL, 0)) < 0 || fcntl(sock, F_SETFL, flags | O_NONBLOCK) < 0)
-        {
-            LogError("Error in fcntl: " << CKM::GetErrnoString(errno));
-            throw CKM_API_ERROR_SOCKET;
-        }
+    if (localSock < 0) {
+        LogError("Error creating socket: " << CKM::GetErrnoString(errno));
+        return CKM_API_ERROR_SOCKET;
+    }
 
-        sockaddr_un clientAddr;
-        memset(&clientAddr, 0, sizeof(clientAddr));
-        clientAddr.sun_family = AF_UNIX;
-        if(strlen(interface) >= sizeof(clientAddr.sun_path))
-        {
-            LogError("Error: interface name " << interface << "is too long. Max len is:" << sizeof(clientAddr.sun_path));
-            throw CKM_API_ERROR_INPUT_PARAM;
-        }
+    int retCode = connectWrapper(localSock, interface);
 
-        strcpy(clientAddr.sun_path, interface);
-        LogDebug("ClientAddr.sun_path = " << interface);
+    if (retCode != CKM_API_SUCCESS) {
+        close(localSock);
+        return retCode;
+    }
 
-        int retval = TEMP_FAILURE_RETRY(::connect(sock, (struct sockaddr*)&clientAddr, SUN_LEN(&clientAddr)));
-        if(-1 == retval)
-        {
-            switch(errno)
-            {
-                case EACCES:
-                    LogError("Access denied");
-                    throw CKM_API_ERROR_ACCESS_DENIED;
-                    break;
-
-                case EINPROGRESS:
-                {
-                    if( 0 >= WaitForSocket(POLLOUT, POLL_TIMEOUT))
-                    {
-                        LogError("Error in WaitForSocket.");
-                        throw CKM_API_ERROR_SOCKET;
-                    }
-
-                    int error = 0;
-                    size_t len = sizeof(error);
-                    retval = getsockopt(sock, SOL_SOCKET, SO_ERROR, &error, &len);
-                    if(-1 == retval)
-                    {
-                        LogError("Error in getsockopt: " << CKM::GetErrnoString(error));
-                        throw CKM_API_ERROR_SOCKET;
-                    }
-
-                    if(error == EACCES)
-                    {
-                        LogError("Access denied");
-                        throw CKM_API_ERROR_ACCESS_DENIED;
-                    }
-
-                    if(error != 0)
-                    {
-                        LogError("Error in connect: " << CKM::GetErrnoString(error));
-                        throw CKM_API_ERROR_SOCKET;
-                    }
-
-                    break;
-                }
-
-                default:
-                    LogError("Error connecting socket: " << CKM::GetErrnoString(errno));
-                    throw CKM_API_ERROR_SOCKET;
-                    break;
-            }
+    disconnect();
+
+    m_sock = localSock;
+
+    return CKM_API_SUCCESS;
+}
+
+int SockRAII::connectWrapper(int sock, const char *interface) {
+    int flags;
+
+    // we need to be sure that socket is in blocking mode
+    if ((flags = fcntl(sock, F_GETFL, 0)) < 0 || fcntl(sock, F_SETFL, flags & ~O_NONBLOCK) < 0) {
+        LogError("Error in fcntl: " << CKM::GetErrnoString(errno));
+        return CKM_API_ERROR_SOCKET;
+    }
+
+    sockaddr_un clientAddr;
+    memset(&clientAddr, 0, sizeof(clientAddr));
+    clientAddr.sun_family = AF_UNIX;
+
+    if (strlen(interface) >= sizeof(clientAddr.sun_path)) {
+        LogError("Error: interface name " << interface << "is too long."
+            " Max len is:" << sizeof(clientAddr.sun_path));
+        return CKM_API_ERROR_INPUT_PARAM;
+    }
+
+    strcpy(clientAddr.sun_path, interface);
+    LogDebug("ClientAddr.sun_path = " << interface);
+
+    int retval = TEMP_FAILURE_RETRY(::connect(sock, (struct sockaddr*)&clientAddr, SUN_LEN(&clientAddr)));
+
+    // we don't need to support EINPROGRESS because the socket is in blocking mode
+    if(-1 == retval)
+    {
+        if (errno == EACCES) {
+            LogError("Access denied to interface: " << interface);
+            return CKM_API_ERROR_ACCESS_DENIED;
         }
+        LogError("Error connecting socket: " << CKM::GetErrnoString(errno));
+        return CKM_API_ERROR_SOCKET;
     }
-    catch(int & error_code) {
-        if(sock > 0)
-            close(sock);
-        return error_code;
+
+    // make the socket non-blocking
+    if ((flags = fcntl(sock, F_GETFL, 0)) < 0 || fcntl(sock, F_SETFL, flags | O_NONBLOCK) < 0) {
+        LogError("Error in fcntl: " << CKM::GetErrnoString(errno));
+        return CKM_API_ERROR_SOCKET;
     }
 
-    m_sock = sock;
     return CKM_API_SUCCESS;
 }
 
 bool SockRAII::isConnected() const {
-    return (m_sock != -1);
+    return (m_sock > -1);
 }
 
-void SockRAII::Disconnect() {
-    if( isConnected() )
+void SockRAII::disconnect() {
+    if (isConnected())
         close(m_sock);
     m_sock = -1;
 }
 
-int SockRAII::WaitForSocket(int event, int timeout)
+int SockRAII::waitForSocket(int event, int timeout)
 {
     int retval;
     pollfd desc[1];
@@ -187,7 +164,7 @@ int SockRAII::WaitForSocket(int event, int timeout)
     return retval;
 }
 
-int SockRAII::Get() const {
+int SockRAII::get() const {
     return m_sock;
 }
 
@@ -232,7 +209,7 @@ bool AliasSupport::isLabelEmpty() const {
 
 ServiceConnection::ServiceConnection(char const * const service_interface) {
     if(service_interface)
-        m_service_interface = std::string(service_interface);
+        m_serviceInterface = std::string(service_interface);
 }
 
 int ServiceConnection::processRequest( const CKM::RawBuffer &send_buf,
@@ -247,10 +224,10 @@ int ServiceConnection::processRequest( const CKM::RawBuffer &send_buf,
 int ServiceConnection::Connect()
 {
     // cleanup
-    if( isConnected() )
-        Disconnect();
+    if (isConnected())
+        disconnect();
 
-    return SockRAII::Connect(m_service_interface.c_str());
+    return SockRAII::connect(m_serviceInterface.c_str());
 }
 
 int ServiceConnection::send(const CKM::RawBuffer &send_buf)
@@ -269,13 +246,13 @@ int ServiceConnection::send(const CKM::RawBuffer &send_buf)
     ssize_t done = 0;
     while((send_buf.size() - done) > 0)
     {
-        if( 0 >= WaitForSocket(POLLOUT, POLL_TIMEOUT)) {
+        if( 0 >= waitForSocket(POLLOUT, POLL_TIMEOUT)) {
             LogError("Error in WaitForSocket.");
             ec = CKM_API_ERROR_SOCKET;
             break;
         }
 
-        ssize_t temp = TEMP_FAILURE_RETRY(write(Get(), &send_buf[done], send_buf.size() - done));
+        ssize_t temp = TEMP_FAILURE_RETRY(write(m_sock, &send_buf[done], send_buf.size() - done));
         if(-1 == temp) {
             LogError("Error in write: " << CKM::GetErrnoString(errno));
             ec = CKM_API_ERROR_SOCKET;
@@ -286,7 +263,7 @@ int ServiceConnection::send(const CKM::RawBuffer &send_buf)
     }
 
     if(ec != CKM_API_SUCCESS)
-        Disconnect();
+        disconnect();
 
     return ec;
 }
@@ -304,13 +281,13 @@ int ServiceConnection::receive(CKM::MessageBuffer &recv_buf)
     char buffer[c_recv_buf_len];
     do
     {
-        if( 0 >= WaitForSocket(POLLIN, POLL_TIMEOUT)) {
+        if( 0 >= waitForSocket(POLLIN, POLL_TIMEOUT)) {
             LogError("Error in WaitForSocket.");
             ec = CKM_API_ERROR_SOCKET;
             break;
         }
 
-        ssize_t temp = TEMP_FAILURE_RETRY(read(Get(), buffer, sizeof(buffer)));
+        ssize_t temp = TEMP_FAILURE_RETRY(read(m_sock, buffer, sizeof(buffer)));
         if(-1 == temp) {
             LogError("Error in read: " << CKM::GetErrnoString(errno));
             ec = CKM_API_ERROR_SOCKET;
@@ -329,7 +306,7 @@ int ServiceConnection::receive(CKM::MessageBuffer &recv_buf)
     while(!recv_buf.Ready());
 
     if(ec != CKM_API_SUCCESS)
-        Disconnect();
+        disconnect();
 
     return ec;
 }

diff --git a/src/manager/client/client-common.h b/src/manager/client/client-common.h
index 04ed36d..536f7b7 100644 (file)
--- a/src/manager/client/client-common.h
+++ b/src/manager/client/client-common.h
@@ -65,22 +65,21 @@ class SockRAII {
 
         virtual ~SockRAII();
 
-        int Connect(char const * const interface);
-        void Disconnect();
+        int connect(const char * interface);
+        void disconnect();
         bool isConnected() const;
-        int Get() const;
+        int get() const;
 
     protected:
-        int WaitForSocket(int event, int timeout);
-
-    private:
+        int connectWrapper(int socket, const char *interface);
+        int waitForSocket(int event, int timeout);
         int m_sock;
 };
 
-class ServiceConnection : public SockRAII
+class ServiceConnection : private SockRAII
 {
     public:
-        ServiceConnection(char const * const service_interface);
+        ServiceConnection(const char * service_interface);
 
         // roundtrip: send and receive
         int processRequest(const CKM::RawBuffer &send_buf,
@@ -93,12 +92,11 @@ class ServiceConnection : public SockRAII
         virtual ~ServiceConnection();
 
     private:
-        std::string m_service_interface;
+        std::string m_serviceInterface;
 
         int Connect();
 };
 
-
 /*
  * Decorator function that performs frequently repeated exception handling in
  * SS client API functions. Accepts lambda expression as an argument.