BIND 10 trac3054, updated. 088a0124e731718b6e8be8092a47a087436ec1dc [3054] Changes as a result of review
BIND 10 source code commits
bind10-changes at lists.isc.org
Fri Jul 19 13:43:57 UTC 2013
The branch, trac3054 has been updated
via 088a0124e731718b6e8be8092a47a087436ec1dc (commit)
from 8e13f44e871bde3f44ffc080a1996149d61fc13d (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 088a0124e731718b6e8be8092a47a087436ec1dc
Author: Stephen Morris <stephen at isc.org>
Date: Fri Jul 19 14:42:53 2013 +0100
[3054] Changes as a result of review
The main change is that the list of libraries failing validation is
returned as a vector of strings instead of as a single comma-separated
string.
-----------------------------------------------------------------------
Summary of changes:
src/lib/hooks/callout_manager.h | 3 +-
src/lib/hooks/hooks_manager.cc | 2 +-
src/lib/hooks/hooks_manager.h | 10 ++---
src/lib/hooks/library_manager_collection.cc | 9 ++--
src/lib/hooks/library_manager_collection.h | 6 +--
src/lib/hooks/tests/hooks_manager_unittest.cc | 46 ++++++++++++--------
.../tests/library_manager_collection_unittest.cc | 46 ++++++++++++--------
7 files changed, 69 insertions(+), 53 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/hooks/callout_manager.h b/src/lib/hooks/callout_manager.h
index 8608f4d..e1b9e57 100644
--- a/src/lib/hooks/callout_manager.h
+++ b/src/lib/hooks/callout_manager.h
@@ -132,8 +132,7 @@ 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.
+ /// @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
diff --git a/src/lib/hooks/hooks_manager.cc b/src/lib/hooks/hooks_manager.cc
index eb9b17f..117db61 100644
--- a/src/lib/hooks/hooks_manager.cc
+++ b/src/lib/hooks/hooks_manager.cc
@@ -191,7 +191,7 @@ HooksManager::postCalloutsLibraryHandle() {
// Validate libraries
-std::string
+std::vector<std::string>
HooksManager::validateLibraries(const std::vector<std::string>& libraries) {
return (LibraryManagerCollection::validateLibraries(libraries));
}
diff --git a/src/lib/hooks/hooks_manager.h b/src/lib/hooks/hooks_manager.h
index ed33d4d..53a2525 100644
--- a/src/lib/hooks/hooks_manager.h
+++ b/src/lib/hooks/hooks_manager.h
@@ -190,13 +190,9 @@ public:
///
/// @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(
+ /// @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.
diff --git a/src/lib/hooks/library_manager_collection.cc b/src/lib/hooks/library_manager_collection.cc
index ef77717..b9122e2 100644
--- a/src/lib/hooks/library_manager_collection.cc
+++ b/src/lib/hooks/library_manager_collection.cc
@@ -111,17 +111,14 @@ LibraryManagerCollection::getLoadedLibraryCount() const {
}
// Validate the libraries.
-std::string
+std::vector<std::string>
LibraryManagerCollection::validateLibraries(
const std::vector<std::string>& libraries) {
- std::string failures("");
+ std::vector<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];
+ failures.push_back(libraries[i]);
}
}
diff --git a/src/lib/hooks/library_manager_collection.h b/src/lib/hooks/library_manager_collection.h
index e45d8c1..0a255ba 100644
--- a/src/lib/hooks/library_manager_collection.h
+++ b/src/lib/hooks/library_manager_collection.h
@@ -139,9 +139,9 @@ public:
///
/// @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
+ /// @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:
diff --git a/src/lib/hooks/tests/hooks_manager_unittest.cc b/src/lib/hooks/tests/hooks_manager_unittest.cc
index c7f0100..136eeae 100644
--- a/src/lib/hooks/tests/hooks_manager_unittest.cc
+++ b/src/lib/hooks/tests/hooks_manager_unittest.cc
@@ -450,42 +450,48 @@ TEST_F(HooksManagerTest, LibraryNames) {
// Test the library validation function.
TEST_F(HooksManagerTest, validateLibraries) {
- const std::string empty;
- const std::string separator(", ");
+ // 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;
- EXPECT_EQ(empty, HooksManager::validateLibraries(libraries));
+
+ failed = HooksManager::validateLibraries(libraries);
+ EXPECT_TRUE(failed.empty());
// Single valid library should validate.
libraries.clear();
libraries.push_back(BASIC_CALLOUT_LIBRARY);
- EXPECT_EQ(empty, HooksManager::validateLibraries(libraries));
+
+ 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);
- EXPECT_EQ(empty, HooksManager::validateLibraries(libraries));
+
+ failed = HooksManager::validateLibraries(libraries);
+ EXPECT_TRUE(failed.empty());
// Single invalid library should fail.
libraries.clear();
libraries.push_back(NOT_PRESENT_LIBRARY);
- EXPECT_EQ(std::string(NOT_PRESENT_LIBRARY),
- HooksManager::validateLibraries(libraries));
+
+ 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);
- 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));
+
+ failed = HooksManager::validateLibraries(libraries);
+ EXPECT_TRUE(failed == libraries);
// Combination of valid and invalid (first one valid) should fail.
libraries.clear();
@@ -493,9 +499,12 @@ TEST_F(HooksManagerTest, validateLibraries) {
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));
+ 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();
@@ -503,9 +512,12 @@ TEST_F(HooksManagerTest, validateLibraries) {
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));
+ 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);
}
diff --git a/src/lib/hooks/tests/library_manager_collection_unittest.cc b/src/lib/hooks/tests/library_manager_collection_unittest.cc
index 1a9951b..7fdbb7d 100644
--- a/src/lib/hooks/tests/library_manager_collection_unittest.cc
+++ b/src/lib/hooks/tests/library_manager_collection_unittest.cc
@@ -178,42 +178,48 @@ TEST_F(LibraryManagerCollectionTest, LibraryNames) {
// Test the library validation function.
TEST_F(LibraryManagerCollectionTest, validateLibraries) {
- const std::string empty;
- const std::string separator(", ");
+ // 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;
- EXPECT_EQ(empty, LibraryManagerCollection::validateLibraries(libraries));
+
+ failed = LibraryManagerCollection::validateLibraries(libraries);
+ EXPECT_TRUE(failed.empty());
// Single valid library should validate.
libraries.clear();
libraries.push_back(BASIC_CALLOUT_LIBRARY);
- EXPECT_EQ(empty, LibraryManagerCollection::validateLibraries(libraries));
+
+ 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);
- EXPECT_EQ(empty, LibraryManagerCollection::validateLibraries(libraries));
+
+ failed = LibraryManagerCollection::validateLibraries(libraries);
+ EXPECT_TRUE(failed.empty());
// Single invalid library should fail.
libraries.clear();
libraries.push_back(NOT_PRESENT_LIBRARY);
- EXPECT_EQ(std::string(NOT_PRESENT_LIBRARY),
- LibraryManagerCollection::validateLibraries(libraries));
+
+ 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);
- 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));
+
+ failed = LibraryManagerCollection::validateLibraries(libraries);
+ EXPECT_TRUE(failed == libraries);
// Combination of valid and invalid (first one valid) should fail.
libraries.clear();
@@ -221,9 +227,12 @@ TEST_F(LibraryManagerCollectionTest, validateLibraries) {
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));
+ 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();
@@ -231,9 +240,12 @@ TEST_F(LibraryManagerCollectionTest, validateLibraries) {
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));
+ 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
More information about the bind10-changes
mailing list