[#1818] fixed doxygen and formatting

src/hooks/dhcp/high_availability/ha_messages.cc

@@ -185,11 +185,11 @@ const char* values[] = {
     "HA_MAINTENANCE_STARTED_IN_PARTNER_DOWN", "the server is now in the partner-down mode as a result of requested maintenance",
     "HA_MAINTENANCE_START_HANDLER_FAILED", "ha-maintenance-start command failed: %1",
     "HA_MISSING_CONFIGURATION", "high-availability parameter not specified for High Availability hooks library",
-    "HA_PAUSE_CLIENT_LISTENER_FAILED", "Pausing mutli-threaded HTTP processing failed: %1",
+    "HA_PAUSE_CLIENT_LISTENER_FAILED", "Pausing multi-threaded HTTP processing failed: %1",
     "HA_RESET_COMMUNICATIONS_FAILED", "failed to send ha-reset command to %1: %2",
     "HA_RESET_FAILED", "failed to reset HA state machine of %1: %2",
     "HA_RESET_HANDLER_FAILED", "ha-reset command failed: %1",
-    "HA_RESUME_CLIENT_LISTENER_FAILED", "Resuming mutli-threaded HTTP processing failed: %1",
+    "HA_RESUME_CLIENT_LISTENER_FAILED", "Resuming multi-threaded HTTP processing failed: %1",
     "HA_SCOPES_HANDLER_FAILED", "ha-scopes command failed: %1",
     "HA_SERVICE_STARTED", "started high availability service in %1 mode as %2 server",
     "HA_STATE_MACHINE_CONTINUED", "state machine is un-paused",

src/hooks/dhcp/high_availability/ha_messages.mes

@@ -481,9 +481,9 @@ This error message is issued to indicate that the configuration for the
 High Availability hooks library hasn't been specified. The 'high-availability'
 parameter must be specified for the hooks library to load properly.
 
-% HA_PAUSE_CLIENT_LISTENER_FAILED Pausing mutli-threaded HTTP processing failed: %1
+% HA_PAUSE_CLIENT_LISTENER_FAILED Pausing multi-threaded HTTP processing failed: %1
-This error message is emitted when an attempting to pause HA's HTTP client and
+This error message is emitted when attempting to pause HA's HTTP client and
-the listener threads. This error is highly unlikely and indicates a programmatic
+listener threads. This error is highly unlikely and indicates a programmatic
 issue that should be reported as a defect.
 
 % HA_RESET_COMMUNICATIONS_FAILED failed to send ha-reset command to %1: %2
@@ -501,9 +501,9 @@ This error message is issued to indicate that the ha-reset command handler
 failed while processing the command. The argument provides the reason for
 failure.
 
-% HA_RESUME_CLIENT_LISTENER_FAILED Resuming mutli-threaded HTTP processing failed: %1
+% HA_RESUME_CLIENT_LISTENER_FAILED Resuming multi-threaded HTTP processing failed: %1
-This error message is emitted when an attempting to resume HA's HTTP client and
+This error message is emitted when attempting to resume HA's HTTP client and
-the listener threads.  This is unlikely to occur and indicates a programmatic
+listener threads. This error is highly unlikely and indicates a programmatic
 issue that should be reported as a defect.
 
 % HA_SCOPES_HANDLER_FAILED ha-scopes command failed: %1

src/hooks/dhcp/high_availability/ha_service.h

@@ -1004,7 +1004,7 @@ public:
 
     /// @brief Start the client and(or) listener instances.
     ///
-    /// When HA+Mt is enabled it starts the client's thread pool
+    /// When HA+MT is enabled it starts the client's thread pool
     /// and the dedicated listener thread pool, if the listener exists.
     /// It registers pauseClientAndListener() and resumeClientAndListener()
     /// as the MultiThreading critical section entry and exit callbacks,
@@ -1030,7 +1030,7 @@ public:
     /// @brief Stop the client and(or) listener instances.
     ///
     /// It unregisters the MultiThreading critical section callbacks,
-    /// closes all connections, and the stops the thread pools for the client
+    /// closes all connections and stops the thread pools for the client
     /// and listener, if they exist.
     void stopClientAndListener();
 

src/lib/config/tests/cmd_http_listener_unittests.cc

@@ -738,7 +738,7 @@ TEST_F(CmdHttpListenerTest, basics) {
     EXPECT_EQ(listener_->getPort(), port);
     EXPECT_EQ(listener_->getThreadPoolSize(), 1);
 
-    // It should not have an IOService, should not be listening
+    // It should not have an IOService, should not be listening and
     // should have no threads.
     ASSERT_FALSE(listener_->getThreadIOService());
     EXPECT_TRUE(listener_->isStopped());
@@ -763,7 +763,6 @@ TEST_F(CmdHttpListenerTest, basics) {
     EXPECT_EQ(listener_->getThreadCount(), 1);
     ASSERT_TRUE(listener_->getThreadIOService());
     EXPECT_FALSE(listener_->getThreadIOService()->stopped());
-    EXPECT_TRUE(listener_->isRunning());
 
     // Trying to start it again should fail.
     ASSERT_THROW_MSG(listener_->start(), InvalidOperation,
@@ -773,7 +772,6 @@ TEST_F(CmdHttpListenerTest, basics) {
     ASSERT_NO_THROW_LOG(listener_->stop());
     ASSERT_TRUE(listener_->isStopped());
     EXPECT_EQ(listener_->getThreadCount(), 0);
-    EXPECT_TRUE(listener_->isStopped());
     ASSERT_FALSE(listener_->getThreadIOService());
 
     // Make sure we can call stop again without problems.
@@ -795,6 +793,7 @@ TEST_F(CmdHttpListenerTest, basics) {
     ASSERT_NO_THROW_LOG(listener_->start());
     EXPECT_EQ(listener_->getAddress(), address);
     EXPECT_EQ(listener_->getPort(), port);
+    EXPECT_EQ(listener_->getThreadCount(), 4);
     EXPECT_EQ(listener_->getThreadPoolSize(), 4);
     ASSERT_TRUE(listener_->isRunning());
     ASSERT_TRUE(listener_->getThreadIOService());
@@ -805,6 +804,7 @@ TEST_F(CmdHttpListenerTest, basics) {
     ASSERT_NO_THROW_LOG(listener_->pause());
     ASSERT_TRUE(listener_->isPaused());
     EXPECT_EQ(listener_->getThreadCount(), 4);
+    EXPECT_EQ(listener_->getThreadPoolSize(), 4);
     ASSERT_TRUE(listener_->getThreadIOService());
     EXPECT_TRUE(listener_->getThreadIOService()->stopped());
 
@@ -812,6 +812,7 @@ TEST_F(CmdHttpListenerTest, basics) {
     ASSERT_NO_THROW_LOG(listener_->resume());
     ASSERT_TRUE(listener_->isRunning());
     EXPECT_EQ(listener_->getThreadCount(), 4);
+    EXPECT_EQ(listener_->getThreadPoolSize(), 4);
     ASSERT_TRUE(listener_->getThreadIOService());
     EXPECT_FALSE(listener_->getThreadIOService()->stopped());
 
@@ -819,6 +820,7 @@ TEST_F(CmdHttpListenerTest, basics) {
     ASSERT_NO_THROW_LOG(listener_->stop());
     ASSERT_TRUE(listener_->isStopped());
     EXPECT_EQ(listener_->getThreadCount(), 0);
+    EXPECT_EQ(listener_->getThreadPoolSize(), 4);
     ASSERT_FALSE(listener_->getThreadIOService());
     EXPECT_TRUE(listener_->isStopped());
 }

src/lib/http/client.cc

@@ -1739,13 +1739,13 @@ public:
     /// @param io_service IOService that will drive connection IO in single
     /// threaded mode.  (Currently ignored in multi-threaded mode)
     /// @param thread_pool_size maximum number of concurrent threads
-    /// Internally this also sets the maximum number concurrent connections
+    /// Internally this also sets the maximum number of concurrent connections
     /// per URL.
-    /// @param defer_thread_start When true, creation of the pool threads is
+    /// @param defer_thread_start When true, starting of the pool threads is
     /// deferred until a subsequent call to @ref start(). In this case the
-    /// pool's operational state post-construction is STOPPED.  Otherwise,
+    /// pool's operational state after construction is STOPPED.  Otherwise,
-    /// the thread pool threads will be created and started, with the post-
+    /// the thread pool threads will be created and started, with the
-    /// construction state being RUNNING.  Applicable only when thread-pool size
+    /// operational state being RUNNING.  Applicable only when thread-pool size
     /// is greater than zero.
     HttpClientImpl(IOService& io_service, size_t thread_pool_size = 0,
                    bool defer_thread_start = false)

src/lib/http/client.h

@@ -136,8 +136,6 @@ public:
     ///
     /// @param io_service IO service to be used by the HTTP client.
     /// @param thread_pool_size maximum number of threads in the thread pool.
-    /// @param defer_thread_start if true, the thread pool will be created but
-    /// not started. Applicable only when thread-pool-size is greater than zero.
     /// A value greater than zero enables multi-threaded mode and sets the
     /// maximum number of concurrent connections per URL.  A value of zero
     /// (default) enables single-threaded mode with one connection per URL.

src/lib/http/http_thread_pool.h

@@ -187,7 +187,7 @@ public:
     uint16_t getThreadCount() const;
 
 private:
-    /// @brief Maxim number of threads in the thread pool.
+    /// @brief Maximum number of threads in the thread pool.
     size_t pool_size_;
 
     /// @brief Pointer to private IOService used in multi-threaded mode.

src/lib/http/tests/Makefile.am

@@ -48,7 +48,7 @@ libhttp_unittests_SOURCES += tls_client_unittests.cc
 endif
 libhttp_unittests_SOURCES += url_unittests.cc
 libhttp_unittests_SOURCES += test_http_client.h
-libhttp_unittests_SOURCES += mt_client_unittests.cc
+libhttp_unittests_SOURCES += client_mt_unittests.cc
 libhttp_unittests_SOURCES += http_thread_pool_unittests.cc
 
 libhttp_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)

src/lib/http/tests/client_mt_unittests.cc

@@ -937,6 +937,8 @@ TEST_F(MtHttpClientTest, restartAfterStop) {
 
     // Starting again should succeed.
     ASSERT_NO_THROW_LOG(client->start());
+
+    // Verify we didn't break it.
     ASSERT_EQ(client->getThreadCount(), 3);
     ASSERT_TRUE(client->getThreadIOService());
     ASSERT_FALSE(client->getThreadIOService()->stopped());

src/lib/http/tests/http_thread_pool_unittests.cc

@@ -57,9 +57,9 @@ TEST_F(HttpThreadPoolTest, deferredStartConstruction) {
     ASSERT_NO_THROW_LOG(pool.reset(new HttpThreadPool(io_service_, 3, true)));
 
     // State should be stopped.
-    // Pool size should be 3
+    // Pool size should be 3.
     // IOService should be there.
-    // IOService is new, so it should not be stopped,
+    // IOService is new, so it should not be stopped.
     // No threads in the pool.
     ASSERT_TRUE(pool->isStopped());
     EXPECT_EQ(pool->getPoolSize(), 3);
@@ -77,8 +77,11 @@ TEST_F(HttpThreadPoolTest, startDuringConstruction) {
 
     ASSERT_NO_THROW_LOG(pool.reset(new HttpThreadPool(io_service_, 3)));
 
-    // Pool size should be 3, state should be RUNNING, IOService should
+    // State should be running.
-    // set but not stopped, and we should have 3 threads in the pool.
+    // Pool size should be 3.
+    // IOService should be there.
+    // IOService is new, so it should not be stopped.
+    // Should have 3 threads in the pool.
     ASSERT_TRUE(pool->isRunning());
     EXPECT_EQ(pool->getPoolSize(), 3);
     ASSERT_TRUE(pool->getIOService());

src/lib/util/multi_threading_mgr.cc

@@ -125,7 +125,7 @@ MultiThreadingMgr::stopProcessing() {
             thread_pool_.stop();
         }
 
-        for (auto cb : critical_entry_cbs_.getCallbacks() ) {
+        for (const auto& cb : critical_entry_cbs_.getCallbacks()) {
             try {
                 (cb.callback_)();
             } catch (...) {
@@ -144,8 +144,8 @@ MultiThreadingMgr::startProcessing() {
             thread_pool_.start(getThreadPoolSize());
         }
 
-        for (auto cb : critical_exit_cbs_.getCallbacks() ) {
+        for (const auto& cb : critical_exit_cbs_.getCallbacks()) {
-           try {
+            try {
                 (cb.callback_)();
             } catch (...) {
                 // We can't log it and throwing could be chaos.

src/lib/util/multi_threading_mgr.h

@@ -139,7 +139,7 @@ public:
     /// @param entry_cb Callback to invoke upon CriticalSection entry. Cannot be
     /// empty.
     /// @param exit_cb Callback to invoke upon CriticalSection exit. Cannot be
-    /// be empty.
+    /// empty.
     void addCriticalSectionCallbacks(const std::string& name,
                                      const NamedCallback::Callback& entry_cb,
                                      const NamedCallback::Callback& exit_cb);

src/lib/util/named_callback.cc

@@ -19,7 +19,7 @@ void
 NamedCallbackList::addCallback(const std::string& name, const NamedCallback::Callback& cb) {
     if (!cb) {
         isc_throw(BadValue, "NamedCallbackList - callback: " << name
-                      << ", cannot be empty");
+                  << ", cannot be empty");
     }
 
     for (auto const& callback : callbacks_) {
@@ -34,13 +34,11 @@ NamedCallbackList::addCallback(const std::string& name, const NamedCallback::Cal
 
 void
 NamedCallbackList::removeCallback(const std::string& name) {
-    for (auto it = callbacks_.begin(); it != callbacks_.end(); ) {
+    for (auto it = callbacks_.begin(); it != callbacks_.end(); ++it) {
         if ((*it).name_ == name) {
-            it = callbacks_.erase(it);
+            callbacks_.erase(it);
             break;
         }
-
-        ++it;
     }
 }
 

src/lib/util/named_callback.h

@@ -27,13 +27,12 @@ struct NamedCallback {
     /// @param name Name by which the callback can be found.
     /// @param cb Callback associated with name.
     NamedCallback(const std::string& name, const Callback& cb)
-        : name_(name), callback_(cb) {
+        : name_(name), callback_(cb) {}
-    };
 
-    /// @Brief Name by which the callback can be found.
+    /// @brief Name by which the callback can be found.
     std::string name_;
 
-    /// @Brief Callback associated with name.
+    /// @brief Callback associated with name.
     Callback callback_;
 };
 
@@ -46,7 +45,7 @@ struct NamedCallback {
 class NamedCallbackList {
 public:
     /// @brief Constructor.
-    NamedCallbackList(){};
+    NamedCallbackList() {}
 
     /// @brief Adds a callback to the list.
     ///
@@ -70,7 +69,7 @@ public:
 
 private:
     /// @brief The list of callbacks.
-    std::list<NamedCallback>  callbacks_;
+    std::list<NamedCallback> callbacks_;
 };
 
 }  // namespace util

src/lib/util/tests/multi_threading_mgr_unittest.cc

@@ -326,25 +326,25 @@ TEST(MultiThreadingMgrTest, criticalSection) {
 /// @brief Test fixture for exercised CriticalSection callbacks.
 class CriticalSectionCallbackTest : public ::testing::Test {
 public:
-    /// @Brief Constructor.
+    /// @brief Constructor.
-    CriticalSectionCallbackTest() {};
+    CriticalSectionCallbackTest() {}
 
-    /// @Brief A callback that adds the value, 1, to invocations lists.
+    /// @brief A callback that adds the value, 1, to invocations lists.
     void one() {
         invocations_.push_back(1);
     }
 
-    /// @Brief A callback that adds the value, 2, to invocations lists.
+    /// @brief A callback that adds the value, 2, to invocations lists.
     void two() {
         invocations_.push_back(2);
     }
 
-    /// @Brief A callback that adds the value, 3, to invocations lists.
+    /// @brief A callback that adds the value, 3, to invocations lists.
     void three() {
         invocations_.push_back(3);
     }
 
-    /// @Brief A callback that adds the value, 4, to invocations lists.
+    /// @brief A callback that adds the value, 4, to invocations lists.
     void four() {
         invocations_.push_back(4);
     }
@@ -357,16 +357,16 @@ public:
     }
 
     /// @brief Checks callback invocations over a series of nested
-    /// CriticalSecitons.
+    /// CriticalSections.
     ///
     /// @param entries A vector of the invocation values that should
     /// be present after entry into the outermost CriticalSection.  The
-    /// expected  values should be in the order the callbacks were
+    /// expected values should be in the order the callbacks were added
-    /// added to the  MultiThreadingMgr's list of callbacks.
+    /// to the MultiThreadingMgr's list of callbacks.
     /// @param exits A vector of the invocation values that should
-    /// be present after exiting the CriticalSection.  The expected
+    /// be present after exiting the outermost CriticalSection.  The
-    /// values should be in the order the callbacks were added to the
+    /// expected values should be in the order the callbacks were added
-    /// MultiThreadingMgr's list of callbacks.
+    /// to the MultiThreadingMgr's list of callbacks.
     void runCriticalSections(std::vector<int> entries, std::vector<int>exits) {
         // Pool must be running.
         ASSERT_TRUE(isThreadPoolRunning());
@@ -384,7 +384,7 @@ public:
 
             if (entries.size()) {
                 // We expect entry invocations.
-                ASSERT_TRUE(invocations_ ==  entries);
+                ASSERT_TRUE(invocations_ == entries);
             } else {
                 // We do not expect entry invocations.
                 ASSERT_FALSE(invocations_.size());
@@ -397,14 +397,14 @@ public:
                 // Enter another CriticalSection.
                 MultiThreadingCriticalSection inner_cs;
 
-                // thread pool should still be stopped
+                // Thread pool should still be stopped
                 ASSERT_FALSE(isThreadPoolRunning());
 
                 // We should not have had any callback invocations.
                 ASSERT_FALSE(invocations_.size());
             }
 
-            // After exiting inner setion, the thread pool should
+            // After exiting inner section, the thread pool should
             // still be stopped
             ASSERT_FALSE(isThreadPoolRunning());
 
@@ -412,20 +412,20 @@ public:
             ASSERT_FALSE(invocations_.size());
         }
 
-        // After exiting the outer setion, the thread pool should
+        // After exiting the outer section, the thread pool should
         // match the thread count.
         ASSERT_TRUE(isThreadPoolRunning());
 
         if (exits.size()) {
             // We expect exit invocations.
-            ASSERT_TRUE(invocations_ ==  exits);
+            ASSERT_TRUE(invocations_ == exits);
         } else {
             // We do not expect exit invocations.
             ASSERT_FALSE(invocations_.size());
         }
     }
 
-    /// @Brief A list of values set by callback invocations.
+    /// @brief A list of values set by callback invocations.
     std::vector<int> invocations_;
 };
 

src/lib/util/tests/named_callback_unittest.cc

@@ -20,28 +20,36 @@ namespace {
 
 class NamedCallbackListTest : public ::testing::Test {
 public:
-    NamedCallbackListTest() {};
+    /// @brief Constructor.
+    NamedCallbackListTest() {}
 
+    /// @brief A callback that adds the value, 1, to invocations lists.
     void one() {
         invocations_.push_back(1);
     }
 
+    /// @brief A callback that adds the value, 2, to invocations lists.
     void two() {
         invocations_.push_back(2);
     }
 
+    /// @brief A callback that adds the value, 3, to invocations lists.
     void three() {
         invocations_.push_back(3);
     }
 
+    /// @brief Run all callbacks.
     void runCallbacks() {
         invocations_.clear();
-        for (auto cb : cbs_.getCallbacks() ) {
+        for (const auto& cb : cbs_.getCallbacks()) {
             ASSERT_NO_THROW((cb.callback_)());
         }
     }
 
+    /// @brief A list of callbacks.
     NamedCallbackList cbs_;
+
+    /// @brief A list of values set by callback invocations.
     std::vector<int> invocations_;
 };
 
@@ -63,15 +71,6 @@ TEST_F(NamedCallbackListTest, basics) {
                                       std::bind(&NamedCallbackListTest::one, this)),
                      BadValue, "NamedCallbackList - callback: one, already exists");
 
-    for (auto cb : cbs_.getCallbacks() ) {
-        ASSERT_NO_THROW((cb.callback_)());
-    }
-
-    int i = 0;
-    for (auto invocation : invocations_) {
-        EXPECT_EQ(invocation, ++i);
-    }
-
     runCallbacks();
 
     EXPECT_EQ(3, invocations_.size());