BIND 10 master, updated. c845a96005b28acde2d62a93ee8c046162da4cb4 [master] ChangeLog for #3054.

BIND 10 source code commits bind10-changes at lists.isc.org
Wed Jul 24 10:27:57 UTC 2013


The branch, master has been updated
       via  c845a96005b28acde2d62a93ee8c046162da4cb4 (commit)
       via  0f845ed94f462dee85b67f056656b2a197878b04 (commit)
       via  088a0124e731718b6e8be8092a47a087436ec1dc (commit)
       via  8e13f44e871bde3f44ffc080a1996149d61fc13d (commit)
       via  a6c4e80483303e6fb4deabb586a357987b79b194 (commit)
      from  fe2331e7e0cadd2e70dfecdb9cf5a67f6484a5bd (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit c845a96005b28acde2d62a93ee8c046162da4cb4
Author: Stephen Morris <stephen at isc.org>
Date:   Wed Jul 24 11:26:27 2013 +0100

    [master] ChangeLog for #3054.

commit 0f845ed94f462dee85b67f056656b2a197878b04
Merge: fe2331e 088a012
Author: Stephen Morris <stephen at isc.org>
Date:   Wed Jul 24 11:01:37 2013 +0100

    [master] Merge branch 'trac3054'

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog                                          |    6 +
 src/lib/hooks/callout_manager.cc                   |   25 ++--
 src/lib/hooks/callout_manager.h                    |   31 +----
 src/lib/hooks/hooks_manager.cc                     |   31 ++++-
 src/lib/hooks/hooks_manager.h                      |   40 ++++++
 src/lib/hooks/library_manager.cc                   |  115 ++++++++++++----
 src/lib/hooks/library_manager.h                    |   59 ++++++--
 src/lib/hooks/library_manager_collection.cc        |   41 ++++--
 src/lib/hooks/library_manager_collection.h         |   45 +++++-
 src/lib/hooks/tests/callout_manager_unittest.cc    |    4 -
 src/lib/hooks/tests/hooks_manager_unittest.cc      |  124 +++++++++++++----
 .../tests/library_manager_collection_unittest.cc   |  143 ++++++++++++++------
 src/lib/hooks/tests/library_manager_unittest.cc    |   14 ++
 13 files changed, 513 insertions(+), 165 deletions(-)

-----------------------------------------------------------------------
diff --git a/ChangeLog b/ChangeLog
index 23b98cb..91b9569 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+646.	[func]		stephen
+	Extended the hooks framework to add a "validate libraries" function.
+	This will be used to check libraries specified during BIND 10
+	configuration.
+	(Trac #3054, git 0f845ed94f462dee85b67f056656b2a197878b04)
+
 645.	[func]		tomek
 	Added initial set of hooks (pk4_receive, subnet4_select,
 	lease4_select, pkt4_send) to the DHCPv6 server.
diff --git a/src/lib/hooks/callout_manager.cc b/src/lib/hooks/callout_manager.cc
index 4ba3640..5a2126a 100644
--- a/src/lib/hooks/callout_manager.cc
+++ b/src/lib/hooks/callout_manager.cc
@@ -29,6 +29,19 @@ using namespace std;
 namespace isc {
 namespace hooks {
 
+// Constructor
+CalloutManager::CalloutManager(int num_libraries)
+    : current_hook_(-1), current_library_(-1),
+      hook_vector_(ServerHooks::getServerHooks().getCount()),
+      library_handle_(this), pre_library_handle_(this, 0),
+      post_library_handle_(this, INT_MAX), num_libraries_(num_libraries)
+{
+    if (num_libraries < 0) {
+        isc_throw(isc::BadValue, "number of libraries passed to the "
+                  "CalloutManager must be >= 0");
+    }
+}
+
 // Check that the index of a library is valid.  It can range from 1 - n
 // (n is the number of libraries), 0 (pre-user library callouts), or INT_MAX
 // (post-user library callouts).  It can also be -1 to indicate an invalid
@@ -46,18 +59,6 @@ CalloutManager::checkLibraryIndex(int library_index) const {
               num_libraries_ << ")");
 }
 
-// Set the number of libraries handled by the CalloutManager.
-
-void
-CalloutManager::setNumLibraries(int num_libraries) {
-    if (num_libraries < 0) {
-        isc_throw(isc::BadValue, "number of libraries passed to the "
-                  "CalloutManager must be >= 0");
-    }
-
-    num_libraries_ = num_libraries;
-}
-
 // Register a callout for the current library.
 
 void
diff --git a/src/lib/hooks/callout_manager.h b/src/lib/hooks/callout_manager.h
index 4619006..e1b9e57 100644
--- a/src/lib/hooks/callout_manager.h
+++ b/src/lib/hooks/callout_manager.h
@@ -132,19 +132,8 @@ public:
     ///
     /// @param num_libraries Number of loaded libraries.
     ///
-    /// @throw isc::BadValue if the number of libraries is less than or equal
-    ///        to 0, or if the pointer to the server hooks object is empty.
-    CalloutManager(int num_libraries = 0)
-        : current_hook_(-1), current_library_(-1),
-          hook_vector_(ServerHooks::getServerHooks().getCount()),
-          library_handle_(this), pre_library_handle_(this, 0),
-          post_library_handle_(this, INT_MAX), num_libraries_(num_libraries)
-    {
-        // Check that the number of libraries is OK.  (This does a redundant
-        // set of the number of libraries, but it's only a single assignment
-        // and avoids the need for a separate "check" method.
-        setNumLibraries(num_libraries);
-    }
+    /// @throw isc::BadValue if the number of libraries is less than 0,
+    CalloutManager(int num_libraries = 0);
 
     /// @brief Register a callout on a hook for the current library
     ///
@@ -224,22 +213,6 @@ public:
         return (current_hook_);
     }
 
-    /// @brief Set number of libraries
-    ///
-    /// Sets the number of libraries.  Although the value is passed to the
-    /// constructor, in some cases that is only an estimate and the number
-    /// can only be determined after the CalloutManager is created.
-    ///
-    /// @note If the number if libraries is reset, it must be done *before*
-    ///       any callouts are registered.
-    ///
-    /// @param num_libraries Number of libraries served by this CalloutManager.
-    ///
-    /// @throw BadValue Number of libraries must be >= 0.
-    /// @throw LibraryCountChanged Number of libraries has been changed after
-    ///        callouts have been registered.
-    void setNumLibraries(int num_libraries);
-
     /// @brief Get number of libraries
     ///
     /// Returns the number of libraries that this CalloutManager is expected
diff --git a/src/lib/hooks/hooks_manager.cc b/src/lib/hooks/hooks_manager.cc
index 39e1fc5..117db61 100644
--- a/src/lib/hooks/hooks_manager.cc
+++ b/src/lib/hooks/hooks_manager.cc
@@ -81,8 +81,15 @@ HooksManager::loadLibrariesInternal(const std::vector<std::string>& libraries) {
     lm_collection_.reset(new LibraryManagerCollection(libraries));
     bool status = lm_collection_->loadLibraries();
 
-    // ... and obtain the callout manager for them.
-    callout_manager_ = lm_collection_->getCalloutManager();
+    if (status) {
+        // ... and obtain the callout manager for them if successful.
+        callout_manager_ = lm_collection_->getCalloutManager();
+    } else {
+        // Unable to load libraries, reset to state before this function was
+        // called.
+        lm_collection_.reset();
+        callout_manager_.reset();
+    }
 
     return (status);
 }
@@ -124,6 +131,19 @@ HooksManager::createCalloutHandle() {
     return (getHooksManager().createCalloutHandleInternal());
 }
 
+// Get the list of the names of loaded libraries.
+
+std::vector<std::string>
+HooksManager::getLibraryNamesInternal() const {
+    return (lm_collection_ ? lm_collection_->getLibraryNames()
+                           : std::vector<std::string>());
+}
+
+std::vector<std::string>
+HooksManager::getLibraryNames() {
+    return (getHooksManager().getLibraryNamesInternal());
+}
+
 // Perform conditional initialization if nothing is loaded.
 
 void
@@ -169,5 +189,12 @@ HooksManager::postCalloutsLibraryHandle() {
     return (getHooksManager().postCalloutsLibraryHandleInternal());
 }
 
+// Validate libraries
+
+std::vector<std::string>
+HooksManager::validateLibraries(const std::vector<std::string>& libraries) {
+    return (LibraryManagerCollection::validateLibraries(libraries));
+}
+
 } // namespace util
 } // namespace isc
