Browse Source

[1522] made sure the status code includes \n. also made sure passed sockets
will be successfully closed.

JINMEI Tatuya 13 years ago
parent
commit
182024bdd5

+ 7 - 6
src/lib/server_common/socket_request.cc

@@ -34,14 +34,14 @@ SocketRequestor* requestor(NULL);
 // Before the boss process calls send_fd, it first sends this
 // string to indicate success, followed by the file descriptor
 const std::string& CREATOR_SOCKET_OK() {
-    static const std::string str("1");
+    static const std::string str("1\n");
     return (str);
 }
 
 // Before the boss process calls send_fd, it sends this
 // string to indicate failure. It will not send a file descriptor.
 const std::string& CREATOR_SOCKET_UNAVAILABLE() {
-    static const std::string str("0");
+    static const std::string str("0\n");
     return (str);
 }
 
@@ -185,9 +185,9 @@ int
 getSocketFd(int sock_pass_fd) {
     // Boss first sends some data to signal that getting the socket
     // from its cache succeeded
-    char status[2];
-    memset(status, 0, 2);
-    if (isc::util::io::read_data(sock_pass_fd, &status, 2) < 2) {
+    char status[3];        // We need a space for trailing \0, hence 3
+    memset(status, 0, 3);
+    if (isc::util::io::read_data(sock_pass_fd, status, 2) < 2) {
         isc_throw(SocketRequestor::SocketError,
                   "Error reading status code while requesting socket");
     }
@@ -197,7 +197,8 @@ getSocketFd(int sock_pass_fd) {
                   "CREATOR_SOCKET_UNAVAILABLE returned");
     } else if (CREATOR_SOCKET_OK() != status) {
         isc_throw(SocketRequestor::SocketError,
-                  "Unknown status code returned before recv_fd " << status);
+                  "Unknown status code returned before recv_fd '" << status <<
+                  "'");
     }
 
     const int passed_sock_fd = isc::util::io::recv_fd(sock_pass_fd);

+ 20 - 12
src/lib/server_common/tests/socket_requestor_test.cc

@@ -401,24 +401,28 @@ private:
             isc_throw(Exception, "Error in accept(): " << strerror(errno));
         }
         BOOST_FOREACH(int cur_data, data) {
-            int result;
+            bool result;
             if (cur_data == -1) {
                 // send 'CREATOR_SOCKET_UNAVAILABLE'
-                result = isc::util::io::write_data(client_fd, "0", 2);
+                result = isc::util::io::write_data(client_fd, "0\n", 2);
             } else if (cur_data == -2) {
                 // send 'CREATOR_SOCKET_OK' first
-                result = isc::util::io::write_data(client_fd, "1", 2);
-                if (result == 1) {
-                    result = send(client_fd, "a", 1, 0);
+                result = isc::util::io::write_data(client_fd, "1\n", 2);
+                if (result) {
+                    if (send(client_fd, "a", 1, 0) != 1) {
+                        result = false;
+                    }
                 }
             } else {
                 // send 'CREATOR_SOCKET_OK' first
-                result = isc::util::io::write_data(client_fd, "1", 2);
-                if (result == 1) {
-                    result = isc::util::io::send_fd(client_fd, cur_data);
+                result = isc::util::io::write_data(client_fd, "1\n", 2);
+                if (result) {
+                    if (isc::util::io::send_fd(client_fd, cur_data) != 0) {
+                        result = false;
+                    }
                 }
             }
-            if (result < 0) {
+            if (!result) {
                 isc_throw(Exception, "Error in send_fd(): " <<
                           strerror(errno));
             }
@@ -445,16 +449,19 @@ TEST_F(SocketRequestorTest, testSocketPassing) {
     addAnswer("foo", ts.getPath());
     SocketRequestor::SocketID socket_id = doRequest();
     ASSERT_EQ("foo", socket_id.second);
+    ASSERT_EQ(0, close(socket_id.first));
 
     // 2 should be ok too
     addAnswer("bar", ts.getPath());
     socket_id = doRequest();
     ASSERT_EQ("bar", socket_id.second);
+    ASSERT_EQ(0, close(socket_id.first));
 
     // 3 should be ok too (reuse earlier token)
     addAnswer("foo", ts.getPath());
     socket_id = doRequest();
     ASSERT_EQ("foo", socket_id.second);
+    ASSERT_EQ(0, close(socket_id.first));
 
     // -1 should not
     addAnswer("foo", ts.getPath());
@@ -475,21 +482,22 @@ TEST_F(SocketRequestorTest, testSocketPassing) {
     addAnswer("foo", ts2.getPath());
     socket_id = doRequest();
     ASSERT_EQ("foo", socket_id.second);
+    ASSERT_EQ(0, close(socket_id.first));
 
     // Now use first socket again
     addAnswer("foo", ts.getPath());
     socket_id = doRequest();
     ASSERT_EQ("foo", socket_id.second);
+    ASSERT_EQ(0, close(socket_id.first));
 
     // Vector is of first socket is now empty, so the socket should be gone
     addAnswer("foo", ts.getPath());
     ASSERT_THROW(doRequest(), SocketRequestor::SocketError);
 
-    // Vector is of second socket is now empty too, so the socket should be gone
+    // Vector is of second socket is now empty too, so the socket should be
+    // gone
     addAnswer("foo", ts2.getPath());
     ASSERT_THROW(doRequest(), SocketRequestor::SocketError);
-
-
 }