[3052] Addressed review comments.

Changes were largely cosmetic.

src/bin/d2/d2_queue_mgr.cc

@@ -32,7 +32,12 @@ D2QueueMgr::D2QueueMgr(isc::asiolink::IOService& io_service,
 
 D2QueueMgr::~D2QueueMgr() {
     // clean up
-    stopListening();
+    try {
+        stopListening();
+    } catch (...) {
+        // This catch is strictly for safety's sake, in case a future
+        // implementation isn't tidy or careful. 
+    }
 }
 
 void
@@ -67,9 +72,9 @@ D2QueueMgr::operator()(const dhcp_ddns::NameChangeListener::Result result,
 
 void
 D2QueueMgr::initUDPListener(const isc::asiolink::IOAddress& ip_address,
-                            const uint32_t& port,
-                            dhcp_ddns::NameChangeFormat format,
-                            bool reuse_address) {
+                            const uint32_t port,
+                            const dhcp_ddns::NameChangeFormat format,
+                            const bool reuse_address) {
 
     if (listener_) {
         isc_throw(D2QueueMgrError,
@@ -150,20 +155,22 @@ D2QueueMgr::peek() const {
 }
 
 const dhcp_ddns::NameChangeRequestPtr&
-D2QueueMgr::peekAt(size_t index) const {
+D2QueueMgr::peekAt(const size_t index) const {
     if (index >= getQueueSize()) {
         isc_throw(D2QueueMgrInvalidIndex,
-                  "D2QueueMgr peek beyond end of queue attempted");
+                  "D2QueueMgr peek beyond end of queue attempted"
+                  << " index: " << index << " queue size: " << getQueueSize());
     }
 
     return (ncr_queue_.at(index));
 }
 
 void
-D2QueueMgr::dequeueAt(size_t index) {
+D2QueueMgr::dequeueAt(const size_t index) {
     if (index >= getQueueSize()) {
         isc_throw(D2QueueMgrInvalidIndex,
-                  "D2QueueMgr dequeue beyond end of queue attempted");
+                  "D2QueueMgr dequeue beyond end of queue attempted"
+                  << " index: " << index << " queue size: " << getQueueSize());
     }
 
     RequestQueue::iterator pos = ncr_queue_.begin() + index;

src/bin/d2/d2_queue_mgr.h

@@ -23,6 +23,7 @@
 #include <dhcp_ddns/ncr_msg.h>
 #include <dhcp_ddns/ncr_io.h>
 
+#include <boost/noncopyable.hpp>
 #include <deque>
 
 namespace isc {
@@ -32,7 +33,7 @@ namespace d2 {
 /// @todo This may be replaced with an actual class in the future.
 typedef std::deque<dhcp_ddns::NameChangeRequestPtr> RequestQueue;
 
-/// @brief Thrown if the queue manager encounters an general error.
+/// @brief Thrown if the queue manager encounters a general error.
 class D2QueueMgrError : public isc::Exception {
 public:
     D2QueueMgrError(const char* file, size_t line, const char* what) :
@@ -128,14 +129,15 @@ public:
 /// It is important to note that the queue contents are preserved between
 /// state transitions.  In other words entries in the queue remain there
 /// until they are removed explicitly via the deque() or implicitly by
-/// via the flushQue() method.
+/// via the clearQueue() method.
 ///
-class D2QueueMgr : public dhcp_ddns::NameChangeListener::RequestReceiveHandler {
+class D2QueueMgr : public dhcp_ddns::NameChangeListener::RequestReceiveHandler,
+                   boost::noncopyable {
 public:
     /// @brief Maximum number of entries allowed in the request queue.
     /// NOTE that 1024 is an arbitrary choice picked for the initial
     /// implementation.
-    static const size_t MAX_QUEUE_DEFAULT=  1024;
+    static const size_t MAX_QUEUE_DEFAULT = 1024;
 
     /// @brief Defines the list of possible states for D2QueueMgr.
     enum State {
@@ -158,12 +160,12 @@ public:
     /// queue.
     /// This value must be greater than zero. It defaults to MAX_QUEUE_DEFAULT.
     ///
-    /// @throw D2QueueMgr error if max_queue_size is zero.
+    /// @throw D2QueueMgrError if max_queue_size is zero.
     D2QueueMgr(isc::asiolink::IOService& io_service,
                const size_t max_queue_size = MAX_QUEUE_DEFAULT);
 
     /// @brief Destructor
-    ~D2QueueMgr();
+    virtual ~D2QueueMgr();
 
     /// @brief Initializes the listener as a UDP listener.
     ///
@@ -177,9 +179,9 @@ public:
     /// @param reuse_address enables IP address sharing when true
     /// It defaults to false.
     void initUDPListener(const isc::asiolink::IOAddress& ip_address,
-                         const uint32_t& port,
-                         dhcp_ddns::NameChangeFormat format,
-                         bool reuse_address = false);
+                         const uint32_t port,
+                         const dhcp_ddns::NameChangeFormat format,
+                         const bool reuse_address = false);
 
     /// @brief Starts actively listening for requests.
     ///
@@ -281,7 +283,7 @@ public:
     ///
     /// @throw D2QueueMgrInvalidIndex if the given index is beyond the
     /// end of the queue.
-    const dhcp_ddns::NameChangeRequestPtr& peekAt(size_t index) const;
+    const dhcp_ddns::NameChangeRequestPtr& peekAt(const size_t index) const;
 
     /// @brief Removes the entry at a given position in the queue.
     ///
@@ -290,7 +292,7 @@ public:
     ///
     /// @throw D2QueueMgrInvalidIndex if the given index is beyond the
     /// end of the queue.
-    void dequeueAt(size_t index);
+    void dequeueAt(const size_t index);
 
     /// @brief Removes the entry at the front of the queue.
     ///

src/bin/d2/tests/d2_queue_mgr_unittests.cc

@@ -112,8 +112,8 @@ TEST(D2QueueMgrBasicTest, basicQueueTests) {
     // Construct the manager with max queue size set to number of messages
     // we'll use.
     D2QueueMgrPtr queue_mgr;
-    EXPECT_NO_THROW(queue_mgr.reset(new D2QueueMgr(io_service, VALID_MSG_CNT)));
-    EXPECT_EQ(VALID_MSG_CNT, queue_mgr->getMaxQueueSize());
+    ASSERT_NO_THROW(queue_mgr.reset(new D2QueueMgr(io_service, VALID_MSG_CNT)));
+    ASSERT_EQ(VALID_MSG_CNT, queue_mgr->getMaxQueueSize());
 
     // Verify queue is empty after construction.
     EXPECT_EQ(0, queue_mgr->getQueueSize());
@@ -150,13 +150,13 @@ TEST(D2QueueMgrBasicTest, basicQueueTests) {
         EXPECT_TRUE (*(ref_msgs[i]) == *ncr);
 
         // Verify that peek did not alter the queue size.
-        EXPECT_EQ(VALID_MSG_CNT-(i), queue_mgr->getQueueSize());
+        EXPECT_EQ(VALID_MSG_CNT - i, queue_mgr->getQueueSize());
 
         // Verify the dequeueing from non-empty queue works
         EXPECT_NO_THROW(queue_mgr->dequeue());
 
         // Verify queue size decrements following dequeue.
-        EXPECT_EQ(VALID_MSG_CNT-(i+1), queue_mgr->getQueueSize());
+        EXPECT_EQ(VALID_MSG_CNT - (i + 1), queue_mgr->getQueueSize());
     }
 
     // Iterate over the list of requests and add each to the queue.
@@ -254,9 +254,9 @@ public:
 /// 5. Starting listener from INITTED transitions to RUNNING.
 /// 6. Stopping the  listener transitions from RUNNING to STOPPED.
 /// 7. Starting listener from STOPPED transitions to RUNNING.
-TEST_F (QueueMgrUDPTest, stateModelTest) {
+TEST_F (QueueMgrUDPTest, stateModel) {
     // Create the queue manager.
-    EXPECT_NO_THROW(queue_mgr_.reset(new D2QueueMgr(io_service_,
+    ASSERT_NO_THROW(queue_mgr_.reset(new D2QueueMgr(io_service_,
                                      VALID_MSG_CNT)));
 
     // Verify that the initial state is NOT_INITTED.
@@ -316,7 +316,7 @@ TEST_F (QueueMgrUDPTest, stateModelTest) {
 /// 6. Requests can be received and queued normally after the queue
 /// has been emptied.
 /// 7. setQueueMax disallows values of 0 or less than current queue size.
-TEST_F (QueueMgrUDPTest, liveFeedTest) {
+TEST_F (QueueMgrUDPTest, liveFeed) {
     NameChangeRequestPtr send_ncr;
     NameChangeRequestPtr received_ncr;