diff --git a/src/lib/hooks/hooks_manager.h b/src/lib/hooks/hooks_manager.h
index 03aa521..53a2525 100644
--- a/src/lib/hooks/hooks_manager.h
+++ b/src/lib/hooks/hooks_manager.h
@@ -171,6 +171,30 @@ public:
     ///         registered.
     static int registerHook(const std::string& name);
 
+    /// @brief Return list of loaded libraries
+    ///
+    /// Returns the names of the loaded libraries.
+    ///
+    /// @return List of loaded library names.
+    static std::vector<std::string> getLibraryNames();
+
+    /// @brief Validate library list
+    ///
+    /// For each library passed to it, checks that the library can be opened
+    /// and that the "version" function is present and gives the right answer.
+    /// Each library is closed afterwards.
+    ///
+    /// This is used during the configuration parsing - when the list of hooks
+    /// libraries is changed, each of the new libraries is checked before the
+    /// change is committed.
+    ///
+    /// @param List of libraries to be validated.
+    ///
+    /// @return An empty vector if all libraries validated.  Otherwise it
+    ///         holds the names of the libraries that failed validation.
+    static std::vector<std::string> validateLibraries(
+                       const std::vector<std::string>& libraries);
+
     /// Index numbers for pre-defined hooks.
     static const int CONTEXT_CREATE = ServerHooks::CONTEXT_CREATE;
     static const int CONTEXT_DESTROY = ServerHooks::CONTEXT_DESTROY;
@@ -188,6 +212,17 @@ private:
     /// but actually do the work on the singleton instance of the HooksManager.
     /// See the descriptions of the static methods for more details.
 
+    /// @brief Validate library list
+    ///
+    /// @param List of libraries to be validated.
+    ///
+    /// @return An empty string if all libraries validated.  Otherwise it is
+    ///         the name of the first library that failed validation.  The
+    ///         configuration code can return this to bindctl as an indication
+    ///         of the problem.
+    std::string validateLibrariesInternal(
+                       const std::vector<std::string>& libraries) const;
+
     /// @brief Load and reload libraries
     ///
     /// @param libraries List of libraries to be loaded.  The order is
