[2980] Final set of changes resulting from the first review

src/lib/hooks/Makefile.am

@@ -29,7 +29,6 @@ lib_LTLIBRARIES = libb10-hooks.la
 libb10_hooks_la_SOURCES  =
 libb10_hooks_la_SOURCES += callout_handle.cc callout_handle.h
 libb10_hooks_la_SOURCES += callout_manager.cc callout_manager.h
-libb10_hooks_la_SOURCES += framework_functions.h
 libb10_hooks_la_SOURCES += hooks.h
 libb10_hooks_la_SOURCES += hooks_log.cc hooks_log.h
 libb10_hooks_la_SOURCES += hooks_manager.cc hooks_manager.h

src/lib/hooks/callout_handle.h

@@ -168,7 +168,7 @@ public:
 
         value = boost::any_cast<T>(element_ptr->second);
     }
-    
+
     /// @brief Get argument names
     ///
     /// Returns a vector holding the names of arguments in the argument
@@ -273,7 +273,7 @@ public:
 
         value = boost::any_cast<T>(element_ptr->second);
     }
-    
+
     /// @brief Get context names
     ///
     /// Returns a vector holding the names of items in the context associated
@@ -355,7 +355,7 @@ private:
     const ElementCollection& getContextForLibrary() const;
 
     // Member variables
-    
+
     /// Pointer to the collection of libraries for which this handle has been
     /// created.
     boost::shared_ptr<LibraryManagerCollection> lm_collection_;

src/lib/hooks/callout_manager.cc

