Parcourir la source

[1522] address vorner's comments

Jelte Jansen il y a 13 ans
Parent
commit
67f67098c1

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

@@ -32,14 +32,14 @@ namespace {
 SocketRequestor* requestor(NULL);
 
 // Before the boss process calls send_fd, it first sends this
-// string to indicate success
+// string to indicate success, followed by the file descriptor
 const std::string& CREATOR_SOCKET_OK() {
     static const std::string str("1");
     return (str);
 }
 
-// Before the boss process calls send_fd, it first sends this
-// string to indicate failure
+// 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");
     return (str);
@@ -276,7 +276,7 @@ private:
         // from its cache succeeded
         char status[2];
         memset(status, 0, 2);
-        if (isc::util::io::read_data(sock_pass_fd, &status, 1) < 1) {
+        if (isc::util::io::read_data(sock_pass_fd, &status, 2) < 2) {
             isc_throw(SocketError,
                       "Error reading status code while requesting socket");
         }
@@ -313,8 +313,10 @@ private:
     // Closes the sockets that has been used for fd_share
     void
     closeFdShareSockets() {
-        std::map<std::string, int>::iterator it;
-        for (it = fd_share_sockets_.begin(); it != fd_share_sockets_.end(); ++it ) {
+        for (std::map<std::string, int>::iterator it =
+                fd_share_sockets_.begin();
+             it != fd_share_sockets_.end();
+             ++it ) {
             close((*it).second);
         }
     }

+ 1 - 2
src/lib/server_common/socket_request.h

@@ -152,8 +152,7 @@ public:
     ///
     /// \param session the CC session that'll be used to talk to the
     ///                socket creator.
-    /// \throw InvalidOperation when it is called more than once,
-    ///                         when socket_path is empty
+    /// \throw InvalidOperation when it is called more than once
     static void init(config::ModuleCCSession& session);
 
     /// \brief Initialization for tests

+ 7 - 10
src/lib/server_common/tests/socket_requestor_test.cc

@@ -110,7 +110,7 @@ public:
         session.getMessages()->add(createAnswer(0, answer_part));
     }
 
-    // Clears the messages the client sent to far on the fake msgq
+    // Clears the messages the client sent so far on the fake msgq
     // (for easier access to new messages later)
     void
     clearMsgQueue() {
@@ -154,7 +154,6 @@ TEST_F(SocketRequestorTest, testSocketRequestMessages) {
     // answer here.
     // We are only testing the request messages that are sent,
     // so for this test that is no problem
-
     clearMsgQueue();
     ConstElementPtr expected_request;
 
@@ -276,9 +275,11 @@ TEST_F(SocketRequestorTest, testSocketReleaseMessages) {
 }
 
 TEST_F(SocketRequestorTest, testBadSocketReleaseAnswers) {
+    // Should fail if there is no answer at all
     ASSERT_THROW(socketRequestor().releaseSocket("bar"),
                  CCSessionError);
 
+    // Should also fail if the answer is an error
     session.getMessages()->add(createAnswer(1, "error"));
     ASSERT_THROW(socketRequestor().releaseSocket("bar"),
                  SocketRequestor::SocketError);
@@ -377,11 +378,7 @@ private:
     // send_fd())
     void
     serve(std::vector<int> data) {
-        struct sockaddr_un client_address;
-        socklen_t ca_len = sizeof(client_address);
-        int client_fd = accept(fd_,
-                               (struct sockaddr*) &client_address,
-                               &ca_len);
+        int client_fd = accept(fd_, NULL, NULL);
         if (client_fd == -1) {
             isc_throw(Exception, "Error in accept(): " << strerror(errno));
         }
@@ -389,16 +386,16 @@ private:
             int result;
             if (cur_data == -1) {
                 // send 'CREATOR_SOCKET_UNAVAILABLE'
-                result = isc::util::io::write_data(client_fd, "0", 1);
+                result = isc::util::io::write_data(client_fd, "0", 2);
             } else if (cur_data == -2) {
                 // send 'CREATOR_SOCKET_OK' first
-                result = isc::util::io::write_data(client_fd, "1", 1);
+                result = isc::util::io::write_data(client_fd, "1", 2);
                 if (result == 1) {
                     result = send(client_fd, "a", 1, 0);
                 }
             } else {
                 // send 'CREATOR_SOCKET_OK' first
-                result = isc::util::io::write_data(client_fd, "1", 1);
+                result = isc::util::io::write_data(client_fd, "1", 2);
                 if (result == 1) {
                     result = isc::util::io::send_fd(client_fd, cur_data);
                 }