@@ -234,6 +269,11 @@ private:
     ///         registration.
     LibraryHandle& postCalloutsLibraryHandleInternal();
 
+    /// @brief Return list of loaded libraries
+    ///
+    /// @return List of loaded library names.
+    std::vector<std::string> getLibraryNamesInternal() const;
+
     //@}
 
     /// @brief Initialization to No Libraries
diff --git a/src/lib/hooks/library_manager.cc b/src/lib/hooks/library_manager.cc
index f425508..70c76ba 100644
--- a/src/lib/hooks/library_manager.cc
+++ b/src/lib/hooks/library_manager.cc
@@ -31,6 +31,42 @@ namespace isc {
 namespace hooks {
 
 
+// Constructor (used by external agency)
+LibraryManager::LibraryManager(const std::string& name, int index,
+                               const boost::shared_ptr<CalloutManager>& manager)
+        : dl_handle_(NULL), index_(index), manager_(manager),
+          library_name_(name)
+{
+    if (!manager) {
+        isc_throw(NoCalloutManager, "must specify a CalloutManager when "
+                  "instantiating a LibraryManager object");
+    }
+}
+
+// Constructor (used by "validate" for library validation).  Note that this
+// sets "manager_" to not point to anything, which means that methods such as
+// registerStandardCallout() will fail, probably with a segmentation fault.
+// There are no checks for this condition in those methods: this constructor
+// is declared "private", so can only be executed by a method in this class.
+// The only method to do so is "validateLibrary", which takes care not to call
+// methods requiring a non-NULL manager.
+LibraryManager::LibraryManager(const std::string& name)
+        : dl_handle_(NULL), index_(-1), manager_(), library_name_(name)
+{}
+
+// Destructor.  
+LibraryManager::~LibraryManager() {
+    if (manager_) {
+        // LibraryManager instantiated to load a library, so ensure that
+        // it is unloaded before exiting.
+        static_cast<void>(unloadLibrary());
+    } else {
+        // LibraryManager instantiated to validate a library, so just ensure
+        // that it is closed before exiting.
+        static_cast<void>(closeLibrary());
+    }
+}
+
 // Open the library
 
 bool
@@ -118,8 +154,9 @@ LibraryManager::registerStandardCallouts() {
             // Found a symbol, so register it.
             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);
+            LOG_DEBUG(hooks_logger, HOOKS_DBG_CALLS,
+                      HOOKS_STD_CALLOUT_REGISTERED).arg(library_name_)
+                      .arg(hook_names[i]).arg(dlsym_ptr);
 
         }
     }
@@ -251,41 +288,65 @@ LibraryManager::loadLibrary() {
 }
 
 // The library unloading function.  Call the unload() function (if present),