@@ -62,7 +62,7 @@ CalloutManager::setNumLibraries(int num_libraries) {
 void
 CalloutManager::registerCallout(const std::string& name, CalloutPtr callout) {
     // Note the registration.
-    LOG_DEBUG(hooks_logger, HOOKS_DBG_CALLS, HOOKS_REGISTER_CALLOUT)
+    LOG_DEBUG(hooks_logger, HOOKS_DBG_CALLS, HOOKS_CALLOUT_REGISTRATION)
         .arg(current_library_).arg(name);
 
     // Sanity check that the current library index is set to a valid value.
@@ -142,7 +142,7 @@ CalloutManager::callCallouts(int hook_index, CalloutHandle& callout_handle) {
                 int status = (*i->second)(callout_handle);
                 if (status == 0) {
                     LOG_DEBUG(hooks_logger, HOOKS_DBG_EXTENDED_CALLS,
-                              HOOKS_CALLOUT).arg(current_library_)
+                              HOOKS_CALLOUT_CALLED).arg(current_library_)
                         .arg(ServerHooks::getServerHooks()
                             .getName(current_hook_))
                         .arg(reinterpret_cast<void*>(i->second));
@@ -209,7 +209,7 @@ CalloutManager::deregisterCallout(const std::string& name, CalloutPtr callout) {
     bool removed = initial_size != hook_vector_[hook_index].size();
     if (removed) {
         LOG_DEBUG(hooks_logger, HOOKS_DBG_EXTENDED_CALLS,
-                  HOOKS_DEREGISTER_CALLOUT).arg(current_library_).arg(name);
+                  HOOKS_CALLOUT_DEREGISTERED).arg(current_library_).arg(name);
     }
 
     return (removed);
@@ -244,7 +244,7 @@ CalloutManager::deregisterAllCallouts(const std::string& name) {
     bool removed = initial_size != hook_vector_[hook_index].size();
     if (removed) {
         LOG_DEBUG(hooks_logger, HOOKS_DBG_EXTENDED_CALLS,
-                  HOOKS_DEREGISTER_ALL_CALLOUTS).arg(current_library_)
+                  HOOKS_ALL_CALLOUTS_DEREGISTERED).arg(current_library_)
                                                 .arg(name);
     }
 

src/lib/hooks/callout_manager.h

@@ -60,7 +60,7 @@ public:
 ///   deregister callouts in the same library (including themselves): they
 ///   cannot affect callouts registered by another library.  When calling a
 ///   callout, the callout manager maintains the idea of a "current library
-///   index": if the callout calls one of the callout registration functions 
+///   index": if the callout calls one of the callout registration functions
 ///   (indirectly via the @ref LibraryHandle object), the registration
 ///   functions use the "current library index" in their processing.
 ///
@@ -385,11 +385,11 @@ private:
     /// such that the index of the library associated with any operation is
     /// whatever is currently set in the CalloutManager.
     LibraryHandle library_handle_;
-    
+
     /// LibraryHandle for callouts to be registered as being called before
     /// the user-registered callouts.
     LibraryHandle pre_library_handle_;
-    
+
     /// LibraryHandle for callouts to be registered as being called after
     /// the user-registered callouts.
     LibraryHandle post_library_handle_;

src/lib/hooks/hooks.h

@@ -21,7 +21,7 @@
 namespace {
 
 // Version 1 of the hooks framework.
-static const int BIND10_HOOKS_VERSION = 1;
+const int BIND10_HOOKS_VERSION = 1;
 
 // Names of the framework functions.
 const char* LOAD_FUNCTION_NAME = "load";
@@ -29,10 +29,9 @@ const char* UNLOAD_FUNCTION_NAME = "unload";
 const char* VERSION_FUNCTION_NAME = "version";
 
 // Typedefs for pointers to the framework functions.
-typedef int (*version_function_ptr)();  ///< version() signature
+typedef int (*version_function_ptr)();
 typedef int (*load_function_ptr)(isc::hooks::LibraryHandle&);
-                                        ///< load() signature
-typedef int (*unload_function_ptr)();   ///< unload() signature
+typedef int (*unload_function_ptr)();
 
 } // Anonymous namespace
 

src/lib/hooks/hooks_messages.mes

@@ -14,7 +14,7 @@
 
 $NAMESPACE isc::hooks
 
-% HOOKS_CALLOUT hooks library with index %1 has called a callout on hook %2 that has address %3
+% HOOKS_CALLOUT_CALLED hooks library with index %1 has called a callout on hook %2 that has address %3
 Only output at a high debugging level, this message indicates that
 a callout on the named hook registered by the library with the given
 index (in the list of loaded libraries) has been called and returned a
@@ -26,10 +26,12 @@ is issued.  It identifies the hook to which the callout is attached, the
 index of the library (in the list of loaded libraries) that registered
 it and the address of the callout.  The error is otherwise ignored.
 
-% HOOKS_CALLOUT_REMOVED callout removed from hook %1 for library %2
+% HOOKS_CALLOUTS_REMOVED callouts 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.
+the specified hook.  This is similar to the HOOKS_DEREGISTER_ALL_CALLOUTS
+message (and the two are likely to be seen together), but is issued at a
+higher-level in the hook framework.
 
 % HOOKS_CLOSE_ERROR failed to close hook library %1: %2
 BIND 10 has failed to close the named hook library for the stated reason.
@@ -43,14 +45,16 @@ issued.  It identifies the hook to which the callout is attached, the
 index of the library (in the list of loaded libraries) that registered
 it and the address of the callout.  The error is otherwise ignored.
 
-% HOOKS_DEREGISTER_ALL_CALLOUTS hook library at index %1 deregistered all callouts on hook %2
+% HOOKS_ALL_CALLOUTS_DEREGISTERED hook library at index %1 removed 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.
+by the library with the given index were removed.  This is similar to
+the HOOKS_CALLOUTS_REMOVED message (and the two are likely to be seen
+together), but is issued at a lower-level in the hook framework.
 
-% HOOKS_DEREGISTER_CALLOUT hook library at index %1 deregistered a callout on hook %2
+% HOOKS_CALLOUT_DEREGISTERED 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.
+with the given index have been 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
@@ -73,7 +77,7 @@ has been successfully unloaded.
 A debug  message issued when the version check on the hooks library
 has succeeded.
 
-% HOOKS_LOAD 'load' function in hook library %1 returned success
+% HOOKS_LOAD_SUCCESS '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.
 
@@ -89,8 +93,10 @@ was called.  The function threw an exception (an error indication)
 during execution, which is an error condition.  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_LIBRARY_LOADING loading hooks library %1
+This is a debug message output just before the specified library is loaded.
+If the action is successfully, it will be followed by the
+HOOKS_LIBRARY_LOADED informational message.
 
 % HOOKS_NO_LOAD no 'load' function found in hook library %1
 This is a debug message saying that the specified library was loaded
@@ -114,27 +120,27 @@ 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'
+% HOOKS_CALLOUT_REGISTRATION hooks library with index %1 registering 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.
 
-% HOOKS_REGISTER_HOOK hook %1 was registered
+% HOOKS_HOOK_REGISTERED 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 at address %3
+% HOOKS_STD_CALLOUT_REGISTERED hooks library %1 registered standard callout for hook %2 at address %3
 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.  The address of the callout is indicated.
 
-% HOOKS_RESET_HOOK_LIST the list of hooks has been reset
+% HOOKS_HOOK_LIST_RESET 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
+% HOOKS_UNLOAD_SUCCESS '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.
 
@@ -150,9 +156,10 @@ called, but in the process generated an exception (an error indication).
 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.
+% HOOKS_LIBRARY_UNLOADING unloading library %1
+This is a debug message called when the specified library is
+being unloaded.  If all is successful, it will be followed by the
+HOOKS_LIBRARY_UNLOADED informational message.
 
 % HOOKS_VERSION_EXCEPTION 'version' function in hook library %1 threw an exception
 This error message is issued if the version() function in the specified

src/lib/hooks/library_manager.cc

@@ -32,7 +32,7 @@ namespace hooks {
 /// @brief Local class for conversion of void pointers to function pointers
 ///
 /// Converting between void* and function pointers in C++ is fraught with
-/// difficulty and pitfalls (e.g. see
+/// difficulty and pitfalls, e.g. see
 /// https://groups.google.com/forum/?hl=en&fromgroups#!topic/comp.lang.c++/37o0l8rtEE0
 ///
 /// The method given in that article - convert using a union is used here.  A
@@ -43,7 +43,8 @@ class PointerConverter {
 public:
     /// @brief Constructor
     ///
-    /// Zeroes the union and stores the void* pointer (returned by dlsym) there.
+    /// Zeroes the union and stores the void* pointer we wish to convert (the
+    /// one returned by dlsym).
     ///
     /// @param dlsym_ptr void* pointer returned by call to dlsym()
     PointerConverter(void* dlsym_ptr) {
@@ -148,7 +149,6 @@ LibraryManager::checkVersion() const {
         try {
             version = (*pc.versionPtr())();
         } catch (...) {
-            // Exception - 
             LOG_ERROR(hooks_logger, HOOKS_VERSION_EXCEPTION).arg(library_name_);
             return (false);
         }
@@ -174,9 +174,9 @@ LibraryManager::checkVersion() const {
 
 void
 LibraryManager::registerStandardCallouts() {
-    // Create a library handle for doing the registration.  We also need to
-    // set the current library index to indicate the current library.
-    LibraryHandle library_handle(manager_.get(), index_);
+    // Set the library index for doing the registration.  This is picked up
+    // when the library handle is created.
+    manager_->setLibraryIndex(index_);
 
     // Iterate through the list of known hooks
     vector<string> hook_names = ServerHooks::getServerHooks().getHookNames();
@@ -187,9 +187,10 @@ LibraryManager::registerStandardCallouts() {
         PointerConverter pc(dlsym_ptr);
         if (pc.calloutPtr() != NULL) {
             // Found a symbol, so register it.
-            LOG_DEBUG(hooks_logger, HOOKS_DBG_CALLS, HOOKS_REGISTER_STD_CALLOUT)
+            manager_->getLibraryHandle().registerCallout(hook_names[i],
+                                                         pc.calloutPtr());
+            LOG_DEBUG(hooks_logger, HOOKS_DBG_CALLS, HOOKS_STD_CALLOUT_REGISTERED)
                 .arg(library_name_).arg(hook_names[i]).arg(dlsym_ptr);
-            library_handle.registerCallout(hook_names[i], pc.calloutPtr());
 
         }
     }
@@ -222,7 +223,7 @@ LibraryManager::runLoad() {
                       .arg(status);
             return (false);
         } else {
-        LOG_DEBUG(hooks_logger, HOOKS_DBG_TRACE, HOOKS_LOAD)
+        LOG_DEBUG(hooks_logger, HOOKS_DBG_TRACE, HOOKS_LOAD_SUCCESS)
             .arg(library_name_);
         }
 
@@ -262,7 +263,7 @@ LibraryManager::runUnload() {
                       .arg(status);
             return (false);
         } else {
-        LOG_DEBUG(hooks_logger, HOOKS_DBG_TRACE, HOOKS_UNLOAD)
+        LOG_DEBUG(hooks_logger, HOOKS_DBG_TRACE, HOOKS_UNLOAD_SUCCESS)
             .arg(library_name_);
         }
     } else {
@@ -277,7 +278,7 @@ LibraryManager::runUnload() {
 
 bool
 LibraryManager::loadLibrary() {
-    LOG_DEBUG(hooks_logger, HOOKS_DBG_TRACE, HOOKS_LOAD_LIBRARY)
+    LOG_DEBUG(hooks_logger, HOOKS_DBG_TRACE, HOOKS_LIBRARY_LOADING)
         .arg(library_name_);
 
     // In the following, if a method such as openLibrary() fails, it will
@@ -305,15 +306,15 @@ LibraryManager::loadLibrary() {
                 // The load function failed, so back out.  We can't just close
                 // the library as (a) we need to call the library's "unload"
                 // function (if present) in case "load" allocated resources that
-                // need to be freed and (b) - we need to remove any callouts
-                // that have been installed.
+                // need to be freed and (b) we need to remove any callouts that
+                // have been installed.
                 static_cast<void>(unloadLibrary());
             }
         }
 
-        // Either version check or call to load() failed, so close the library
-        // and free up resources.  Ignore the status return here - we already
-        // know there's an error and will have output a message.
+        // Either the version check or call to load() failed, so close the
+        // library and free up resources.  Ignore the status return here - we
+        // already know there's an error and will have output a message.
         static_cast<void>(closeLibrary());
     }
 
@@ -325,7 +326,7 @@ LibraryManager::loadLibrary() {
 
 bool
 LibraryManager::unloadLibrary() {
-    LOG_DEBUG(hooks_logger, HOOKS_DBG_TRACE, HOOKS_UNLOAD_LIBRARY)
+    LOG_DEBUG(hooks_logger, HOOKS_DBG_TRACE, HOOKS_LIBRARY_UNLOADING)
         .arg(library_name_);
 
     // Call the unload() function if present.  Note that this is done first -
@@ -340,7 +341,7 @@ LibraryManager::unloadLibrary() {
     for (int i = 0; i < hooks.size(); ++i) {
         bool removed = manager_->deregisterAllCallouts(hooks[i]);
         if (removed) {
-            LOG_DEBUG(hooks_logger, HOOKS_DBG_CALLS, HOOKS_CALLOUT_REMOVED)
+            LOG_DEBUG(hooks_logger, HOOKS_DBG_CALLS, HOOKS_CALLOUTS_REMOVED)
                 .arg(hooks[i]).arg(library_name_);
         }
     }

src/lib/hooks/server_hooks.cc

@@ -55,7 +55,7 @@ ServerHooks::registerHook(const string& name) {
     inverse_hooks_[index] = name;
 
     // Log it if debug is enabled
-    LOG_DEBUG(hooks_logger, HOOKS_DBG_TRACE, HOOKS_REGISTER_HOOK).arg(name);
+    LOG_DEBUG(hooks_logger, HOOKS_DBG_TRACE, HOOKS_HOOK_REGISTERED).arg(name);
 
     // ... and return numeric index.
     return (index);
@@ -65,9 +65,6 @@ ServerHooks::registerHook(const string& name) {
 
 void
 ServerHooks::reset() {
-    // Log a warning - 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();
@@ -85,6 +82,10 @@ ServerHooks::reset() {
                   ". context_destroy: expected = " << CONTEXT_DESTROY <<
                   ", actual = " << destroy);
     }
+
+    // Log a warning - although this is done during testing, it should never be
+    // seen in a production system.
+    LOG_WARN(hooks_logger, HOOKS_HOOK_LIST_RESET);
 }
 
 // Find the name associated with a hook index.

src/lib/hooks/tests/basic_callout_library.cc

@@ -24,7 +24,7 @@
 /// - A context_create callout is supplied.
 ///
 /// - Three "standard" callouts are supplied corresponding to the hooks
-///   "hook_point_one", "hook_point_two", "hook_point_three".  All do some trivial calculations
+///   "hookpt_one", "hookpt_two", "hookpt_three".  All do some trivial calculations
 ///   on the arguments supplied to it and the context variables, returning
 ///   intermediate results through the "result" argument. The result of
 ///   executing all four callouts in order is:
@@ -32,8 +32,8 @@
 ///   @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 hook_point_one, data_2 to
-///   hook_point_two etc.) and the result is returned in the argument "result".
+///   the same name to the three callouts (data_1 passed to hookpt_one, data_2 to
+///   hookpt_two etc.) and the result is returned in the argument "result".
 
 #include <hooks/hooks.h>
 #include <fstream>
@@ -58,7 +58,7 @@ context_create(CalloutHandle& handle) {
 // between callouts in the same library.)
 
 int
-hook_point_one(CalloutHandle& handle) {
+hookpt_one(CalloutHandle& handle) {
     int data;
     handle.getArgument("data_1", data);
 
@@ -75,7 +75,7 @@ hook_point_one(CalloutHandle& handle) {
 // argument.
 
 int
-hook_point_two(CalloutHandle& handle) {
+hookpt_two(CalloutHandle& handle) {
     int data;
     handle.getArgument("data_2", data);
 
@@ -91,7 +91,7 @@ hook_point_two(CalloutHandle& handle) {
 // Final callout subtracts the result in "data_3".
 
 int
-hook_point_three(CalloutHandle& handle) {
+hookpt_three(CalloutHandle& handle) {
     int data;
     handle.getArgument("data_3", data);
 

src/lib/hooks/tests/callout_handle_unittest.cc

@@ -83,7 +83,7 @@ TEST_F(CalloutHandleTest, ArgumentDistinctSimpleType) {
     EXPECT_EQ(142, d);
 
     // Add a short (random value).
-    short e = -81; 
+    short e = -81;
     handle.setArgument("short", e);
     EXPECT_EQ(-81, e);
 

src/lib/hooks/tests/callout_manager_unittest.cc

@@ -258,7 +258,7 @@ TEST_F(CalloutManagerTest, RegisterCallout) {
     EXPECT_TRUE(getCalloutManager()->calloutsPresent(beta_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(gamma_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(delta_index_));
-                 
+
     // Check that calling the callouts returns as expected. (This is also a
     // test of the callCallouts method.)
     callout_value_ = 0;
@@ -312,7 +312,7 @@ TEST_F(CalloutManagerTest, CalloutsPresent) {
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(beta_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(gamma_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(delta_index_));
-                 
+
     // Set up so that hooks "alpha", "beta" and "delta" have callouts attached
     // to them, and callout  "gamma" does not. (In the statements below, the
     // exact callouts attached to a hook are not relevant - only the fact
@@ -348,7 +348,7 @@ TEST_F(CalloutManagerTest, CallNoCallouts) {
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(beta_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(gamma_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(delta_index_));
-                 
+
     // Call the callouts on an arbitrary hook and ensure that nothing happens.
     callout_value_ = 475;
     getCalloutManager()->callCallouts(alpha_index_, getCalloutHandle());
@@ -365,7 +365,7 @@ TEST_F(CalloutManagerTest, CallCalloutsSuccess) {
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(beta_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(gamma_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(delta_index_));
-                 
+
     // Each library contributes one callout on hook "alpha".
     callout_value_ = 0;
     getCalloutManager()->setLibraryIndex(1);
@@ -409,7 +409,7 @@ TEST_F(CalloutManagerTest, CallCalloutsError) {
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(beta_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(gamma_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(delta_index_));
-                 
+
     // Each library contributing one callout on hook "alpha". The first callout
     // returns an error (after adding its value to the result).
     callout_value_ = 0;
@@ -481,7 +481,7 @@ TEST_F(CalloutManagerTest, DeregisterSingleCallout) {
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(beta_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(gamma_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(delta_index_));
-                 
+
     // Add a callout to hook "alpha" and check it is added correctly.
     callout_value_ = 0;
     getCalloutManager()->setLibraryIndex(0);
@@ -507,7 +507,7 @@ TEST_F(CalloutManagerTest, DeregisterSingleCalloutSameLibrary) {
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(beta_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(gamma_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(delta_index_));
-                 
+
     // Add multiple callouts to hook "alpha".
     callout_value_ = 0;
     getCalloutManager()->setLibraryIndex(0);
@@ -543,7 +543,7 @@ TEST_F(CalloutManagerTest, DeregisterMultipleCalloutsSameLibrary) {
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(beta_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(gamma_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(delta_index_));
-                 
+
     // Each library contributes one callout on hook "alpha".
     callout_value_ = 0;
     getCalloutManager()->setLibraryIndex(0);
@@ -599,7 +599,7 @@ TEST_F(CalloutManagerTest, DeregisterMultipleCalloutsMultipleLibraries) {
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(beta_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(gamma_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(delta_index_));
-                 
+
     // Each library contributes two callouts to hook "alpha".
     callout_value_ = 0;
     getCalloutManager()->setLibraryIndex(0);
@@ -628,7 +628,7 @@ TEST_F(CalloutManagerTest, DeregisterMultipleCalloutsMultipleLibraries) {
 TEST_F(CalloutManagerTest, DeregisterAllCallouts) {
     // Ensure that no callouts are attached to hook one.
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(alpha_index_));
-                 
+
     // Each library contributes two callouts to hook "alpha".
     callout_value_ = 0;
     getCalloutManager()->setLibraryIndex(0);
@@ -668,7 +668,7 @@ TEST_F(CalloutManagerTest, MultipleCalloutsLibrariesHooks) {
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(beta_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(gamma_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(delta_index_));
-                 
+
     // Register callouts on the alpha hook.
     callout_value_ = 0;
     getCalloutManager()->setLibraryIndex(0);
@@ -744,7 +744,7 @@ TEST_F(CalloutManagerTest, LibraryHandleRegistration) {
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(beta_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(gamma_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(delta_index_));
-                 
+
     // Check that calling the callouts returns as expected. (This is also a
     // test of the callCallouts method.)
     callout_value_ = 0;
@@ -794,7 +794,7 @@ TEST_F(CalloutManagerTest, LibraryHandleAlternateConstructor) {
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(beta_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(gamma_index_));
     EXPECT_FALSE(getCalloutManager()->calloutsPresent(delta_index_));
-                 
+
     // Check that calling the callouts returns as expected. (This is also a
     // test of the callCallouts method.)
     callout_value_ = 0;
@@ -862,7 +862,7 @@ TEST_F(CalloutManagerTest, LibraryHandlePrePostUserLibrary) {
                                                                 callout_four);
     getCalloutManager()->getPreLibraryHandle().registerCallout("alpha",
                                                                 callout_one);
-    
+
     // ... and set up a callout in between, on library number 2.
     LibraryHandle lh1(getCalloutManager().get(), 2);
     lh1.registerCallout("alpha", callout_five);

src/lib/hooks/tests/common_test_class.h

@@ -44,18 +44,18 @@ public:
         isc::hooks::ServerHooks& hooks =
             isc::hooks::ServerHooks::getServerHooks();
         hooks.reset();
-        hook_point_one_index_ = hooks.registerHook("hook_point_one");
-        hook_point_two_index_ = hooks.registerHook("hook_point_two");
-        hook_point_three_index_ = hooks.registerHook("hook_point_three");
+        hookpt_one_index_ = hooks.registerHook("hookpt_one");
+        hookpt_two_index_ = hooks.registerHook("hookpt_two");
+        hookpt_three_index_ = hooks.registerHook("hookpt_three");
     }
 
     /// @brief Call callouts test
     ///
     /// 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 hook_point_one, hook_point_two and hook_point_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".
+    /// to hooks hookpt_one, hookpt_two and hookpt_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, called r0 here.  This value is dependent on the library being
@@ -63,20 +63,24 @@ public:
     /// the purpose being to avoid exceptions when running this test with no
     /// libraries loaded.
     ///
-    /// Callout hook_point_one is passed a value d1 and performs a simple arithmetic
+    /// Callout hookpt_one is passed a value d1 and performs a simple arithmetic
     /// operation on it and r0 yielding a result r1.  Hence we can say that
-    /// @f[ r1 = lm1(r0, d1) @f]
+    /// @f[ r1 = hookpt_one(r0, d1) @f]
     ///
-    /// Callout hook_point_two is passed a value d2 and peforms another simple
+    /// Callout hookpt_two is passed a value d2 and peforms another simple
     /// arithmetic operation on it and d2, yielding r2, i.e.
-    /// @f[ r2 = lm2(d1, d2) @f]
+    /// @f[ r2 = hookpt_two(d1, d2) @f]
     ///
-    /// hook_point_three does a similar operation giving @f[ r3 = lm3(r2, d3) @f].
+    /// hookpt_three does a similar operation giving
+    /// @f[ r3 = hookpt_three(r2, d3) @f].
     ///
-    /// 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 of each step.
+    /// The details of the operations hookpt_one, hookpt_two and hookpt_three
+    /// depend on the library, so the results obtained not only depend on
+    /// the data, but also on the library loaded.  This method is passed both
+    /// data and expected results.  It executes the three callouts in sequence,
+    /// checking the intermediate and final results.  Only if the expected
+    /// library has been loaded correctly and the callouts in it registered
+    /// correctly will be the results be as expected.
     ///
     /// It is assumed that callout_manager_ has been set up appropriately.
     ///
@@ -86,9 +90,9 @@ public:
     ///       allocated by loaded libraries while they are still loaded.
     ///
     /// @param manager CalloutManager to use for the test
-    /// @param r0...r3, d1..d3 Values and intermediate values expected.  They
-    ///        are ordered so that the variables appear in the argument list in
-    ///        the order they are used.
+    /// @param r0...r3, d1..d3 Data (dN) and expected results (rN) - both
+    ///        intermediate and final.  The arguments are ordered so that they
+    ///        appear in the argument list in the order they are used.
     void executeCallCallouts(
             const boost::shared_ptr<isc::hooks::CalloutManager>& manager,
             int r0, int d1, int r1, int d2, int r2, int d3, int r3) {
@@ -112,27 +116,27 @@ public:
 
         // Perform the first calculation.
         handle.setArgument("data_1", d1);
-        manager->callCallouts(hook_point_one_index_, handle);
+        manager->callCallouts(hookpt_one_index_, handle);
         handle.getArgument(RESULT, result);
-