Browse Source

some changes inspired by jinmei's review

git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac183@1933 e5f2f494-b856-4b98-b285-d166d9295462
Jelte Jansen 15 years ago
parent
commit
1306183eb7

+ 0 - 3
src/bin/bind10/bind10.py.in

@@ -176,9 +176,6 @@ class BoB:
         self.dead_processes = {}
         self.runnable = False
 
-        if self.msgq_socket_file is not None:
-            os.environ['BIND10_MSGQ_SOCKET_FILE'] = str(self.msgq_socket_file)
-
     def config_handler(self, new_config):
         if self.verbose:
             print("[bind10] handling new config:")

+ 0 - 2
src/bin/msgq/msgq.py.in

@@ -140,8 +140,6 @@ class MsgQ:
     def setup_listener(self):
         """Set up the listener socket.  Internal function."""
         self.listen_socket = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
-        #self.listen_socket.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
-        #self.listen_socket.bind(("127.0.0.1", self.c_channel_port))
         
         if os.path.exists(self.socket_file):
             os.remove(self.socket_file)

+ 13 - 6
src/bin/msgq/tests/msgq_test.py

@@ -78,21 +78,28 @@ class TestSubscriptionManager(unittest.TestCase):
         self.assertFalse(os.path.exists("./my_socket_file"))
 
     def test_open_socket_default(self):
+        env_var = None
         if "BIND10_MSGQ_SOCKET_FILE" in os.environ:
+            env_var = os.environ["BIND10_MSGQ_SOCKET_FILE"]
             del os.environ["BIND10_MSGQ_SOCKET_FILE"]
         socket_file = MsgQ.SOCKET_FILE
         self.assertFalse(os.path.exists(socket_file))
         msgq = MsgQ();
-        msgq.setup()
-        self.assertTrue(os.path.exists(socket_file))
-        msgq.shutdown();
-        self.assertFalse(os.path.exists(socket_file))
-        pass
+        try:
+            msgq.setup()
+            self.assertTrue(os.path.exists(socket_file))
+            msgq.shutdown();
+            self.assertFalse(os.path.exists(socket_file))
+        except socket.error:
+            # ok, the install path doesn't exist at all,
+            # so we can't check any further
+            pass
+        if env_var is not None:
+            os.environ["BIND10_MSGQ_SOCKET_FILE"] = env_var
 
     def test_open_socket_bad(self):
         msgq = MsgQ("/does/not/exist")
         self.assertRaises(socket.error, msgq.setup)
-        pass
 
 if __name__ == '__main__':
     unittest.main()

+ 7 - 1
src/lib/cc/Makefile.am

@@ -13,12 +13,18 @@ BUILT_SOURCES = session_config.h
 TESTS =
 if HAVE_GTEST
 TESTS += run_unittests
-run_unittests_SOURCES = data_unittests.cc run_unittests.cc
+run_unittests_SOURCES = data_unittests.cc session_unittests.cc run_unittests.cc
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)
 run_unittests_LDADD = libcc.a $(GTEST_LDADD)
 run_unittests_LDADD +=  $(top_builddir)/src/lib/dns/.libs/libdns.a
 run_unittests_LDADD +=  $(top_builddir)/src/lib/exceptions/.libs/libexceptions.a
+
+if HAVE_BOOST_SYSTEM
+run_unittests_LDFLAGS += $(AM_LDFLAGS) $(BOOST_LDFLAGS)
+run_unittests_LDADD += $(BOOST_SYSTEM_LIB)
+endif
+
 endif
 
 noinst_PROGRAMS = $(TESTS)

+ 12 - 2
src/lib/cc/session.cc

@@ -107,7 +107,11 @@ ASIOSession::establish(const char* socket_file) {
     if (!socket_file) {
         socket_file = BIND10_MSGQ_SOCKET_FILE;
     }
-    socket_.connect(boost::asio::local::stream_protocol::endpoint(socket_file), error_);
+    try {
+        socket_.connect(boost::asio::local::stream_protocol::endpoint(socket_file), error_);
+    } catch (boost::system::system_error se) {
+        isc_throw(SessionError, se.what());
+    }
     if (error_) {
         isc_throw(SessionError, "Unable to connect to message queue.");
     }
@@ -227,6 +231,9 @@ SocketSession::establish(const char* socket_file) {
     int s;
     struct sockaddr_un sun;
 
+#ifdef HAVE_SUN_LEN
+    sun.sun_len = sizeof(struct sockaddr_un);
+#endif
     s = socket(AF_UNIX, SOCK_STREAM, IPPROTO_TCP);
     if (s < 0) {
         isc_throw(SessionError, "socket() failed");
@@ -239,9 +246,12 @@ SocketSession::establish(const char* socket_file) {
         socket_file = BIND10_MSGQ_SOCKET_FILE;
     }
 
+/*    if (strlen(socket_file) >= sizeof(sun.sun_path)) {
+        isc_throw(SessionError, "Unable to connect to message queue; socket file path too long");
+    }
+*/
     sun.sun_family = AF_UNIX;
     strncpy(sun.sun_path, socket_file, sizeof(sun.sun_path) - 1);
-    sun.sun_path[sizeof(sun.sun_path) - 1] = '\0';
 
     if (connect(s, (struct sockaddr *)&sun, sizeof(sun)) < 0) {
         close(s);

+ 61 - 0
src/lib/cc/session_unittests.cc

@@ -0,0 +1,61 @@
+// Copyright (C) 2009  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+// $Id: data_unittests.cc 1899 2010-05-21 12:03:59Z jelte $
+
+#include <gtest/gtest.h>
+#include <session.h>
+#include <boost/asio.hpp>
+#include <exceptions/exceptions.h>
+
+using namespace isc::cc;
+
+TEST(AsioSession, establish) {
+    boost::asio::io_service io_service_;
+    Session sess(io_service_);
+
+    EXPECT_THROW(
+        sess.establish("/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/"
+                       "/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/"
+                       "/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/"
+                       "/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/"
+                       "/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/"
+                       "/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/"
+                       "/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/"
+                       "/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/"
+                       "/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/"
+                       "/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/"
+                  ), isc::cc::SessionError
+    );
+                  
+}
+
+TEST(Session, establish) {
+    Session sess;
+
+    EXPECT_THROW(
+        sess.establish("/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/"
+                       "/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/"
+                       "/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/"
+                       "/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/"
+                       "/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/"
+                       "/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/"
+                       "/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/"
+                       "/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/"
+                       "/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/"
+                       "/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/"
+                  ), isc::cc::SessionError
+    );
+                  
+}