-// remove callouts from the callout manager, then close the library.
+// remove callouts from the callout manager, then close the library.  This is
+// only run if the library is still loaded and is a no-op if the library is
+// not open.
 
 bool
 LibraryManager::unloadLibrary() {
-    LOG_DEBUG(hooks_logger, HOOKS_DBG_TRACE, HOOKS_LIBRARY_UNLOADING)
-        .arg(library_name_);
+    bool result = true;
+    if (dl_handle_ != NULL) {
+        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 -
-    // operations take place in the reverse order to which they were done when
-    // the library was loaded.
-    bool result = runUnload();
+        // Call the unload() function if present.  Note that this is done first
+        // - operations take place in the reverse order to which they were done
+        // when the library was loaded.
+        result = runUnload();
+
+        // Regardless of status, remove all callouts associated with this
+        // library on all hooks.
+        vector<string> hooks = ServerHooks::getServerHooks().getHookNames();
+        manager_->setLibraryIndex(index_);
+        for (int i = 0; i < hooks.size(); ++i) {
+            bool removed = manager_->deregisterAllCallouts(hooks[i]);
+            if (removed) {
+                LOG_DEBUG(hooks_logger, HOOKS_DBG_CALLS, HOOKS_CALLOUTS_REMOVED)
+                    .arg(hooks[i]).arg(library_name_);
+            }
+        }
 
-    // Regardless of status, remove all callouts associated with this library
-    // on all hooks.
-    vector<string> hooks = ServerHooks::getServerHooks().getHookNames();
-    manager_->setLibraryIndex(index_);
-    for (int i = 0; i < hooks.size(); ++i) {
-        bool removed = manager_->deregisterAllCallouts(hooks[i]);
-        if (removed) {
-            LOG_DEBUG(hooks_logger, HOOKS_DBG_CALLS, HOOKS_CALLOUTS_REMOVED)
-                .arg(hooks[i]).arg(library_name_);
+        // ... and close the library.
+        result = closeLibrary() && result;
+        if (result) {
+
+            // Issue the informational message only if the library was unloaded
+            // with no problems.  If there was an issue, an error message would
+            // have been issued.
+            LOG_INFO(hooks_logger, HOOKS_LIBRARY_UNLOADED).arg(library_name_);
         }
     }
+    return (result);
+}
 
-    // ... and close the library.
-    result = closeLibrary() && result;
-    if (result) {
+// Validate the library.  We must be able to open it, and the version function
+// must both exist and return the right number.  Note that this is a static
+// method.
 
-        // Issue the informational message only if the library was unloaded
-        // with no problems.  If there was an issue, an error message would
-        // have been issued.
-        LOG_INFO(hooks_logger, HOOKS_LIBRARY_UNLOADED).arg(library_name_);
-    }
+bool
+LibraryManager::validateLibrary(const std::string& name) {
+    // Instantiate a library manager for the validation.  We use the private
+    // constructor as we don't supply a CalloutManager.
+    LibraryManager manager(name);
 
-    return (result);
+    // Try to open it and, if we succeed, check the version.
+    bool validated = manager.openLibrary() && manager.checkVersion();
+
+    // Regardless of whether the version checked out, close the library. (This
+    // is a no-op if the library failed to open.)
+    static_cast<void>(manager.closeLibrary());
+
+    return (validated);
 }
 
 } // namespace hooks
diff --git a/src/lib/hooks/library_manager.h b/src/lib/hooks/library_manager.h
index 56c770b..dc7c5eb 100644
--- a/src/lib/hooks/library_manager.h
+++ b/src/lib/hooks/library_manager.h
@@ -15,6 +15,8 @@
 #ifndef LIBRARY_MANAGER_H
 #define LIBRARY_MANAGER_H
 
+#include <exceptions/exceptions.h>
+
 #include <boost/shared_ptr.hpp>
 
 #include <string>
@@ -22,13 +24,25 @@
 namespace isc {
 namespace hooks {
 
+/// @brief No Callout Manager
+///
+/// Thrown if a library manager is instantiated by an external agency without
+/// specifying a CalloutManager object.
+class NoCalloutManager : public Exception {
+public:
+    NoCalloutManager(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) {}
+};
+
 class CalloutManager;
 class LibraryHandle;
 class LibraryManager;
 
 /// @brief Library manager
 ///
-/// This class handles the loading and unloading of a specific library.
+/// This class handles the loading and unloading of a specific library.  It also
+/// provides a static method for checking that a library is valid (this is used
+/// in configuration parsing).
 ///
 /// On loading, it opens the library using dlopen and checks the version (set
 /// with the "version" method.  If all is OK, it iterates through the list of
@@ -58,22 +72,28 @@ class LibraryManager;
 ///       suspends processing of new requests until all existing ones have
 ///       been serviced and all packet/context structures destroyed before
 ///       reloading the libraries.
+///
+/// When validating a library, only the fact that the library can be opened and
+/// version() exists and returns the correct number is checked.  The library
+/// is closed after the validation.
 
 class LibraryManager {
 public:
     /// @brief Constructor
     ///
-    /// Stores the library name.  The actual loading is done in loadLibrary().
+    /// This constructor is used by external agencies (i.e. the
+    /// LibraryManagerCollection) when instantiating a LibraryManager.  It
+    /// stores the library name - the actual actual loading is done in
+    /// loadLibrary().
     ///
     /// @param name Name of the library to load.  This should be an absolute
     ///        path name.
     /// @param index Index of this library
     /// @param manager CalloutManager object
+    ///
+    /// @throw NoCalloutManager Thrown if the manager argument is NULL.
     LibraryManager(const std::string& name, int index,
-                   const boost::shared_ptr<CalloutManager>& manager)
-        : dl_handle_(NULL), index_(index), manager_(manager),
-          library_name_(name)
-    {}
+                   const boost::shared_ptr<CalloutManager>& manager);
 
     /// @brief Destructor
     ///
@@ -81,9 +101,19 @@ public:
     /// feature to ensure closure in the case of an exception destroying this
     /// object.  However, see the caveat in the class header about when it is
     /// safe to unload libraries.
-    ~LibraryManager() {
-        static_cast<void>(unloadLibrary());
-    }
+    ~LibraryManager();
+
+    /// @brief Validate library
+    ///
+    /// A static method that is used to validate a library.  Validation checks
+    /// that the library can be opened, that "version" exists, and that it
+    /// returns the right number.
+    ///
+    /// @param name Name of the library to validate
+    ///
+    /// @return true if the library validated, false if not.  If the library
+    /// fails to validate, the reason for the failure is logged.
+    static bool validateLibrary(const std::string& name);
 
     /// @brief Loads a library
     ///
@@ -176,6 +206,17 @@ protected:
     bool runUnload();
 
 private:
+    /// @brief Validating constructor
+    ///
+    /// Constructor used when the LibraryManager is instantiated to validate
+    /// a library (i.e. by the "validateLibrary" static method).
+    ///
+    /// @param name Name of the library to load.  This should be an absolute
+    ///        path name.
+    LibraryManager(const std::string& name);
+
+    // Member variables
+
     void*       dl_handle_;     ///< Handle returned by dlopen
     int         index_;         ///< Index associated with this library
     boost::shared_ptr<CalloutManager> manager_;
diff --git a/src/lib/hooks/library_manager_collection.cc b/src/lib/hooks/library_manager_collection.cc
index d3f7f34..b9122e2 100644
--- a/src/lib/hooks/library_manager_collection.cc
+++ b/src/lib/hooks/library_manager_collection.cc
@@ -73,24 +73,18 @@ LibraryManagerCollection::loadLibraries() {
                                    callout_manager_));
 
         // Load the library.  On success, add it to the list of loaded
-        // libraries.  On failure, an error will have been logged and the
-        // library closed.
+        // libraries.  On failure, unload all currently loaded libraries,
+        // leaving the object in the state it was in before loadLibraries was
+        // called.
         if (manager->loadLibrary()) {
             lib_managers_.push_back(manager);
+        } else {
+            static_cast<void>(unloadLibraries());
+            return (false);
         }
     }
 
-    // Update the CalloutManager's idea of the number of libraries it is
-    // handling.
-    callout_manager_->setNumLibraries(lib_managers_.size());
-
-    // Get an indication of whether all libraries loaded successfully.
-    bool status = (library_names_.size() == lib_managers_.size());
-
-    // Don't need the library names any more, so free up the space.
-    library_names_.clear();
-
-    return (status);
+    return (true);
 }
 
 // Unload the libraries.
