[2995] Changes after review:

- callouts called using HooksManager::callCallouts
- removed Dhcp6Srv::getServerHooks() and getHooksManager() declarations.
- updated sendPacket comment (reception->transmission)
- documented getCalloutHandle()
- indentation fixed for getSubnets6()

src/bin/dhcp6/dhcp6_srv.cc

@@ -141,8 +141,6 @@ Dhcpv6Srv::Dhcpv6Srv(uint16_t port)
         hook_index_pkt6_send_      = Hooks.hook_index_pkt6_send_;
 
         /// @todo call loadLibraries() when handling configuration changes
-        vector<string> libraries; // no libraries at this time
-        HooksManager::loadLibraries(libraries);
 
     } catch (const std::exception &e) {
         LOG_ERROR(dhcp6_logger, DHCP6_SRV_CONSTRUCT_ERROR).arg(e.what());
@@ -217,8 +215,7 @@ bool Dhcpv6Srv::run() {
                 callout_handle->setArgument("query6", query);
 
                 // Call callouts
-                HooksManager::getHooksManager().callCallouts(hook_index_pkt6_receive_,
-                                                *callout_handle);
+                HooksManager::callCallouts(hook_index_pkt6_receive_, *callout_handle);
 
                 // Callouts decided to skip the next processing step. The next
                 // processing step would to process the packet, so skip at this
@@ -308,15 +305,11 @@ bool Dhcpv6Srv::run() {
                     // Delete all previous arguments
                     callout_handle->deleteAllArguments();
 
-                    // Clear skip flag if it was set in previous callouts
-                    callout_handle->setSkip(false);
-
                     // Set our response
                     callout_handle->setArgument("response6", rsp);
 
                     // Call all installed callouts
-                    HooksManager::getHooksManager().callCallouts(hook_index_pkt6_send_,
-                                                    *callout_handle);
+                    HooksManager::callCallouts(hook_index_pkt6_send_, *callout_handle);
 
                     // Callouts decided to skip the next processing step. The next
                     // processing step would to send the packet, so skip at this
@@ -670,8 +663,7 @@ Dhcpv6Srv::selectSubnet(const Pkt6Ptr& question) {
         callout_handle->setArgument("subnet6collection", CfgMgr::instance().getSubnets6());
 
         // Call user (and server-side) callouts
-        HooksManager::getHooksManager().callCallouts(hook_index_subnet6_select_,
-                                        *callout_handle);
+        HooksManager::callCallouts(hook_index_subnet6_select_, *callout_handle);
 
         // Callouts decided to skip this step. This means that no subnet will be
         // selected. Packet processing will continue, but it will be severly limited

src/bin/dhcp6/dhcp6_srv.h

@@ -23,7 +23,7 @@
 #include <dhcp/pkt6.h>
 #include <dhcpsrv/alloc_engine.h>
 #include <dhcpsrv/subnet.h>
-#include <hooks/hooks_manager.h>
+#include <hooks/callout_handle.h>
 
 #include <boost/noncopyable.hpp>
 
@@ -88,20 +88,6 @@ public:
     /// @brief Instructs the server to shut down.
     void shutdown();
 
-    /// @brief returns ServerHooks object
-    /// @todo: remove this as soon as ServerHooks object is converted
-    /// to a signleton.
-    //static boost::shared_ptr<isc::util::ServerHooks> getServerHooks();
-
-    /// @brief returns Callout Manager object
-    ///
-    /// This manager is used to manage callouts registered on various hook
-    /// points. @todo exact access method for HooksManager manager will change
-    /// when it will be converted to a singleton.
-    ///
-    /// @return CalloutManager instance
-    //static boost::shared_ptr<isc::util::HooksManager> getHooksManager();
-
 protected:
 
     /// @brief verifies if specified packet meets RFC requirements
@@ -347,7 +333,7 @@ protected:
     /// @brief dummy wrapper around IfaceMgr::send()
     ///
     /// This method is useful for testing purposes, where its replacement
-    /// simulates reception of a packet. For that purpose it is protected.
+    /// simulates transmission of a packet. For that purpose it is protected.
     virtual void sendPacket(const Pkt6Ptr& pkt);
 
 private:
@@ -364,6 +350,11 @@ private:
     /// initiate server shutdown procedure.
     volatile bool shutdown_;
 
+    /// @brief returns callout handle for specified packet
+    ///
+    /// @param pkt packet for which the handle should be returned
+    ///
+    /// @return a callout handle to be used in hooks related to said packet
     isc::hooks::CalloutHandlePtr getCalloutHandle(const Pkt6Ptr& pkt);
 
     /// Indexes for registered hook points

src/bin/dhcp6/tests/dhcp6_srv_unittest.cc

@@ -295,6 +295,13 @@ public:
     virtual ~NakedDhcpv6SrvTest() {
         // Let's clean up if there is such a file.
         unlink(DUID_FILE);
+        HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts(
+                                                 "pkt6_receive");
+        HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts(
+                                                 "pkt6_send");
+        HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts(
+                                                 "subnet6_select");
+
     };
 
     // A DUID used in most tests (typically as client-id)
@@ -1897,8 +1904,8 @@ Pkt6* captureSimpleSolicit() {
 
 /// @brief a class dedicated to Hooks testing in DHCPv6 server
 ///
-/// This class has a some static members, because each non-static
-/// method has implicit this parameter, so it does not match callout
+/// This class has a number of static members, because each non-static
+/// method has implicit 'this' parameter, so it does not match callout
 /// signature and couldn't be registered. Furthermore, static methods
 /// can't modify non-static members (for obvious reasons), so many
 /// fields are declared static. It is still better to keep them as

src/lib/dhcpsrv/alloc_engine.cc

@@ -509,8 +509,7 @@ Lease6Ptr AllocEngine::reuseExpiredLease(Lease6Ptr& expired,
         callout_handle->setArgument("lease6", expired);
 
         // Call the callouts
-        HooksManager::getHooksManager().callCallouts(hook_index_lease6_select_,
-                                                     *callout_handle);
+        HooksManager::callCallouts(hook_index_lease6_select_, *callout_handle);
 
         // Callouts decided to skip the action. This means that the lease is not
         // assigned, so the client will get NoAddrAvail as a result. The lease
@@ -608,8 +607,7 @@ Lease6Ptr AllocEngine::createLease6(const Subnet6Ptr& subnet,
         callout_handle->setArgument("lease6", lease);
 
         // This is the first callout, so no need to clear any arguments
-        HooksManager::getHooksManager().callCallouts(hook_index_lease6_select_,
-                                                     *callout_handle);
+        HooksManager::callCallouts(hook_index_lease6_select_, *callout_handle);
 
         // Callouts decided to skip the action. This means that the lease is not
         // assigned, so the client will get NoAddrAvail as a result. The lease

src/lib/dhcpsrv/cfgmgr.h

@@ -209,8 +209,8 @@ public:
     /// of possible choices (i.e. all subnets).
     /// @return const reference to Subnet6 collection
     inline const Subnet6Collection& getSubnets6() {
-    return (subnets6_);
-}
+        return (subnets6_);
+    }
 
 
     /// @brief get IPv4 subnet by address