[2974] Updated CalloutHandle in line with new CalloutManager

src/lib/util/Makefile.am

@@ -38,7 +38,7 @@ libb10_util_la_SOURCES += encode/base_n.cc encode/hex.h
 libb10_util_la_SOURCES += encode/binary_from_base32hex.h
 libb10_util_la_SOURCES += encode/binary_from_base16.h
 libb10_util_la_SOURCES += hooks/callout_manager.h hooks/callout_manager.cc
-# libb10_util_la_SOURCES += hooks/callout_handle.h hooks/callout_handle.cc
+libb10_util_la_SOURCES += hooks/callout_handle.h hooks/callout_handle.cc
 libb10_util_la_SOURCES += hooks/library_handle.h hooks/library_handle.cc
 libb10_util_la_SOURCES += hooks/server_hooks.h hooks/server_hooks.cc
 libb10_util_la_SOURCES += random/qid_gen.h random/qid_gen.cc

src/lib/util/hooks/callout_handle.cc

@@ -13,6 +13,7 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <util/hooks/callout_handle.h>
+#include <util/hooks/callout_manager.h>
 #include <util/hooks/library_handle.h>
 #include <util/hooks/server_hooks.h>
 
@@ -27,16 +28,13 @@ namespace isc {
 namespace util {
 
 // Constructor.
-CalloutHandle::CalloutHandle(
-    boost::shared_ptr<LibraryHandleCollection>& collection)
-    : arguments_(), context_collection_(), library_collection_(collection),
-      skip_(false) {
+CalloutHandle::CalloutHandle(const boost::shared_ptr<CalloutManager>& manager)
+    : arguments_(), context_collection_(), manager_(manager), skip_(false) {
 
     // Call the "context_create" hook.  We should be OK doing this - although
     // the constructor has not finished running, all the member variables
     // have been created.
-    int status = library_collection_->callCallouts(ServerHooks::CONTEXT_CREATE,
-                                                   *this);
+    int status = manager_->callCallouts(ServerHooks::CONTEXT_CREATE, *this);
     if (status > 0) {
         isc_throw(ContextCreateFail, "error code of " << status << " returned "
                   "from context_create callout during the creation of a "
@@ -50,11 +48,10 @@ CalloutHandle::~CalloutHandle() {
     // Call the "context_destroy" hook.  We should be OK doing this - although
     // the destructor is being called, all the member variables are still in
     // existence.
-    int status = library_collection_->callCallouts(ServerHooks::CONTEXT_DESTROY,
-                                                   *this);
+    int status = manager_->callCallouts(ServerHooks::CONTEXT_DESTROY, *this);
     if (status > 0) {
         // An exception is thrown on failure.  This may be severe, but if
-        // none is thrown a resoucre leak in a library (signalled by the
+        // none is thrown a resource leak in a library (signalled by the
         // context_destroy callout returning an error) may be difficult to
         // trace.
         isc_throw(ContextDestroyFail, "error code of " << status << " returned "
@@ -77,30 +74,12 @@ CalloutHandle::getArgumentNames() const {
     return (names);
 }
 
-// Return the "current" library handle.
+// Return the library handle allowing the callout to access the CalloutManager
+// registration/deregistration functions.
 
 LibraryHandle&
 CalloutHandle::getLibraryHandle() const {
-    boost::shared_ptr<LibraryHandle> handle =
-        library_collection_->getLibraryHandle();
-
-    // Return refernce to this library handle.  This remains valid even
-    // after this method returns, because this object maintains a shared
-    // pointer to the LibraryHandleCollection, which in turn maintains
-    // a shared pointer to the LibraryHandle in question.
-    return (*handle);
-}
-
-// Check the current library index.
-
-int
-CalloutHandle::getLibraryIndex() const {
-    int curidx = library_collection_->getLibraryIndex();
-    if (curidx < 0) {
-        isc_throw(InvalidIndex, "current library handle index is not valid");
-    }
-
-    return (curidx);
+    return (manager_->getLibraryHandle());
 }
 
 // Return the context for the currently pointed-to library.  This version is
@@ -109,7 +88,7 @@ CalloutHandle::getLibraryIndex() const {
 
 CalloutHandle::ElementCollection&
 CalloutHandle::getContextForLibrary() {
-    int libindex = getLibraryIndex();
+    int libindex = manager_->getLibraryIndex();
 
     // Access a reference to the element collection for the given index,
     // creating a new element collection if necessary, and return it.
@@ -121,7 +100,7 @@ CalloutHandle::getContextForLibrary() {
 
 const CalloutHandle::ElementCollection&
 CalloutHandle::getContextForLibrary() const {
-    int libindex = getLibraryIndex();
+    int libindex = manager_->getLibraryIndex();
 
     ContextCollection::const_iterator libcontext =
         context_collection_.find(libindex);

src/lib/util/hooks/callout_handle.h

@@ -16,6 +16,7 @@
 #define CALLOUT_HANDLE_H
 
 #include <exceptions/exceptions.h>
+#include <util/hooks/library_handle.h>
 
 #include <boost/any.hpp>
 #include <boost/shared_ptr.hpp>
@@ -100,14 +101,11 @@ class LibraryHandle;
 ///   "context_destroy" callout.  The information is accessed through the
 ///   {get,set}Context() methods.
 ///
-/// - Per-library context.  Each library has a context associated with it that
-///   is independent of the packet.  All callouts in the library share the
-///   information in the context, regardless of the packet being processed.
-///   The context is typically created in the library's "load()" method and
-///   destroyed in its "unload()" method.  The information is available by
-///   obtaining the handle to the library through this class's
-///   getLibraryHandle() method, then calling {get,set}Context() on the
-///   object returned.
+/// - Per-library handle.  Allows the callout to dynamically register and
+///   deregister callouts. (In the latter case, only functions registered by
+///   functions in the same library as the callout doing the deregistration
+///   can be removed: callouts registered by other libraries cannot be
+///   modified.)
 
 class CalloutHandle {
 public:
@@ -136,12 +134,12 @@ public:
     /// hook.
     ///
     /// @param manager Pointer to the callout manager object.
-    CalloutHandle(boost::shared_ptr<CalloutManager>& /* manager */) {}
+    CalloutHandle(const boost::shared_ptr<CalloutManager>& manager);
 
     /// @brief Destructor
     ///
     /// Calls the context_destroy callback to release any per-packet context.
-    ~CalloutHandle() {}
+    ~CalloutHandle();
 
     /// @brief Set argument
     ///

src/lib/util/hooks/callout_manager.h

@@ -91,7 +91,7 @@ public:
     ///        to 0, or if the pointer to the server hooks object is empty.
     CalloutManager(const boost::shared_ptr<ServerHooks>& hooks,
                    int num_libraries)
-        : current_library_(-1), hooks_(hooks), hook_vector_(hooks->getCount()),
+        : current_library_(-1), hooks_(hooks), hook_vector_(),
           library_handle_(this), num_libraries_(num_libraries)
     {
         if (!hooks) {
@@ -101,6 +101,9 @@ public:
             isc_throw(isc::BadValue, "number of libraries passed to the "
                       "CalloutManager must be >= 0");
         }
+
+        // Parameters OK, do operations that depend on them.
+        hook_vector_.resize(hooks_->getCount());
     }
 
     /// @brief Register a callout on a hook

src/lib/util/tests/Makefile.am

@@ -25,7 +25,7 @@ run_unittests_SOURCES  = run_unittests.cc
 run_unittests_SOURCES += base32hex_unittest.cc
 run_unittests_SOURCES += base64_unittest.cc
 run_unittests_SOURCES += buffer_unittest.cc
-# run_unittests_SOURCES += callout_handle_unittest.cc
+run_unittests_SOURCES += callout_handle_unittest.cc
 run_unittests_SOURCES += callout_manager_unittest.cc
 run_unittests_SOURCES += fd_share_tests.cc
 run_unittests_SOURCES += fd_tests.cc

src/lib/util/tests/callout_handle_unittest.cc

@@ -13,6 +13,7 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <util/hooks/callout_handle.h>
+#include <util/hooks/callout_manager.h>
 #include <util/hooks/library_handle.h>
 #include <util/hooks/server_hooks.h>
 
@@ -30,18 +31,24 @@ public:
 
     /// @brief Constructor
     ///
-    /// Sets up an appropriate number of server hooks to pass to the
-    /// constructed callout handle objects.
-    CalloutHandleTest() : collection_(new LibraryHandleCollection()) {
-    }
+    /// Sets up a callout manager to be referenced by the CalloutHandle in
+    /// these tests. (The "4" for the number of libraries in the
+    /// CalloutManager is arbitrary - it is not used in these tests.)
+    CalloutHandleTest()
+        : hooks_(new ServerHooks()), manager_(new CalloutManager(hooks_, 4))
+    {}
 
     /// Obtain hook manager
-    boost::shared_ptr<LibraryHandleCollection>& getLibraryHandleCollection() {
-        return (collection_);
+    boost::shared_ptr<CalloutManager>& getCalloutManager() {
+        return (manager_);
     }
 
 private:
-    boost::shared_ptr<LibraryHandleCollection> collection_;
+    /// List of server hooks
+    boost::shared_ptr<ServerHooks> hooks_;
+
+    /// Callout manager accessed by this CalloutHandle.
+    boost::shared_ptr<CalloutManager> manager_;
 };
 
 // *** Argument Tests ***
@@ -53,7 +60,7 @@ private:
 // are distinct.
 
 TEST_F(CalloutHandleTest, ArgumentDistinctSimpleType) {
-    CalloutHandle handle(getLibraryHandleCollection());
+    CalloutHandle handle(getCalloutManager());
 
     // Store and retrieve an int (random value).
     int a = 42;
@@ -86,7 +93,7 @@ TEST_F(CalloutHandleTest, ArgumentDistinctSimpleType) {
 // Test that trying to get an unknown argument throws an exception.
 
 TEST_F(CalloutHandleTest, ArgumentUnknownName) {
-    CalloutHandle handle(getLibraryHandleCollection());
+    CalloutHandle handle(getCalloutManager());
 
     // Set an integer
     int a = 42;
@@ -107,7 +114,7 @@ TEST_F(CalloutHandleTest, ArgumentUnknownName) {
 // exception.
 
 TEST_F(CalloutHandleTest, ArgumentIncorrectType) {
-    CalloutHandle handle(getLibraryHandleCollection());
+    CalloutHandle handle(getCalloutManager());
 
     // Set an integer
     int a = 42;
@@ -136,7 +143,7 @@ struct Beta {
 };
 
 TEST_F(CalloutHandleTest, ComplexTypes) {
-    CalloutHandle handle(getLibraryHandleCollection());
+    CalloutHandle handle(getCalloutManager());
 
     // Declare two variables of different (complex) types. (Note as to the
     // variable names: aleph and beth are the first two letters of the Hebrew
@@ -175,7 +182,7 @@ TEST_F(CalloutHandleTest, ComplexTypes) {
 // that a "pointer to X" is not the same as a "pointer to const X".
 
 TEST_F(CalloutHandleTest, PointerTypes) {
-    CalloutHandle handle(getLibraryHandleCollection());
+    CalloutHandle handle(getCalloutManager());
 
     // Declare a couple of variables, const and non-const.
     Alpha aleph(5, 10);
@@ -209,7 +216,7 @@ TEST_F(CalloutHandleTest, PointerTypes) {
 // Check that we can get the names of the arguments.
 
 TEST_F(CalloutHandleTest, ContextItemNames) {
-    CalloutHandle handle(getLibraryHandleCollection());
+    CalloutHandle handle(getCalloutManager());
 
     vector<string> expected_names;
     int value = 42;
@@ -233,7 +240,7 @@ TEST_F(CalloutHandleTest, ContextItemNames) {
 // Test that we can delete an argument.
 
 TEST_F(CalloutHandleTest, DeleteArgument) {
-    CalloutHandle handle(getLibraryHandleCollection());
+    CalloutHandle handle(getCalloutManager());
 
     int one = 1;
     int two = 2;
@@ -275,7 +282,7 @@ TEST_F(CalloutHandleTest, DeleteArgument) {
 // Test that we can delete all arguments.
 
 TEST_F(CalloutHandleTest, DeleteAllArguments) {
-    CalloutHandle handle(getLibraryHandleCollection());
+    CalloutHandle handle(getCalloutManager());
 
     int one = 1;
     int two = 2;
@@ -302,7 +309,7 @@ TEST_F(CalloutHandleTest, DeleteAllArguments) {
 // Test the "skip" flag.
 
 TEST_F(CalloutHandleTest, SkipFlag) {
-    CalloutHandle handle(getLibraryHandleCollection());
+    CalloutHandle handle(getCalloutManager());
 
     // Should be false on construction.
     EXPECT_FALSE(handle.getSkip());