@@ -110,5 +104,26 @@ LibraryManagerCollection::unloadLibraries() {
     callout_manager_.reset();
 }
 
+// Return number of loaded libraries.
+int
+LibraryManagerCollection::getLoadedLibraryCount() const {
+    return (lib_managers_.size());
+}
+
+// Validate the libraries.
+std::vector<std::string>
+LibraryManagerCollection::validateLibraries(
+                          const std::vector<std::string>& libraries) {
+
+    std::vector<std::string> failures;
+    for (int i = 0; i < libraries.size(); ++i) {
+        if (!LibraryManager::validateLibrary(libraries[i])) {
+            failures.push_back(libraries[i]);
+        }
+    }
+
+    return (failures);
+}
+
 } // namespace hooks
 } // namespace isc
diff --git a/src/lib/hooks/library_manager_collection.h b/src/lib/hooks/library_manager_collection.h
index c0f6def..0a255ba 100644
--- a/src/lib/hooks/library_manager_collection.h
+++ b/src/lib/hooks/library_manager_collection.h
@@ -69,13 +69,17 @@ class LibraryManager;
 /// code.  However, the link with the CalloutHandle does at least mean that
 /// authors of server code do not need to be so careful about when they destroy
 /// CalloutHandles.
+///
+/// The collection object also provides a utility function to validate a set
+/// of libraries.  The function checks that each library exists, can be opened,
+/// that the "version" function exists and return the right number.
 
 class LibraryManagerCollection {
 public:
     /// @brief Constructor
     ///
-    /// @param List of libraries that this collection will manage.  The order
-    ///        of the libraries is important.
+    /// @param libraries List of libraries that this collection will manage.
+    ///        The order of the libraries is important.
     LibraryManagerCollection(const std::vector<std::string>& libraries)
         : library_names_(libraries)
     {}
@@ -91,8 +95,11 @@ public:
     ///
     /// Loads the libraries.  This creates the LibraryManager associated with
     /// each library and calls its loadLibrary() method.  If a library fails
-    /// to load, the fact is noted but attempts are made to load the remaining
-    /// libraries.
+    /// to load, the loading is abandoned and all libraries loaded so far
+    /// are unloaded.
+    ///
+    /// @return true if all libraries loaded, false if one or more failed t
+    ////        load.
     bool loadLibraries();
 
     /// @brief Get callout manager
@@ -107,6 +114,36 @@ public:
     ///        construction and the time loadLibraries() is called.
     boost::shared_ptr<CalloutManager> getCalloutManager() const;
 
+    /// @brief Get library names
+    ///
+    /// Returns the list of library names.  If called before loadLibraries(),
+    /// the list is the list of names to be loaded; if called afterwards, it
+    /// is the list of libraries that have been loaded.
+    std::vector<std::string> getLibraryNames() const {
+        return (library_names_);
+    }
+
+    /// @brief Get number of loaded libraries
+    ///
+    /// Mainly for testing, this returns the number of libraries that are
+    /// loaded.
+    ///
+    /// @return Number of libraries that are loaded.
+    int getLoadedLibraryCount() const;
+
+    /// @brief Validate libraries
+    ///
+    /// Utility function to validate libraries.  It checks that the libraries
+    /// exist, can be opened, that a "version" function is present in them, and
+    /// that it returns the right number.  All errors are logged.
+    ///
+    /// @param libraries List of libraries to validate
+    ///
+    /// @return Vector of libraries that faled to validate, or an empty vector
+    ///         if all validated.
+    static std::vector<std::string>
+    validateLibraries(const std::vector<std::string>& libraries);
+
 protected:
     /// @brief Unload libraries
     ///
diff --git a/src/lib/hooks/tests/callout_manager_unittest.cc b/src/lib/hooks/tests/callout_manager_unittest.cc
index 935987a..c3f3b1d 100644
--- a/src/lib/hooks/tests/callout_manager_unittest.cc
+++ b/src/lib/hooks/tests/callout_manager_unittest.cc
@@ -199,10 +199,6 @@ TEST_F(CalloutManagerTest, NumberOfLibraries) {
 
     EXPECT_NO_THROW(cm.reset(new CalloutManager(42)));
     EXPECT_EQ(42, cm->getNumLibraries());
-
-    // Check that setting the number of libraries alterns the number reported.
-    EXPECT_NO_THROW(cm->setNumLibraries(27));
-    EXPECT_EQ(27, cm->getNumLibraries());
 }
 
 // Check that we can only set the current library index to the correct values.
diff --git a/src/lib/hooks/tests/hooks_manager_unittest.cc b/src/lib/hooks/tests/hooks_manager_unittest.cc
index c5dba60..136eeae 100644
--- a/src/lib/hooks/tests/hooks_manager_unittest.cc
+++ b/src/lib/hooks/tests/hooks_manager_unittest.cc
@@ -157,33 +157,6 @@ TEST_F(HooksManagerTest, LoadLibrariesWithError) {
     // Load the libraries.  We expect a failure return because one of the
     // libraries fails to load.
     EXPECT_FALSE(HooksManager::loadLibraries(library_names));
-
-    // Execute the callouts.  The first library implements the calculation.
-    //
-    // r3 = (7 * d1 - d2) * d3
-    //
-    // The last-loaded library implements the calculation
-    //
-    // r3 = (10 + d1) * d2 - d3
-    //
-    // Putting the processing for each library together in the appropriate
-    // order, we get:
-    //
-    // r3 = ((10 * d1 + d1) - d2) * d2 * d3 - d3
-    {
-        SCOPED_TRACE("Calculation with libraries loaded");
-        executeCallCallouts(10, 3, 33, 2, 62, 3, 183);
-    }
-
-    // Try unloading the libraries.
-    EXPECT_NO_THROW(HooksManager::unloadLibraries());
-
-    // Re-execute the calculation - callouts can be called but as nothing
-    // happens, the result should always be -1.
-    {
-        SCOPED_TRACE("Calculation with libraries not loaded");
-        executeCallCallouts(-1, 3, -1, 22, -1, 83, -1);
-    }
 }
 
 // Test that we can unload a set of libraries while we have a CalloutHandle
@@ -450,5 +423,102 @@ TEST_F(HooksManagerTest, RegisterHooks) {
     EXPECT_EQ(string("gamma"), names[4]);
 }
 
+// Check that we can get the names of the libraries.
+
+TEST_F(HooksManagerTest, LibraryNames) {
+
+    // Set up the list of libraries to be loaded.
+    std::vector<std::string> library_names;
+    library_names.push_back(std::string(FULL_CALLOUT_LIBRARY));
+    library_names.push_back(std::string(BASIC_CALLOUT_LIBRARY));
+
+    // Check the names before the libraries are loaded.
+    std::vector<std::string> loaded_names = HooksManager::getLibraryNames();
+    EXPECT_TRUE(loaded_names.empty());
+
+    // Load the libraries and check the names again.
+    EXPECT_TRUE(HooksManager::loadLibraries(library_names));
+    loaded_names = HooksManager::getLibraryNames();
+    EXPECT_TRUE(library_names == loaded_names);
+
+    // Unload the libraries and check again.
+    EXPECT_NO_THROW(HooksManager::unloadLibraries());
+    loaded_names = HooksManager::getLibraryNames();
+    EXPECT_TRUE(loaded_names.empty());
+}
+
+// Test the library validation function.
+
+TEST_F(HooksManagerTest, validateLibraries) {
+    // Vector of libraries that failed validation
+    std::vector<std::string> failed;
+
+    // Test different vectors of libraries.
+
+    // No libraries should return a success.
+    std::vector<std::string> libraries;
+
+    failed = HooksManager::validateLibraries(libraries);
+    EXPECT_TRUE(failed.empty());
+
+    // Single valid library should validate.
+    libraries.clear();
+    libraries.push_back(BASIC_CALLOUT_LIBRARY);
+
+    failed = HooksManager::validateLibraries(libraries);
+    EXPECT_TRUE(failed.empty());
+
+    // Multiple valid libraries should succeed.
+    libraries.clear();
+    libraries.push_back(BASIC_CALLOUT_LIBRARY);
+    libraries.push_back(FULL_CALLOUT_LIBRARY);
+    libraries.push_back(UNLOAD_CALLOUT_LIBRARY);
+
+    failed = HooksManager::validateLibraries(libraries);
+    EXPECT_TRUE(failed.empty());
+
+    // Single invalid library should fail.
+    libraries.clear();
+    libraries.push_back(NOT_PRESENT_LIBRARY);
+
+    failed = HooksManager::validateLibraries(libraries);
+    EXPECT_TRUE(failed == libraries);
+
+    // Multiple invalid libraries should fail.
+    libraries.clear();
+    libraries.push_back(INCORRECT_VERSION_LIBRARY);
+    libraries.push_back(NO_VERSION_LIBRARY);
+    libraries.push_back(FRAMEWORK_EXCEPTION_LIBRARY);
+
+    failed = HooksManager::validateLibraries(libraries);
+    EXPECT_TRUE(failed == libraries);
+
+    // Combination of valid and invalid (first one valid) should fail.
+    libraries.clear();
+    libraries.push_back(FULL_CALLOUT_LIBRARY);
+    libraries.push_back(INCORRECT_VERSION_LIBRARY);
+    libraries.push_back(NO_VERSION_LIBRARY);
+
+    std::vector<std::string> expected_failures;
+    expected_failures.push_back(INCORRECT_VERSION_LIBRARY);
+    expected_failures.push_back(NO_VERSION_LIBRARY);
+
+    failed = HooksManager::validateLibraries(libraries);
+    EXPECT_TRUE(failed == expected_failures);
+
+    // Combination of valid and invalid (first one invalid) should fail.
+    libraries.clear();
+    libraries.push_back(NO_VERSION_LIBRARY);
+    libraries.push_back(FULL_CALLOUT_LIBRARY);
+    libraries.push_back(INCORRECT_VERSION_LIBRARY);
+
+    expected_failures.clear();
+    expected_failures.push_back(NO_VERSION_LIBRARY);
+    expected_failures.push_back(INCORRECT_VERSION_LIBRARY);
+
+    failed = HooksManager::validateLibraries(libraries);
+    EXPECT_TRUE(failed == expected_failures);
+}
+
 
 } // Anonymous namespace
