[2980] Miscellaneous tidy-up

src/lib/hooks/callout_manager.cc

@@ -31,6 +31,10 @@ namespace hooks {
 
 void
 CalloutManager::registerCallout(const std::string& name, CalloutPtr callout) {
+    // Note the registration.
+    LOG_DEBUG(hooks_logger, HOOKS_DBG_CALLS, HOOKS_REGISTER_CALLOUT)
+        .arg(current_library_).arg(name);
+
     // Sanity check that the current library index is set to a valid value.
     checkLibraryIndex(current_library_);
 
@@ -48,8 +52,6 @@ CalloutManager::registerCallout(const std::string& name, CalloutPtr callout) {
             // current index, so insert the new element ahead of this one.
             hook_vector_[hook_index].insert(i, make_pair(current_library_,
                                                          callout));
-            LOG_DEBUG(hooks_logger, HOOKS_DBG_CALLS, HOOKS_REGISTER_CALLOUT)
-                .arg(current_library_).arg(name);
             return;
         }
     }

src/lib/hooks/hooks_messages.mes

@@ -21,30 +21,30 @@ index (in the list of loaded libraries) has been called and returned a
 success state.  The address of the callout is given in the message
 
 % HOOKS_CALLOUT_ERROR error returned by callout on hook %1 registered by library with index %2 (callout address %3)
-If a callout returns an error status when called, this warning message is
-issues.  It identifies the hook to which the callout is attached, and the
-index of the library (in the list of loaded libraries) that registered it.
-The address of the callout is also given.
+If a callout returns an error status when called, this warning message
+is issues.  It identifies the hook to which the callout is attached,
+and the index of the library (in the list of loaded libraries) that
+registered it.  The address of the callout is also given.
 
 % HOOKS_CALLOUT_REMOVED callout removed from hook %1 for library %2
-This is a debug message issued during library unloading.  It notes that one
-of more callouts registered by that library have been removed from the
-specified hook.
+This is a debug message issued during library unloading.  It notes that
+one of more callouts registered by that library have been removed from
+the specified hook.
 
 % HOOKS_CLOSE_ERROR failed to close hook library %1: %2
 BIND 10 has failed to close the named hook library for the stated reason.
-Although this is an error, this should not affect the running system other
-than as a loss of resources.  If this error persists, you should restart
-BIND 10.
+Although this is an error, this should not affect the running system
+other than as a loss of resources.  If this error persists, you should
+restart BIND 10.
 
 % HOOKS_DEREGISTER_ALL_CALLOUTS hook library at index %1 deregistered all callouts on hook %2
 A debug message issued when all callouts on the specified hook registered
 by the library with the given index were removed.
 
 % HOOKS_DEREGISTER_CALLOUT hook library at index %1 deregistered a callout on hook %2
-A debug message issued when all instances of a particular callouts on the
-hook identified in the message that were registered by the library with the
-given index were removed.
+A debug message issued when all instances of a particular callouts on
+the hook identified in the message that were registered by the library
+with the given index were removed.
 
 % HOOKS_INCORRECT_VERSION hook library %1 is at version %2, require version %3
 BIND 10 has detected that the named hook library has been built against
@@ -52,74 +52,86 @@ a version of BIND 10 that is incompatible with the version of BIND 10
 running on your system.  It has not loaded the library.
 
 This is most likely due to the installation of a new version of BIND 10
-without rebuilding the hook library.  A rebuild and re-install of the library
-should fix the problem in most cases.
+without rebuilding the hook library.  A rebuild and re-install of the
+library should fix the problem in most cases.
 
 % HOOKS_LIBRARY_LOADED hooks library %1 successfully loaded
-This information message is issued when a user-supplied hooks library has been
-successfully loaded.
+This information message is issued when a user-supplied hooks library
+has been successfully loaded.
 
 % HOOKS_LIBRARY_UNLOADED hooks library %1 successfully unloaded
-This information message is issued when a user-supplied hooks library has been
-successfully unloaded.
+This information message is issued when a user-supplied hooks library
+has been successfully unloaded.
 
 % HOOKS_LIBRARY_VERSION hooks library %1 reports its version as %2
-A debug  message issued when the version check on the hooks library has
-succeeded.
+A debug  message issued when the version check on the hooks library
+has succeeded.
 
 % HOOKS_LOAD 'load' function in hook library %1 returned success
-This is a debug message issued when the "load" function has been found in a
-hook library and has been successfully called.
+This is a debug message issued when the "load" function has been found
+in a hook library and has been successfully called.
 
 % HOOKS_LOAD_ERROR 'load' function in hook library %1 returned error %2
-A "load" function was found in the library named in the message and was
-called.  The function returned a non-zero status (also given in the message)
-which was interpreted as an error.  The library has been unloaded and
-no callouts from it will be installed.
+A "load" function was found in the library named in the message and
+was called.  The function returned a non-zero status (also given in
+the message) which was interpreted as an error.  The library has been
+unloaded and no callouts from it will be installed.
 
 % HOOKS_LOAD_LIBRARY loading hooks library %1
 This is a debug message called when the specified library is being loaded.
 
 % HOOKS_NO_LOAD no 'load' function found in hook library %1
-This is a debug message saying that the specified library was loaded but
-no function called "load" was found in it.  Providing the library contained
-some "standard" functions (i.e. functions with the names of the hooks for
-the given server), this is not an issue.
+This is a debug message saying that the specified library was loaded
+but no function called "load" was found in it.  Providing the library
+contained some "standard" functions (i.e. functions with the names of
+the hooks for the given server), this is not an issue.
 
 % HOOKS_NO_UNLOAD no 'unload' function found in hook library %1
-This is a debug message issued when the library is being unloaded.  It merely
-states that the library did not contain an "unload" function.
+This is a debug message issued when the library is being unloaded.
+It merely states that the library did not contain an "unload" function.
 
 % HOOKS_NO_VERSION no 'version' function found in hook library %1
-The shared library named in the message was found and successfully loaded, but
-BIND 10 did not find a function named "version" in it.  This function is
-required and should return the version of BIND 10 against which the library
-was built.  The value is used to check that the library was built against a
-compatible version of BIND 10.  The library has not been loaded.
+The shared library named in the message was found and successfully loaded,
+but BIND 10 did not find a function named "version" in it.  This function
+is required and should return the version of BIND 10 against which the
+library was built.  The value is used to check that the library was built
+against a compatible version of BIND 10.  The library has not been loaded.
 
 % HOOKS_OPEN_ERROR failed to open hook library %1: %2
-BIND 10 failed to open the specified hook library for the stated reason. The
-library has not been loaded.  BIND 10 will continue to function, but without
-the services offered by the library.
+BIND 10 failed to open the specified hook library for the stated
+reason. The library has not been loaded.  BIND 10 will continue to
+function, but without the services offered by the library.
 
 % HOOKS_REGISTER_CALLOUT hooks library with index %1 registered callout for hook '%2'
-This is a debug message, output when a library (whose index in the list of
-libraries (being) loaded is given) registers a callout.
+This is a debug message, output when a library (whose index in the list
+of libraries (being) loaded is given) registers a callout.
+
+% HOOKS_REGISTER_HOOK hook %1 was registered
+This is a debug message indicating that a hook of the specified name
+was registered by a BIND 10 server.  The server doing the logging is
+indicated by the full name of the logger used to log this message.
 
 % HOOKS_REGISTER_STD_CALLOUT hooks library %1 registered standard callout for hook %2
-This is a debug message, output when the library loading function has located
-a standard callout (a callout with the same name as a hook point) and
-registered it.
+This is a debug message, output when the library loading function has
+located a standard callout (a callout with the same name as a hook point)
+and registered it.
+
+% HOOKS_RESET_HOOK_LIST the list of hooks has been reset
+This is a message indicating that the list of hooks has been reset.
+While this is usual when running the BIND 10 test suite, it should not be
+seen when running BIND 10 in a producion environment.  If this appears,
+please report a bug through the usual channels.
 
 % HOOKS_UNLOAD 'unload' function in hook library %1 returned success
-This is a debug message issued when an "unload" function has been found in a
-hook library during the unload process, called, and returned success.
+This is a debug message issued when an "unload" function has been found
+in a hook library during the unload process, called, and returned success.
 
 % HOOKS_UNLOAD_ERROR 'unload' function in hook library %1 returned error %2
-During the unloading of a library, an "unload" function was found.  It was
-called, but returned an error (non-zero) status, resulting in the issuing of
-this message.  The unload process continued after this message and the library
-has been unloaded.
+During the unloading of a library, an "unload" function was found.
+It was called, but returned an error (non-zero) status, resulting in
+the issuing of this message.  The unload process continued after this
+message and the library has been unloaded.
 
 % HOOKS_UNLOAD_LIBRARY unloading library %1
-This is a debug message called when the specified library is being unloaded.
+This is a debug message called when the specified library is being
+unloaded.

src/lib/hooks/library_manager.h

@@ -43,10 +43,10 @@ class LibraryManager;
 ///       process, the library may allocate memory and pass it back to the
 ///       server.  This could happen by the server setting arguments or context
 ///       in the CalloutHandle object, or by the library modifying the content
-///       of pointed-to data. A problem arises when the library is unloaded,
-///       because the addresses of allocated day may lie in the virtual
-///       address space deleted in that process.  If this happens, any
-///       reference to the memory will cause a segmentation fault.  This can
+///       of pointed-to data. If the library is unloaded, this memory may lie
+///       in the virtual address space deleted in that process. (The word "may"
+///       is used, as this could be operating-system specific.) If this happens,
+///       any reference to the memory will cause a segmentation fault.  This can
 ///       occur in a quite obscure place, for example in the middle of a
 ///       destructor of an STL class when it is deleting memory allocated
 ///       when the data structure was extended.

src/lib/hooks/server_hooks.cc

@@ -13,6 +13,7 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <exceptions/exceptions.h>
+#include <hooks/hooks_log.h>
 #include <hooks/server_hooks.h>
 
 #include <utility>
@@ -53,6 +54,9 @@ ServerHooks::registerHook(const string& name) {
     // Element was inserted, so add to the inverse hooks collection.
     inverse_hooks_[index] = name;
 
+    // Log it if debug is enabled
+    LOG_DEBUG(hooks_logger, HOOKS_DBG_TRACE, HOOKS_REGISTER_HOOK).arg(name);
+
     // ... and return numeric index.
     return (index);
 }
@@ -61,6 +65,10 @@ ServerHooks::registerHook(const string& name) {
 
 void
 ServerHooks::reset() {
+    // Log a wanring - although this is done during testing, it should never be
+    // seen in a production system.
+    LOG_WARN(hooks_logger, HOOKS_RESET_HOOK_LIST);
+
     // Clear out the name->index and index->name maps.
     hooks_.clear();
     inverse_hooks_.clear();

src/lib/hooks/tests/basic_callout_library.cc

@@ -13,12 +13,11 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 /// @file
-/// @brief Basic Callout Library
+/// @brief Basic callout library
 ///
-/// This is a test file for the LibraryManager test.  It produces a library
-/// that allows for tests of the basic library manager functions.
-///
-/// The characteristics of this library are:
+/// This is source of a test library for various test (LibraryManager and
+/// HooksManager).  The characteristics of the library produced from this
+/// file are:
 ///
 /// - Only the "version" framework function is supplied.
 ///
@@ -32,10 +31,9 @@
 ///
 ///   @f[ (10 + data_1) * data_2 - data_3 @f]
 ///
-///   ...where data_1, data_2 and data_3 are the values passed in arguments
-///   of the same name to the three callouts (data_1 passed to lm_one,
-///   data_2 to lm_two etc.) and the result is returned in the argument
-///   "result".
+///   ...where data_1, data_2 and data_3 are the values passed in arguments of
+///   the same name to the three callouts (data_1 passed to lm_one, data_2 to
+///   lm_two etc.) and the result is returned in the argument "result".
 
 #include <hooks/hooks.h>
 #include <fstream>
@@ -45,7 +43,7 @@ using namespace std;
 
 extern "C" {
 
-// Callouts
+// Callouts.  All return their result through the "result" argument.
 
 int
 context_create(CalloutHandle& handle) {
@@ -55,7 +53,9 @@ context_create(CalloutHandle& handle) {
 }
 
 // First callout adds the passed "data_1" argument to the initialized context
-// value of 10.
+// value of 10. (Note that the value set by context_create is accessed through
+// context and not the argument, so checking that context is correctly passed
+// between callouts in the same library.)
 
 int
 lm_one(CalloutHandle& handle) {
@@ -88,7 +88,7 @@ lm_two(CalloutHandle& handle) {
     return (0);
 }
 
-// Final callout subtracts the result in "data_3" and.
+// Final callout subtracts the result in "data_3".
 
 int
 lm_three(CalloutHandle& handle) {
@@ -104,7 +104,7 @@ lm_three(CalloutHandle& handle) {
     return (0);
 }
 
-// Framework functions
+// Framework functions.  Only version() is supplied here.
 
 int
 version() {

src/lib/hooks/tests/incorrect_version_library.cc

@@ -14,9 +14,13 @@
 
 /// @file
 /// @brief Incorrect version function test
-/// This is a test file for the LibraryManager test.  It produces a library
-/// that contans a "version" function but which returns an incorrect version
-/// number.
+///
+/// This is source of a test library for various test (LibraryManager and
+/// HooksManager).  The characteristics of the library produced from this
+/// file are:
+///
+/// - It contains the version() framework function only, which returns an
+///   incorrect version number.
 
 #include <hooks/hooks.h>
 

src/lib/hooks/tests/library_manager_unittest.cc.in

@@ -23,7 +23,6 @@
 #include <algorithm>
 #include <fstream>
 #include <string>
-#include <iostream>
 
 #include <unistd.h>
 
@@ -32,6 +31,8 @@ using namespace isc;
 using namespace isc::hooks;
 using namespace std;
 
+namespace {
+
 /// @brief Library manager test class
 
 class LibraryManagerTest : public ::testing::Test {
@@ -41,8 +42,8 @@ public:
     /// Sets up a collection of three LibraryHandle objects to use in the test.
     LibraryManagerTest() {
 
-        // Set up the server hooks.  There is sone singleton for all tests,
-        // so reset it and explicitly set up the hooks for the test.
+        // Set up the server hooks.  ServerHooks is a singleton, so we reset it
+        // between each test.
         ServerHooks& hooks = ServerHooks::getServerHooks();
         hooks.reset();
         lm_one_index_ = hooks.registerHook("lm_one");
@@ -53,10 +54,7 @@ public:
         // four libraries.
         callout_manager_.reset(new CalloutManager(4));
 
-        // Set up the callout handle.
-        callout_handle_.reset(new CalloutHandle(callout_manager_));
-
-        // Ensure the marker file is not present
+        // Ensure the marker file is not present at the start of a test.
         static_cast<void>(unlink(MARKER_FILE));
     }
 
@@ -69,15 +67,17 @@ public:
 
     /// @brief Call callouts test
     ///
-    /// All of the loaded libraries have four callouts: a context_create
-    /// callout and three callouts that are attached to hooks lm_one,
-    /// lm_two and lm_three.
+    /// All of the loaded libraries for which callouts are called register four
+    /// callouts: a context_create callout and three callouts that are attached
+    /// to hooks lm_one, lm_two and lm_three.  These four callouts, executed
+    /// in sequence, perform a series of calculations. Data is passed between
+    /// callouts in the argument list, in a variable named "result".
     ///
     /// context_create initializes the calculation by setting a seed
-    /// value of r0 say.
+    /// value, called r0 here.
     ///
     /// Callout lm_one is passed a value d1 and performs a simple arithmetic
-    /// operation on it yielding a result r1.  Hence we can say that
+    /// operation on it and r0 yielding a result r1.  Hence we can say that
     /// @f[ r1 = lm1(r0, d1) @f]
     ///
     /// Callout lm_two is passed a value d2 and peforms another simple
@@ -89,54 +89,57 @@ public:
     /// The details of the operations lm1, lm2 and lm3 depend on the library.
     /// However the sequence of calls needed to do this set of calculations
     /// is identical regardless of the exact functions. This method performs
-    /// those operations and checks the results.
+    /// those operations and checks the results of each step.
     ///
-    /// It is assumed that callout_manager_ and callout_handle_ have been
-    /// set up appropriately.
+    /// It is assumed that callout_manager_ has been set up appropriately.
+    ///
+    /// @note The CalloutHandle used in the calls is declared locally here.
+    ///       The advantage of this (apart from scope reduction) is that on
+    ///       exit, it is destroyed.  This removes any references to memory
+    ///       allocated by loaded libraries while they are still loaded.
     ///
     /// @param r0...r3, d1..d3 Values and intermediate values expected.  They
-    ///        are ordered so that the variables are used in the order left to
-    ///        right.
+    ///        are ordered so that the variables appear in the argument list in
+    ///        the order they are used.
     void executeCallCallouts(int r0, int d1, int r1, int d2, int r2, int d3,
                              int r3) {
         static const char* COMMON_TEXT = " callout returned the wong value";
         int result;
 
+        // Set up a callout handle for the calls.
+        CalloutHandle callout_handle(callout_manager_);
+
         // Seed the calculation.
         callout_manager_->callCallouts(ServerHooks::CONTEXT_CREATE,
-                                       *callout_handle_);
-        callout_handle_->getArgument("result", result);
+                                       callout_handle);
+        callout_handle.getArgument("result", result);
         EXPECT_EQ(r0, result) << "context_create" << COMMON_TEXT;
 
         // Perform the first calculation.
-        callout_handle_->setArgument("data_1", d1);
-        callout_manager_->callCallouts(lm_one_index_, *callout_handle_);
-        callout_handle_->getArgument("result", result);
+        callout_handle.setArgument("data_1", d1);
+        callout_manager_->callCallouts(lm_one_index_, callout_handle);
+        callout_handle.getArgument("result", result);
         EXPECT_EQ(r1, result) << "lm_one" << COMMON_TEXT;
 
         // ... the second ...
-        callout_handle_->setArgument("data_2", d2);
-        callout_manager_->callCallouts(lm_two_index_, *callout_handle_);
-        callout_handle_->getArgument("result", result);
+        callout_handle.setArgument("data_2", d2);
+        callout_manager_->callCallouts(lm_two_index_, callout_handle);
+        callout_handle.getArgument("result", result);
         EXPECT_EQ(r2, result) << "lm_two" << COMMON_TEXT;
 
         // ... and the third.
-        callout_handle_->setArgument("data_3", d3);
-        callout_manager_->callCallouts(lm_three_index_, *callout_handle_);
-        callout_handle_->getArgument("result", result);
+        callout_handle.setArgument("data_3", d3);
+        callout_manager_->callCallouts(lm_three_index_, callout_handle);
+        callout_handle.getArgument("result", result);
         EXPECT_EQ(r3, result) << "lm_three" << COMMON_TEXT;
     }
 
-    /// Hook indexes.  These are somewhat ubiquitous, so are made public for
-    /// ease of reference instead of being accessible by a function.
+    /// Hook indexes.  These are are made public for ease of reference.
     int lm_one_index_;
     int lm_two_index_;
     int lm_three_index_;
 
-    /// Callout handle used in calls
-    boost::shared_ptr<CalloutHandle> callout_handle_;
-
-    /// Callout manager used for the test
+    /// Callout manager used for the test.
     boost::shared_ptr<CalloutManager> callout_manager_;
 };
 
@@ -162,13 +165,6 @@ public:
         : LibraryManager(name, index, manager)
     {}
 
-    /// @brief Destructor
-    ///
-    /// Ensures that the library is closed after the test.
-    ~PublicLibraryManager() {
-        static_cast<void>(closeLibrary());
-    }
-
     /// Public methods that call protected methods on the superclass.
     using LibraryManager::openLibrary;
     using LibraryManager::closeLibrary;
@@ -193,9 +189,8 @@ static const char* NO_VERSION_LIBRARY = "@abs_builddir@/.libs/libnvl.so";
 static const char* UNLOAD_CALLOUT_LIBRARY = "@abs_builddir@/.libs/libucl.so";
 
 
-namespace {
-
-// Tests that OpenLibrary reports an error for an unknown library.
+// Check that openLibrary() reports an error when it can't find the specified
+// library.
 
 TEST_F(LibraryManagerTest, NoLibrary) {
     PublicLibraryManager lib_manager(std::string(NOT_PRESENT_LIBRARY),
@@ -203,7 +198,7 @@ TEST_F(LibraryManagerTest, NoLibrary) {
     EXPECT_FALSE(lib_manager.openLibrary());
 }
 
-// Tests that the openLibrary() and closeLibrary() methods work.
+// Check that the openLibrary() and closeLibrary() methods work.
 
 TEST_F(LibraryManagerTest, OpenClose) {
     PublicLibraryManager lib_manager(std::string(BASIC_CALLOUT_LIBRARY),
@@ -213,7 +208,8 @@ TEST_F(LibraryManagerTest, OpenClose) {
     EXPECT_TRUE(lib_manager.openLibrary());
     EXPECT_TRUE(lib_manager.closeLibrary());
 
-    // Check that a second close does not report an error.
+    // Check that a second close on an already closed library does not report
+    // an error.
     EXPECT_TRUE(lib_manager.closeLibrary());
 }
 
@@ -286,10 +282,6 @@ TEST_F(LibraryManagerTest, RegisterStandardCallouts) {
     // r3 = (10 + d1) * d2 - d3
     executeCallCallouts(10, 5, 15, 7, 105, 17, 88);
 
-    // Explicitly clear the callout_handle_ so that we can delete the library.
-    // This is the only object to contain memory allocated by it.
-    callout_handle_.reset();
-
     // Tidy up
     EXPECT_TRUE(lib_manager.closeLibrary());
 }
@@ -310,8 +302,6 @@ TEST_F(LibraryManagerTest, CheckLoadCalled) {
     // Load the standard callouts
     EXPECT_NO_THROW(lib_manager.registerStandardCallouts());
 
-    int result = 0;
-
     // Check that only context_create and lm_one have callouts registered.
     EXPECT_TRUE(callout_manager_->calloutsPresent(
                 ServerHooks::CONTEXT_CREATE));
@@ -337,10 +327,6 @@ TEST_F(LibraryManagerTest, CheckLoadCalled) {
     // r3 = (5 * d1 + d2) * d3
     executeCallCallouts(5, 5, 25, 7, 32, 10, 320);
 
-    // Explicitly clear the callout_handle_ so that we can delete the library.
-    // This is the only object to contain memory allocated by it.
-    callout_handle_.reset();
-
     // Tidy up
     EXPECT_TRUE(lib_manager.closeLibrary());
 }
@@ -358,10 +344,6 @@ TEST_F(LibraryManagerTest, CheckLoadError) {
     // Check that we catch a load error
     EXPECT_FALSE(lib_manager.runLoad());
 
-    // Explicitly clear the callout_handle_ so that we can delete the library.
-    // This is the only object to contain memory allocated by it.
-    callout_handle_.reset();
-
     // Tidy up
     EXPECT_TRUE(lib_manager.closeLibrary());
 }
@@ -379,10 +361,6 @@ TEST_F(LibraryManagerTest, CheckNoUnload) {
     // Check that no unload function returns true.
     EXPECT_TRUE(lib_manager.runUnload());
 
-    // Explicitly clear the callout_handle_ so that we can delete the library.
-    // This is the only object to contain memory allocated by it.
-    callout_handle_.reset();
-
     // Tidy up
     EXPECT_TRUE(lib_manager.closeLibrary());
 }
@@ -400,10 +378,6 @@ TEST_F(LibraryManagerTest, CheckUnloadError) {
     // Check that unload function returning an error returns false.
     EXPECT_FALSE(lib_manager.runUnload());
 
-    // Explicitly clear the callout_handle_ so that we can delete the library.
-    // This is the only object to contain memory allocated by it.
-    callout_handle_.reset();
-
     // Tidy up
     EXPECT_TRUE(lib_manager.closeLibrary());
 }
@@ -435,10 +409,6 @@ TEST_F(LibraryManagerTest, CheckUnload) {
     // Tidy up
     marker.close();
 
-    // Explicitly clear the callout_handle_ so that we can delete the library.
-    // This is the only object to contain memory allocated by it.
-    callout_handle_.reset();
-
     // Tidy up
     EXPECT_TRUE(lib_manager.closeLibrary());
 }
@@ -527,10 +497,6 @@ TEST_F(LibraryManagerTest, LoadLibrary) {
     // r3 = (7 * d1 - d2) * d3
     executeCallCallouts(7, 5, 35, 9, 26, 3, 78);
 
-    // All done, so unload the library.  First we have to delete the
-    // CalloutHandle as it may contain memory allocated by that library.
-    callout_handle_.reset();
-
     EXPECT_TRUE(lib_manager.unloadLibrary());
 
     // Check that the callouts have been removed from the callout manager.
@@ -587,19 +553,14 @@ TEST_F(LibraryManagerTest, LoadMultipleLibraries) {
     // r3 = ((10 * d1 + d1) - d2) * d2 * d3 - d3
     executeCallCallouts(10, 3, 33, 2, 62, 3, 183);
 
-    // All done, so unload the first library.  First we have to delete the
-    // CalloutHandle as it may contain memory allocated by that library.
-    callout_handle_.reset();
+    // All done, so unload the first library.
     EXPECT_TRUE(lib_manager_1.unloadLibrary());
-    //EXPECT_TRUE(lib_manager_4.unloadLibrary());
 
     // Now execute the callouts again and check that the results are as
     // expected for the new calculation.
-    callout_handle_.reset(new CalloutHandle(callout_manager_));
     executeCallCallouts(10, 5, 15, 7, 105, 17, 88);
 
     // ... and tidy up.
-    callout_handle_.reset();
     EXPECT_TRUE(lib_manager_4.unloadLibrary());
 }
 

src/lib/hooks/tests/load_callout_library.cc

@@ -13,29 +13,27 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 /// @file
-/// @brief Basic Load Library
+/// @brief Basic library with load() function
 ///
-/// This is a test file for the LibraryManager test.  It produces a library
-/// that allows for tests of the basic library manager functions.
-///
-/// The characteristics of this library are:
+/// This is source of a test library for various test (LibraryManager and
+/// HooksManager).  The characteristics of the library produced from this
+/// file are:
 ///
 /// - The "version" and "load" framework functions are supplied.  One "standard"
 ///   callout is supplied ("lm_one") and two non-standard ones which are
 ///   registered during the call to "load" on the hooks "lm_two" and
 ///   "lm_three". 
 ///
-///   All callouts do trivial calculations, the result of the calculation being
+///   All callouts do trivial calculations, the result of all being called in
+///   sequence being
 ///
 ///   @f[ ((5 * data_1) + data_2) * data_3 @f]
 ///
-///   ...where data_1, data_2 and data_3 are the values passed in arguments
-///   of the same name to the three callouts (data_1 passed to lm_one,
-///   data_2 to lm_two etc.) and the result is returned in the argument
-///   "result".
+///   ...where data_1, data_2 and data_3 are the values passed in arguments of
+///   the same name to the three callouts (data_1 passed to lm_one, data_2 to
+///   lm_two etc.) and the result is returned in the argument "result".
 
 #include <hooks/hooks.h>
-#include <iostream>
 
 using namespace isc::hooks;
 
@@ -50,8 +48,10 @@ context_create(CalloutHandle& handle) {
     return (0);
 }
 
-// First callout multiples the passed "data_1" argument to the initialized
-// context value of 5.
+// First callout adds the passed "data_1" argument to the initialized context
+// value of 5. (Note that the value set by context_create is accessed through
+// context and not the argument, so checking that context is correctly passed
+// between callouts in the same library.)
 
 int
 lm_one(CalloutHandle& handle) {
@@ -84,7 +84,7 @@ lm_nonstandard_two(CalloutHandle& handle) {
     return (0);
 }
 
-// Final callout adds the result in "data_3" and.
+// Final callout adds "data_3" to the result.
 
 static int
 lm_nonstandard_three(CalloutHandle& handle) {

src/lib/hooks/tests/load_error_callout_library.cc

@@ -13,18 +13,16 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 /// @file
-/// @brief Error Load Library
+/// @brief Error load library
 ///
-/// This is a test file for the LibraryManager test.  It produces a library
-/// that allows for tests of the basic library manager functions.
-///
-/// The characteristics of this library are:
+/// This is source of a test library for various test (LibraryManager and
+/// HooksManager).  The characteristics of the library produced from this
+/// file are:
 ///
 /// - All framework functions are supplied.  "version" returns the correct
 ///   value, but "load" and unload return an error.
 
 #include <hooks/hooks.h>
-#include <iostream>
 
 using namespace isc::hooks;
 

src/lib/hooks/tests/no_version_library.cc

@@ -15,8 +15,11 @@
 /// @file
 /// @brief No version function library
 ///
-/// This is a test file for the LibraryManager test.  It produces a library
-/// that does not have a "version" function.
+/// This is source of a test library for various test (LibraryManager and
+/// HooksManager).  The characteristics of the library produced from this
+/// file are:
+///
+/// - No version() function is present.
 
 extern "C" {
 

src/lib/hooks/tests/unload_callout_library.cc

@@ -15,10 +15,9 @@
 /// @file
 /// @brief Basic unload library
 ///
-/// This is a test file for the LibraryManager test.  It produces a library
-/// that allows for tests of the basic library manager functions.
-///
-/// The characteristics of this library are:
+/// This is source of a test library for various test (LibraryManager and
+/// HooksManager).  The characteristics of the library produced from this
+/// file are:
 ///
 /// - The "version" and "unload" framework functions are supplied. "version"
 ///   returns a valid value and "unload" creates a marker file and returns
@@ -51,4 +50,3 @@ unload() {
 }
 
 };
-