BIND 10 trac3186, updated. 2c28d84f3657c10e6520c413705f532c0b42b1c8 [3186] Addressed review comments.
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Oct 23 18:09:22 UTC 2013
The branch, trac3186 has been updated
via 2c28d84f3657c10e6520c413705f532c0b42b1c8 (commit)
from 4707a2dbce19333a1c6cb1bc9020ef2051eca1fe (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 2c28d84f3657c10e6520c413705f532c0b42b1c8
Author: Thomas Markwalder <tmark at isc.org>
Date: Wed Oct 23 14:08:10 2013 -0400
[3186] Addressed review comments.
Mostly minor cosmetics. Added logic to select callouts to
simply return if no subnets are configured.
-----------------------------------------------------------------------
Summary of changes:
src/Makefile.am | 2 +-
src/hooks/dhcp/user_chk/load_unload.cc | 13 ++++++----
src/hooks/dhcp/user_chk/subnet_select_co.cc | 21 ++++++++++++----
src/hooks/dhcp/user_chk/tests/Makefile.am | 1 -
.../user_chk/tests/test_data_files_config.h.in | 4 ++--
src/hooks/dhcp/user_chk/user.cc | 25 ++------------------
src/hooks/dhcp/user_chk/user.h | 15 +++---------
src/hooks/dhcp/user_chk/user_chk_log.cc | 5 ++--
src/hooks/dhcp/user_chk/user_chk_messages.mes | 3 ++-
src/hooks/dhcp/user_chk/user_file.cc | 20 ++++++++--------
src/hooks/dhcp/user_chk/user_file.h | 7 +++++-
11 files changed, 54 insertions(+), 62 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/Makefile.am b/src/Makefile.am
index 04e464c..0e0109a 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1,5 +1,5 @@
SUBDIRS = lib bin
-# hooks lib could be a configurable switch
+# @todo hooks lib could be a configurable switch
SUBDIRS += hooks
EXTRA_DIST = \
diff --git a/src/hooks/dhcp/user_chk/load_unload.cc b/src/hooks/dhcp/user_chk/load_unload.cc
index bd14057..79afb82 100644
--- a/src/hooks/dhcp/user_chk/load_unload.cc
+++ b/src/hooks/dhcp/user_chk/load_unload.cc
@@ -30,12 +30,17 @@ UserRegistryPtr user_registry;
/// @brief Output filestream for recording user check outcomes.
std::fstream user_chk_output;
-/// @brief For now, hard-code registry input file name.
-const char* registry_fname = "/tmp/user_registry.txt";
+/// @brief User registry input file name.
+/// @todo Hard-coded for now, this should be configurable.
+const char* registry_fname = "/tmp/user_chk_registry.txt";
-/// @brief For now, hard-code user check outcome file name.
-const char* user_chk_output_fname = "/tmp/user_check_output.txt";
+/// @brief User check outcome file name.
+/// @todo Hard-coded for now, this should be configurable.
+const char* user_chk_output_fname = "/tmp/user_chk_outcome.txt";
+// Functions accessed by the hooks framework use C linkage to avoid the name
+// mangling that accompanies use of the C++ compiler as well as to avoid
+// issues related to namespaces.
extern "C" {
/// @brief Called by the Hooks library manager when the library is loaded.
diff --git a/src/hooks/dhcp/user_chk/subnet_select_co.cc b/src/hooks/dhcp/user_chk/subnet_select_co.cc
index c75fd3f..31fbd6f 100644
--- a/src/hooks/dhcp/user_chk/subnet_select_co.cc
+++ b/src/hooks/dhcp/user_chk/subnet_select_co.cc
@@ -33,6 +33,9 @@ extern std::fstream user_chk_output;
extern const char* registry_fname;
extern const char* user_chk_output_fname;
+// Functions accessed by the hooks framework use C linkage to avoid the name
+// mangling that accompanies use of the C++ compiler as well as to avoid
+// issues related to namespaces.
extern "C" {
/// @brief Adds an entry to the end of the user check outcome file.
@@ -115,6 +118,13 @@ int subnet4_select(CalloutHandle& handle) {
}
try {
+ // Get subnet collection. If it's empty just bail nothing to do.
+ const isc::dhcp::Subnet4Collection *subnets = NULL;
+ handle.getArgument("subnet4collection", subnets);
+ if (subnets->size() == 0) {
+ return 0;
+ }
+
// Refresh the registry.
user_registry->refresh();
@@ -136,8 +146,6 @@ int subnet4_select(CalloutHandle& handle) {
// User is not in the registry, so assign them to the last subnet
// in the collection. By convention we are assuming this is the
// restricted subnet.
- const isc::dhcp::Subnet4Collection *subnets = NULL;
- handle.getArgument("subnet4collection", subnets);
Subnet4Ptr subnet = subnets->back();
handle.setArgument("subnet4", subnet);
// Add the outcome entry to the output file.
@@ -175,6 +183,13 @@ int subnet6_select(CalloutHandle& handle) {
}
try {
+ // Get subnet collection. If it's empty just bail nothing to do.
+ const isc::dhcp::Subnet6Collection *subnets = NULL;
+ handle.getArgument("subnet6collection", subnets);
+ if (subnets->size() == 0) {
+ return 0;
+ }
+
// Refresh the registry.
user_registry->refresh();
@@ -204,8 +219,6 @@ int subnet6_select(CalloutHandle& handle) {
// User is not in the registry, so assign them to the last subnet
// in the collection. By convention we are assuming this is the
// restricted subnet.
- const isc::dhcp::Subnet6Collection *subnets = NULL;
- handle.getArgument("subnet6collection", subnets);
Subnet6Ptr subnet = subnets->back();
handle.setArgument("subnet6", subnet);
// Add the outcome entry to the output file.
diff --git a/src/hooks/dhcp/user_chk/tests/Makefile.am b/src/hooks/dhcp/user_chk/tests/Makefile.am
index 28393b4..8047244 100644
--- a/src/hooks/dhcp/user_chk/tests/Makefile.am
+++ b/src/hooks/dhcp/user_chk/tests/Makefile.am
@@ -64,7 +64,6 @@ libdhcp_user_chk_unittests_LDADD += $(top_builddir)/src/lib/hooks/libb10-hooks.l
libdhcp_user_chk_unittests_LDADD += $(top_builddir)/src/lib/dhcp/libb10-dhcp++.la
libdhcp_user_chk_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
libdhcp_user_chk_unittests_LDADD += $(top_builddir)/src/lib/cc/libb10-cc.la
-#libdhcp_user_chk_unittests_LDADD += $(top_builddir)/src/hooks/dhcp/user_chk/libdhcp_user_chk.la
libdhcp_user_chk_unittests_LDADD += ${BOTAN_LIBS} ${BOTAN_RPATH}
libdhcp_user_chk_unittests_LDADD += $(GTEST_LDADD)
endif
diff --git a/src/hooks/dhcp/user_chk/tests/test_data_files_config.h.in b/src/hooks/dhcp/user_chk/tests/test_data_files_config.h.in
index 019104b..9abdbc6 100644
--- a/src/hooks/dhcp/user_chk/tests/test_data_files_config.h.in
+++ b/src/hooks/dhcp/user_chk/tests/test_data_files_config.h.in
@@ -1,4 +1,4 @@
-// Copyright (C) 2009 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013 Internet Systems Consortium, Inc. ("ISC")
//
// Permission to use, copy, modify, and/or distribute this software for any
// purpose with or without fee is hereby granted, provided that the above
@@ -11,6 +11,6 @@
// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
// PERFORMANCE OF THIS SOFTWARE.
-
+//
/// @brief Path to local dir so tests can locate test data files
#define USER_CHK_TEST_DIR "@abs_top_srcdir@/src/hooks/dhcp/user_chk/tests"
diff --git a/src/hooks/dhcp/user_chk/user.cc b/src/hooks/dhcp/user_chk/user.cc
index 8d53408..c7c18fa 100644
--- a/src/hooks/dhcp/user_chk/user.cc
+++ b/src/hooks/dhcp/user_chk/user.cc
@@ -15,6 +15,7 @@
#include <dhcp/hwaddr.h>
#include <dhcp/duid.h>
#include <exceptions/exceptions.h>
+#include <util/encode/hex.h>
#include <user.h>
@@ -42,7 +43,7 @@ UserId::UserId(UserIdType id_type, const std::string & id_str) :
// Convert the id string to vector.
// Input is expected to be 2-digits per bytes, no delimiters.
std::vector<uint8_t> addr_bytes;
- decodeHex(id_str, addr_bytes);
+ isc::util::encode::decodeHex(id_str, addr_bytes);
// Attempt to instantiate the appropriate id class to leverage validation.
switch (id_type) {
@@ -111,28 +112,6 @@ UserId::operator <(const UserId & other) const {
((this->id_type_ == other.id_type_) && (this->id_ < other.id_)));
}
-void
-UserId::decodeHex(const std::string& input, std::vector<uint8_t>& bytes)
-const {
- int len = input.size();
- if ((len % 2) != 0) {
- isc_throw (isc::BadValue,
- "input string must be event number of digits: "
- << input);
- }
-
- for (int i = 0; i < len / 2; i++) {
- unsigned int tmp;
- if (sscanf (&input[i*2], "%02x", &tmp) != 1) {
- isc_throw (isc::BadValue,
- "input string contains invalid characters: "
- << input);
- }
-
- bytes.push_back(static_cast<uint8_t>(tmp));
- }
-}
-
std::string
UserId::lookupTypeStr(UserIdType type) {
const char* tmp = NULL;
diff --git a/src/hooks/dhcp/user_chk/user.h b/src/hooks/dhcp/user_chk/user.h
index 656ee17..20167b6 100644
--- a/src/hooks/dhcp/user_chk/user.h
+++ b/src/hooks/dhcp/user_chk/user.h
@@ -12,8 +12,8 @@
// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
// PERFORMANCE OF THIS SOFTWARE.
-#ifndef _USER_H
-#define _USER_H
+#ifndef USER_H
+#define USER_H
#include <boost/shared_ptr.hpp>
@@ -23,7 +23,7 @@
/// @file user.h This file defines classes: UserId and User.
/// @brief These classes are used to describe and recognize DHCP lease
-/// requestors (i.e. clients).
+/// clients.
/// @brief Encapsulates a unique identifier for a DHCP client.
/// This class provides a generic wrapper around the information used to
@@ -120,15 +120,6 @@ public:
static UserIdType lookupType(const std::string& type_str);
private:
- /// @brief Converts a string of hex digits to vector of bytes
- ///
- /// @param input string of hex digits to convert
- /// @param bytes vector in which to place the result
- ///
- /// @throw isc::BadValue if input string contains invalid hex digits
- /// or has an odd number of digits.
- void decodeHex(const std::string& input, std::vector<uint8_t>& bytes) const;
-
/// @brief The type of id value
UserIdType id_type_;
diff --git a/src/hooks/dhcp/user_chk/user_chk_log.cc b/src/hooks/dhcp/user_chk/user_chk_log.cc
index 3be5252..f104efb 100644
--- a/src/hooks/dhcp/user_chk/user_chk_log.cc
+++ b/src/hooks/dhcp/user_chk/user_chk_log.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2011 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013 Internet Systems Consortium, Inc. ("ISC")
//
// Permission to use, copy, modify, and/or distribute this software for any
// purpose with or without fee is hereby granted, provided that the above
@@ -12,8 +12,7 @@
// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
// PERFORMANCE OF THIS SOFTWARE.
-/// Defines the logger used by the NSAS
-
+/// Defines the logger used by the user check hooks library.
#include <user_chk_log.h>
isc::log::Logger user_chk_logger("user_chk");
diff --git a/src/hooks/dhcp/user_chk/user_chk_messages.mes b/src/hooks/dhcp/user_chk/user_chk_messages.mes
index 6becd8c..23e3023 100644
--- a/src/hooks/dhcp/user_chk/user_chk_messages.mes
+++ b/src/hooks/dhcp/user_chk/user_chk_messages.mes
@@ -14,7 +14,8 @@
% USER_CHK_HOOK_LOAD_ERROR DHCP UserCheckHook could not be loaded: %1
This is an error message issued when the DHCP UserCheckHook could not be loaded.
-The exact cause should be explained in the log message. User subnet selection will revert to default processing.
+The exact cause should be explained in the log message. User subnet selection
+will revert to default processing.
% USER_CHK_HOOK_UNLOAD_ERROR DHCP UserCheckHook an error occurred unloading the library: %1
This is an error message issued when an error occurs while unloading the
diff --git a/src/hooks/dhcp/user_chk/user_file.cc b/src/hooks/dhcp/user_chk/user_file.cc
index 83f1b19..4181dc4 100644
--- a/src/hooks/dhcp/user_chk/user_file.cc
+++ b/src/hooks/dhcp/user_chk/user_file.cc
@@ -20,7 +20,7 @@
#include <errno.h>
#include <iostream>
-UserFile::UserFile(const std::string& fname) : fname_(fname), ifs_() {
+UserFile::UserFile(const std::string& fname) : fname_(fname), file_() {
if (fname_.empty()) {
isc_throw(UserFileError, "file name cannot be blank");
}
@@ -36,9 +36,9 @@ UserFile::open() {
isc_throw (UserFileError, "file is already open");
}
- ifs_.open(fname_.c_str(), std::ifstream::in);
+ file_.open(fname_.c_str(), std::ifstream::in);
int sav_error = errno;
- if (!ifs_.is_open()) {
+ if (!file_.is_open()) {
isc_throw(UserFileError, "cannot open file:" << fname_
<< " reason: " << strerror(sav_error));
}
@@ -50,14 +50,14 @@ UserFile::readNextUser() {
isc_throw (UserFileError, "cannot read, file is not open");
}
- if (ifs_.good()) {
- char buf[4096];
+ if (file_.good()) {
+ char buf[USER_ENTRY_MAX_LEN];
// Get the next line.
- ifs_.getline(buf, sizeof(buf));
+ file_.getline(buf, sizeof(buf));
// We got something, try to make a user out of it.
- if (ifs_.gcount() > 0) {
+ if (file_.gcount() > 0) {
return(makeUser(buf));
}
}
@@ -140,14 +140,14 @@ UserFile::makeUser(const std::string& user_string) {
bool
UserFile::isOpen() const {
- return (ifs_.is_open());
+ return (file_.is_open());
}
void
UserFile::close() {
try {
- if (ifs_.is_open()) {
- ifs_.close();
+ if (file_.is_open()) {
+ file_.close();
}
} catch (const std::exception& ex) {
// Highly unlikely to occur but let's at least spit out an error.
diff --git a/src/hooks/dhcp/user_chk/user_file.h b/src/hooks/dhcp/user_chk/user_file.h
index 4bfb038..b65862e 100644
--- a/src/hooks/dhcp/user_chk/user_file.h
+++ b/src/hooks/dhcp/user_chk/user_file.h
@@ -64,6 +64,11 @@ public:
/// @endcode
class UserFile : public UserDataSource {
public:
+ /// @brief Maximum length of a single user entry.
+ /// This value is somewhat arbitrary. 4K seems reasonably large. If it
+ /// goes beyond this, then a flat file is not likely the way to go.
+ static const size_t USER_ENTRY_MAX_LEN = 4096;
+
/// @brief Constructor
///
/// Create a UserFile for the given file name without opening the file.
@@ -120,7 +125,7 @@ private:
string fname_;
/// @brief Input file stream.
- std::ifstream ifs_;
+ std::ifstream file_;
};
More information about the bind10-changes
mailing list