diff --git a/src/lib/hooks/tests/library_manager_collection_unittest.cc b/src/lib/hooks/tests/library_manager_collection_unittest.cc
index 549142f..7fdbb7d 100644
--- a/src/lib/hooks/tests/library_manager_collection_unittest.cc
+++ b/src/lib/hooks/tests/library_manager_collection_unittest.cc
@@ -76,8 +76,7 @@ TEST_F(LibraryManagerCollectionTest, LoadLibraries) {
 
     // Load the libraries.
     EXPECT_TRUE(lm_collection.loadLibraries());
-    boost::shared_ptr<CalloutManager> manager =
-                                      lm_collection.getCalloutManager();
+    EXPECT_EQ(2, lm_collection.getLoadedLibraryCount());
 
     // Execute the callouts.  The first library implements the calculation.
     //
@@ -91,6 +90,8 @@ TEST_F(LibraryManagerCollectionTest, LoadLibraries) {
     // order, we get:
     //
     // r3 = ((10 * d1 + d1) - d2) * d2 * d3 - d3
+    boost::shared_ptr<CalloutManager> manager =
+                                      lm_collection.getCalloutManager();
     {
         SCOPED_TRACE("Doing calculation with libraries loaded");
         executeCallCallouts(manager, 10, 3, 33, 2, 62, 3, 183);
@@ -98,6 +99,7 @@ TEST_F(LibraryManagerCollectionTest, LoadLibraries) {
 
     // Try unloading the libraries.
     EXPECT_NO_THROW(lm_collection.unloadLibraries());
+    EXPECT_EQ(0, lm_collection.getLoadedLibraryCount());
 
     // Re-execute the calculation - callouts can be called but as nothing
     // happens, the result should always be -1.
@@ -108,8 +110,7 @@ TEST_F(LibraryManagerCollectionTest, LoadLibraries) {
 }
 
 // This is effectively the same test as above, but with a library generating
-// an error when loaded. It is expected that the failing library will not be
-// loaded, but others will be.
+// an error when loaded. It is expected that no libraries will be loaded.
 
 TEST_F(LibraryManagerCollectionTest, LoadLibrariesWithError) {
 
@@ -126,38 +127,9 @@ TEST_F(LibraryManagerCollectionTest, LoadLibrariesWithError) {
     // Load the libraries.  We expect a failure status to be returned as
     // one of the libraries failed to load.
     EXPECT_FALSE(lm_collection.loadLibraries());
-    boost::shared_ptr<CalloutManager> manager =
-                                      lm_collection.getCalloutManager();
-
-    // Expect only two libraries were loaded.
-    EXPECT_EQ(2, manager->getNumLibraries());
-
-    // Execute the callouts.  The first library implements the calculation.
-    //
-    // r3 = (7 * d1 - d2) * d3
-    //
-    // The last-loaded library implements the calculation
-    //
-    // r3 = (10 + d1) * d2 - d3
-    //
-    // Putting the processing for each library together in the appropriate
-    // order, we get:
-    //
-    // r3 = ((10 * d1 + d1) - d2) * d2 * d3 - d3
-    {
-        SCOPED_TRACE("Doing calculation with libraries loaded");
-        executeCallCallouts(manager, 10, 3, 33, 2, 62, 3, 183);
-    }
-
-    // Try unloading the libraries.
-    EXPECT_NO_THROW(lm_collection.unloadLibraries());
 
-    // Re-execute the calculation - callouts can be called but as nothing
-    // happens, the result should always be -1.
-    {
-        SCOPED_TRACE("Doing calculation with libraries not loaded");
-        executeCallCallouts(manager, -1, 3, -1, 22, -1, 83, -1);
-    }
+    // Expect no libraries were loaded.
+    EXPECT_EQ(0, lm_collection.getLoadedLibraryCount());
 }
 
 // Check that everything works even with no libraries loaded.
@@ -170,15 +142,110 @@ TEST_F(LibraryManagerCollectionTest, NoLibrariesLoaded) {
     // be using.
     LibraryManagerCollection lm_collection(library_names);
     EXPECT_TRUE(lm_collection.loadLibraries());
+    EXPECT_EQ(0, lm_collection.getLoadedLibraryCount());
     boost::shared_ptr<CalloutManager> manager =
                                       lm_collection.getCalloutManager();
 
-    // Load the libraries.
-    EXPECT_TRUE(lm_collection.loadLibraries());
-
     // Eecute the calculation - callouts can be called but as nothing
     // happens, the result should always be -1.
     executeCallCallouts(manager, -1, 3, -1, 22, -1, 83, -1);
 }
 
+// Check that we can get the names of the libraries.
+
+TEST_F(LibraryManagerCollectionTest, LibraryNames) {
+
+    // Set up the list of libraries to be loaded.
+    std::vector<std::string> library_names;
+    library_names.push_back(std::string(FULL_CALLOUT_LIBRARY));
+    library_names.push_back(std::string(BASIC_CALLOUT_LIBRARY));
+
+    // Set up the library manager collection and get the callout manager we'll
+    // be using.
+    PublicLibraryManagerCollection lm_collection(library_names);
+
+    // Check the names before the libraries are loaded.
+    std::vector<std::string> collection_names = lm_collection.getLibraryNames();
+    EXPECT_TRUE(library_names == collection_names);
+
+    // Load the libraries and check the names again.
+    EXPECT_TRUE(lm_collection.loadLibraries());
+    EXPECT_EQ(2, lm_collection.getLoadedLibraryCount());
+    collection_names = lm_collection.getLibraryNames();
+    EXPECT_TRUE(library_names == collection_names);
+}
+
+// Test the library validation function.
+
+TEST_F(LibraryManagerCollectionTest, validateLibraries) {
+    // Vector of libraries that failed validation
+    std::vector<std::string> failed;
+
+    // Test different vectors of libraries.
+
+    // No libraries should return a success.
+    std::vector<std::string> libraries;
+
+    failed = LibraryManagerCollection::validateLibraries(libraries);
+    EXPECT_TRUE(failed.empty());
+
+    // Single valid library should validate.
+    libraries.clear();
+    libraries.push_back(BASIC_CALLOUT_LIBRARY);
+
+    failed = LibraryManagerCollection::validateLibraries(libraries);
+    EXPECT_TRUE(failed.empty());
+
+    // Multiple valid libraries should succeed.
+    libraries.clear();
+    libraries.push_back(BASIC_CALLOUT_LIBRARY);
+    libraries.push_back(FULL_CALLOUT_LIBRARY);
+    libraries.push_back(UNLOAD_CALLOUT_LIBRARY);
+
+    failed = LibraryManagerCollection::validateLibraries(libraries);
+    EXPECT_TRUE(failed.empty());
+
+    // Single invalid library should fail.
+    libraries.clear();
+    libraries.push_back(NOT_PRESENT_LIBRARY);
+
+    failed = LibraryManagerCollection::validateLibraries(libraries);
+    EXPECT_TRUE(failed == libraries);
+
+    // Multiple invalid libraries should fail.
+    libraries.clear();
+    libraries.push_back(INCORRECT_VERSION_LIBRARY);
+    libraries.push_back(NO_VERSION_LIBRARY);
+    libraries.push_back(FRAMEWORK_EXCEPTION_LIBRARY);
+
+    failed = LibraryManagerCollection::validateLibraries(libraries);
+    EXPECT_TRUE(failed == libraries);
+
+    // Combination of valid and invalid (first one valid) should fail.
+    libraries.clear();
+    libraries.push_back(FULL_CALLOUT_LIBRARY);
+    libraries.push_back(INCORRECT_VERSION_LIBRARY);
+    libraries.push_back(NO_VERSION_LIBRARY);
+
+    std::vector<std::string> expected_failures;
+    expected_failures.push_back(INCORRECT_VERSION_LIBRARY);
+    expected_failures.push_back(NO_VERSION_LIBRARY);
+
+    failed = LibraryManagerCollection::validateLibraries(libraries);
+    EXPECT_TRUE(failed == expected_failures);
+
+    // Combination of valid and invalid (first one invalid) should fail.
+    libraries.clear();
+    libraries.push_back(NO_VERSION_LIBRARY);
+    libraries.push_back(FULL_CALLOUT_LIBRARY);
+    libraries.push_back(INCORRECT_VERSION_LIBRARY);
+
+    expected_failures.clear();
+    expected_failures.push_back(NO_VERSION_LIBRARY);
+    expected_failures.push_back(INCORRECT_VERSION_LIBRARY);
+
+    failed = LibraryManagerCollection::validateLibraries(libraries);
+    EXPECT_TRUE(failed == expected_failures);
+}
+
 } // Anonymous namespace
diff --git a/src/lib/hooks/tests/library_manager_unittest.cc b/src/lib/hooks/tests/library_manager_unittest.cc
index c2f8cb7..9336946 100644
--- a/src/lib/hooks/tests/library_manager_unittest.cc
+++ b/src/lib/hooks/tests/library_manager_unittest.cc
@@ -552,4 +552,18 @@ TEST_F(LibraryManagerTest, LoadMultipleLibraries) {
     EXPECT_TRUE(lib_manager_4.unloadLibrary());
 }
 
+// Check that libraries can be validated.
+
+TEST_F(LibraryManagerTest, validateLibraries) {
+    EXPECT_TRUE(LibraryManager::validateLibrary(BASIC_CALLOUT_LIBRARY));
+    EXPECT_TRUE(LibraryManager::validateLibrary(FULL_CALLOUT_LIBRARY));
+    EXPECT_FALSE(LibraryManager::validateLibrary(FRAMEWORK_EXCEPTION_LIBRARY));
+    EXPECT_FALSE(LibraryManager::validateLibrary(INCORRECT_VERSION_LIBRARY));
+    EXPECT_TRUE(LibraryManager::validateLibrary(LOAD_CALLOUT_LIBRARY));
+    EXPECT_TRUE(LibraryManager::validateLibrary(LOAD_ERROR_CALLOUT_LIBRARY));
+    EXPECT_FALSE(LibraryManager::validateLibrary(NOT_PRESENT_LIBRARY));
+    EXPECT_FALSE(LibraryManager::validateLibrary(NO_VERSION_LIBRARY));
+    EXPECT_TRUE(LibraryManager::validateLibrary(UNLOAD_CALLOUT_LIBRARY));
+}
+
 } // Anonymous namespace



More information about the bind10-changes mailing list