Browse Source

[3054] Added a "validate libraries" function

When the BIND 10 configuration is changed, any changes to the hooks
libraries need to be validated.  The validation includes checking that
the libraries exist, that the "version()" function is present, and
that it returns the correct value.
Stephen Morris 11 years ago
parent
commit
a6c4e80483

+ 13 - 12
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

+ 1 - 27
src/lib/hooks/callout_manager.h

@@ -134,17 +134,7 @@ public:
     ///
     /// @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);
-    }
+    CalloutManager(int num_libraries = 0);
 
     /// @brief Register a callout on a hook for the current library
     ///
@@ -224,22 +214,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

+ 29 - 2
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::string
+HooksManager::validateLibraries(const std::vector<std::string>& libraries) {
+    return (LibraryManagerCollection::validateLibraries(libraries));
+}
+
 } // namespace util
 } // namespace isc

+ 61 - 0
src/lib/hooks/hooks_manager.h

@@ -49,6 +49,27 @@ public:
     /// @return Reference to the singleton hooks manager.
     static HooksManager& getHooksManager();
 
+    /// @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 string if all libraries validated.  Otherwise it is
+    ///         the names of the libraries that failed validation, separated
+    ///         by a command and a space.  The configuration code can return
+    ///         this to bindctl as an indication of the problem. (Note that
+    ///         validation failures are logged, so more information can be
+    ///         obtained if necessary.)
+    static std::string validateLibraries(
+                       const std::vector<std::string>& libraries);
+
     /// @brief Load and reload libraries
     ///
     /// Loads the list of libraries into the server address space.  For each
@@ -171,6 +192,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 set of libraries
+    ///
+    /// Validates the names of the libraries passed to it.  The function checks
+    /// that the libraries exist, that they contain a "version" function and
+    /// that it returns the right value.
+    ///
+    /// This is really just a wrapper around the LibraryManagerCollection
+    /// static method of the same name, and is supplied so that the server
+    /// does not have to know about that object.
+    ///
+    /// @param libraries Names of the libraries to validate
+    ///
+    /// @return Comma-separated list of libraries that failed to validate,
+    ///         empty string if not. (Actually, if the list of failures is
+    ///         more than one, each item is separated by a command and a space.)
+    static std::string validateLibraries();
+
     /// Index numbers for pre-defined hooks.
     static const int CONTEXT_CREATE = ServerHooks::CONTEXT_CREATE;
     static const int CONTEXT_DESTROY = ServerHooks::CONTEXT_DESTROY;
@@ -188,6 +233,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 +290,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

+ 88 - 27
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

+ 50 - 9
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_;

+ 31 - 13
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,29 @@ LibraryManagerCollection::unloadLibraries() {
     callout_manager_.reset();
 }
 
+// Return number of loaded libraries.
+int
+LibraryManagerCollection::getLoadedLibraryCount() const {
+    return (lib_managers_.size());
+}
+
+// Validate the libraries.
+std::string
+LibraryManagerCollection::validateLibraries(
+                          const std::vector<std::string>& libraries) {
+
+    std::string failures("");
+    for (int i = 0; i < libraries.size(); ++i) {
+        if (!LibraryManager::validateLibrary(libraries[i])) {
+            if (!failures.empty()) {
+                failures += std::string(", ");
+            }
+            failures += libraries[i];
+        }
+    }
+
+    return (failures);
+}
+
 } // namespace hooks
 } // namespace isc

+ 41 - 4
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 Comma-separated list of libraries that faled to validate, or
+    ///         the empty string if all validated.
+    static std::string
+    validateLibraries(const std::vector<std::string>& libraries);
+
 protected:
     /// @brief Unload libraries
     ///

+ 0 - 4
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.

+ 85 - 27
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,90 @@ 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) {
+    const std::string empty;
+    const std::string separator(", ");
+
+    // Test different vectors of libraries.
+
+    // No libraries should return a success.
+    std::vector<std::string> libraries;
+    EXPECT_EQ(empty, HooksManager::validateLibraries(libraries));
+
+    // Single valid library should validate.
+    libraries.clear();
+    libraries.push_back(BASIC_CALLOUT_LIBRARY);
+    EXPECT_EQ(empty, HooksManager::validateLibraries(libraries));
+
+    // 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);
+    EXPECT_EQ(empty, HooksManager::validateLibraries(libraries));
+
+    // Single invalid library should fail.
+    libraries.clear();
+    libraries.push_back(NOT_PRESENT_LIBRARY);
+    EXPECT_EQ(std::string(NOT_PRESENT_LIBRARY),
+              HooksManager::validateLibraries(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);
+    std::string expected = std::string(INCORRECT_VERSION_LIBRARY) + separator +
+                           std::string(NO_VERSION_LIBRARY) + separator +
+                           std::string(FRAMEWORK_EXCEPTION_LIBRARY);
+    EXPECT_EQ(expected, HooksManager::validateLibraries(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);
+
+    expected = std::string(INCORRECT_VERSION_LIBRARY) + separator +
+               std::string(NO_VERSION_LIBRARY);
+    EXPECT_EQ(expected, HooksManager::validateLibraries(libraries));
+
+    // 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 = std::string(NO_VERSION_LIBRARY) + separator +
+               std::string(INCORRECT_VERSION_LIBRARY);
+    EXPECT_EQ(expected, HooksManager::validateLibraries(libraries));
+}
+
 
 } // Anonymous namespace

+ 93 - 38
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,98 @@ 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) {
+    const std::string empty;
+    const std::string separator(", ");
+
+    // Test different vectors of libraries.
+
+    // No libraries should return a success.
+    std::vector<std::string> libraries;
+    EXPECT_EQ(empty, LibraryManagerCollection::validateLibraries(libraries));
+
+    // Single valid library should validate.
+    libraries.clear();
+    libraries.push_back(BASIC_CALLOUT_LIBRARY);
+    EXPECT_EQ(empty, LibraryManagerCollection::validateLibraries(libraries));
+
+    // 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);
+    EXPECT_EQ(empty, LibraryManagerCollection::validateLibraries(libraries));
+
+    // Single invalid library should fail.
+    libraries.clear();
+    libraries.push_back(NOT_PRESENT_LIBRARY);
+    EXPECT_EQ(std::string(NOT_PRESENT_LIBRARY),
+              LibraryManagerCollection::validateLibraries(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);
+    std::string expected = std::string(INCORRECT_VERSION_LIBRARY) + separator +
+                           std::string(NO_VERSION_LIBRARY) + separator +
+                           std::string(FRAMEWORK_EXCEPTION_LIBRARY);
+    EXPECT_EQ(expected, LibraryManagerCollection::validateLibraries(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);
+
+    expected = std::string(INCORRECT_VERSION_LIBRARY) + separator +
+               std::string(NO_VERSION_LIBRARY);
+    EXPECT_EQ(expected, LibraryManagerCollection::validateLibraries(libraries));
+
+    // 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 = std::string(NO_VERSION_LIBRARY) + separator +
+               std::string(INCORRECT_VERSION_LIBRARY);
+    EXPECT_EQ(expected, LibraryManagerCollection::validateLibraries(libraries));
+}
+
 } // Anonymous namespace

+ 14 - 0
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