[2340] Merge branch 'master' into trac2340

ChangeLog

@@ -1,3 +1,30 @@
+488.	[build]		jinmei
+	On configure, changed the search order for Python executable.
+	It first ties more specific file names such as "python3.2" before
+	more generic "python3".  This will prevent configure failure on
+	Mac OS X that installs Python3 via recent versions of Homebrew.
+	(Trac #2339, git 88db890d8d1c64de49be87f03c24a2021bcf63da)
+
+487.	[bug]		jinmei
+	The bind10 process now terminates a component (subprocess) by the
+	"config remove Boss/components" bindctl command even if the
+	process crashes immediately before the command is sent to bind10.
+	Previously this led to an inconsistent state between the
+	configuration and an internal component list of bind10, and bind10
+	kept trying to restart the component.  A known specific case of
+	this problem is that b10-ddns could keep failing (due to lack of
+	dependency modules) and the administrator couldn't stop the
+	restart via bindctl.
+	(Trac #2244, git 7565788d06f216ab254008ffdfae16678bcd00e5)
+
+486.	[bug]*		jinmei
+	All public header files for libb10-dns++ are now installed.
+	Template configure.ac and utility AC macros for external projects
+	using the library are provided under the "examples" directory.
+	The src/bin/host was moved as part of the examples (and not
+	installed with other BIND 10 programs any more).
+	(Trac #1870, git 4973e638d354d8b56dcadf71123ef23c15662021)
+
 485.	[bug]		jelte
 	Several bugs have been fixed in bindctl; tab-completion now works
 	within configuration lists, the problem where sometimes the

configure.ac

@@ -4,7 +4,7 @@
 AC_PREREQ([2.59])
 AC_INIT(bind10-devel, 20120817, bind10-dev@isc.org)
 AC_CONFIG_SRCDIR(README)
-AM_INIT_AUTOMAKE
+AM_INIT_AUTOMAKE([foreign])
 m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])])dnl be backward compatible
 AC_CONFIG_HEADERS([config.h])
 AC_CONFIG_MACRO_DIR([m4macros])
@@ -232,7 +232,7 @@ AM_CONDITIONAL(SET_ENV_LIBRARY_PATH, test $SET_ENV_LIBRARY_PATH = yes)
 AC_SUBST(SET_ENV_LIBRARY_PATH)
 AC_SUBST(ENV_LIBRARY_PATH)
 
-m4_define([_AM_PYTHON_INTERPRETER_LIST], [python python3 python3.1 python3.2])
+m4_define([_AM_PYTHON_INTERPRETER_LIST], [python python3.2 python3.1 python3])
 AC_ARG_WITH([pythonpath],
 AC_HELP_STRING([--with-pythonpath=PATH],
   [specify an absolute path to python executable when automatic version check (incorrectly) fails]),
@@ -1111,7 +1111,6 @@ AC_CONFIG_FILES([Makefile
                  src/bin/dbutil/Makefile
                  src/bin/dbutil/tests/Makefile
                  src/bin/dbutil/tests/testdata/Makefile
-                 src/bin/host/Makefile
                  src/bin/loadzone/Makefile
                  src/bin/loadzone/tests/correct/Makefile
                  src/bin/loadzone/tests/error/Makefile

examples/COPYING

@@ -0,0 +1,13 @@
+Copyright (C) 2012  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
+copyright notice and this permission notice appear in all copies.
+
+THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+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.

examples/INSTALL

@@ -0,0 +1,9 @@
+If using git (not the tarball), build the "configure" file:
+    autoreconf --install
+
+To then build from source:
+    ./configure
+    make
+
+You may have to specify the location of BIND 10 header files and
+library objects.  See configure options by ./configure --help.

examples/Makefile.am

@@ -0,0 +1,4 @@
+SUBDIRS = host
+
+# Make sure macros under m4 will be included
+ACLOCAL_AMFLAGS = -I m4

examples/README

@@ -0,0 +1,32 @@
+This is the top directory for sample programs that can be developed
+using public BIND 10 libraries outside of the BIND 10 project.  It's
+intended to be built with installed BIND 10 header files and library
+objects, so it's not a target of the main build tree, and does not
+refer to any other part of the BIND 10 source tree that contains
+this directory.
+
+On the top (sub) directory (where this README file is stored), we
+provide a sample configure.ac and Makefile.am files for GNU automake
+environments with helper autoconf macros to detect the available and
+location of BIND 10 header files and library objects.
+
+You can use the configure.ac and Makefile.am files with macros under
+the "m4" subdirectory as a template for your own project.  The key is
+to call the AX_ISC_BIND10 function (as the sample configure.ac does)
+from your configure.ac.  Then it will check the availability of
+necessary stuff and set some corresponding AC variables.  You can then
+use the resulting variables in your Makefile.in or Makefile.ac.
+
+If you use automake, don't forget adding the following line to the top
+level Makefile.am:
+
+ACLOCAL_AMFLAGS = -I m4
+
+This is necessary to incorporate the helper macro definitions.
+
+If you don't use automake but autoconf, make sure to add the following
+to the configure.ac file:
+
+sinclude(m4/ax_boost_include.m4)
+sinclude(m4/ax_isc_bind10.m4)
+(and same for other m4 files as they are added under m4/)

examples/configure.ac

@@ -0,0 +1,28 @@
+#                                               -*- Autoconf -*-
+# Process this file with autoconf to produce a configure script.
+
+AC_PREREQ([2.59])
+AC_INIT(bind10-examples, 20120817, bind10-dev@isc.org)
+AC_CONFIG_SRCDIR([README])
+AM_INIT_AUTOMAKE
+AC_CONFIG_HEADERS([config.h])
+
+# Checks for programs.
+AC_PROG_CXX
+AC_LANG([C++])
+
+# Checks for BIND 10 headers and libraries
+AX_ISC_BIND10
+
+# For the example host program, we require the BIND 10 DNS library
+if test "x$BIND10_DNS_LIB" = "x"; then
+   AC_MSG_ERROR([unable to find BIND 10 DNS library needed to build 'host'])
+fi
+
+# Checks for typedefs, structures, and compiler characteristics.
+AC_HEADER_STDBOOL
+
+AC_CONFIG_FILES([Makefile
+                 host/Makefile])
+
+AC_OUTPUT

examples/host/Makefile.am

@@ -0,0 +1,6 @@
+AM_CPPFLAGS = $(BOOST_CPPFLAGS) $(BIND10_CPPFLAGS)
+
+bin_PROGRAMS = b10-host
+b10_host_SOURCES = host.cc
+b10_host_LDFLAGS = ${BIND10_LDFLAGS}
+b10_host_LDADD = ${BIND10_DNS_LIB}

examples/m4/ax_boost_include.m4

@@ -0,0 +1,64 @@
+dnl @synopsis AX_BOOST_INCLUDE
+dnl
+dnl Test for the Boost C++ header files
+dnl
+dnl If no path to the installed boost header files is given via the
+dnl --with-boost-include option,  the macro searchs under
+dnl /usr/local /usr/pkg /opt /opt/local directories.
+dnl
+dnl This macro calls:
+dnl
+dnl   AC_SUBST(BOOST_CPPFLAGS)
+dnl
+
+AC_DEFUN([AX_BOOST_INCLUDE], [
+AC_LANG_SAVE
+AC_LANG([C++])
+
+#
+# Configure Boost header path
+#
+# If explicitly specified, use it.
+AC_ARG_WITH([boost-include],
+  AS_HELP_STRING([--with-boost-include=PATH],
+    [specify exact directory for Boost headers]),
+    [boost_include_path="$withval"])
+# If not specified, try some common paths.
+if test -z "$with_boost_include"; then
+	boostdirs="/usr/local /usr/pkg /opt /opt/local"
+	for d in $boostdirs
+	do
+		if test -f $d/include/boost/shared_ptr.hpp; then
+			boost_include_path=$d/include
+			break
+		fi
+	done
+fi
+CPPFLAGS_SAVES="$CPPFLAGS"
+if test "${boost_include_path}" ; then
+	BOOST_CPPFLAGS="-I${boost_include_path}"
+	CPPFLAGS="$CPPFLAGS $BOOST_CPPFLAGS"
+fi
+# Make sure some commonly used headers are available
+AC_CHECK_HEADERS([boost/shared_ptr.hpp boost/bind.hpp boost/function.hpp],,
+  AC_MSG_ERROR([Missing required Boost header files.]))
+
+# Detect whether Boost tries to use threads by default, and, if not,
+# make it sure explicitly.  In some systems the automatic detection
+# may depend on preceding header files, and if inconsistency happens
+# it could lead to a critical disruption.
+AC_MSG_CHECKING([whether Boost tries to use threads])
+AC_TRY_COMPILE([
+#include <boost/config.hpp>
+#ifdef BOOST_HAS_THREADS
+#error "boost will use threads"
+#endif],,
+[AC_MSG_RESULT(no)
+ CPPFLAGS_BOOST_THREADCONF="-DBOOST_DISABLE_THREADS=1"],
+[AC_MSG_RESULT(yes)])
+
+CPPFLAGS="$CPPFLAGS_SAVES $CPPFLAGS_BOOST_THREADCONF"
+AC_SUBST(BOOST_CPPFLAGS)
+
+AC_LANG_RESTORE
+])dnl AX_BOOST_INCLUDE

examples/m4/ax_isc_bind10.m4

@@ -0,0 +1,122 @@
+dnl @synopsis AX_BIND10
+dnl
+dnl @summary figure out how to build C++ programs using ISC BIND 10 libraries
+dnl
+dnl If no path to the installed BIND 10 header files or libraries is given
+dnl via the --with-bind10-include  or --with-bind10-lib option, the macro
+dnl searchs under /usr/local/{include, lib}, /usr/pkg/{include, lib},
+dnl /opt/{include, lib}, /opt/local/{include, lib} directories, respectively.
+dnl
+dnl This macro calls:
+dnl
+dnl   AC_SUBST(BIND10_CPPFLAGS)
+dnl   AC_SUBST(BIND10_LDFLAGS)
+dnl   AC_SUBST(BIND10_COMMON_LIB)
+dnl   AC_SUBST(BIND10_DNS_LIB)
+dnl
+dnl If this macro finds CPPFLAGS, LDFLAGS or COMMON_LIB unavailable, it treats
+dnl that as a fatal error.
+dnl Checks for other BIND 10 module libraries are option, as not all
+dnl applications need all libraries.  The main configure.ac can handle any
+dnl missing library as fatal by checking whether the corresponding
+dnl BIND10_xxx_LIB is defined.
+
+AC_DEFUN([AX_ISC_BIND10], [
+AC_REQUIRE([AX_BOOST_INCLUDE])
+AC_LANG_SAVE
+AC_LANG([C++])
+
+# Check for BIND10 common headers
+
+AC_ARG_WITH(bind10-include,
+  AS_HELP_STRING([--with-bind10-include=PATH],
+  [specify a path to BIND 10 header files]),
+    bind10_inc_path="$withval", bind10_inc_path="no")
+# If not specified, try some common paths.
+if test "$bind10_inc_path" = "no"; then
+   for d in /usr/local /usr/pkg /opt /opt/local
+   do
+	if test -f $d/include/util/buffer.h; then
+	   bind10_inc_path=$d
+	   break
+	fi
+   done
+fi
+CPPFLAGS_SAVES="$CPPFLAGS"
+if test "${bind10_inc_path}" != "no"; then
+   BIND10_CPPFLAGS="-I${bind10_inc_path}"
+   CPPFLAGS="$CPPFLAGS $BIND10_CPPFLAGS"
+fi
+AC_CHECK_HEADERS([util/buffer.h],,
+  AC_MSG_ERROR([Missing a commonly used BIND 10 header files]))
+CPPFLAGS="$CPPFLAGS_SAVES"
+AC_SUBST(BIND10_CPPFLAGS)
+
+# Check for BIND10 libraries
+CPPFLAGS_SAVED="$CPPFLAGS"
+CPPFLAGS="$CPPFLAGS $BIND10_CPPFLAGS"
+
+AC_ARG_WITH(bind10-lib,
+  AS_HELP_STRING([--with-bind10-lib=PATH],
+  [specify a path to BIND 10 library files]),
+    bind10_lib_path="$withval", bind10_lib_path="no")
+if test $bind10_lib_path != "no"; then
+   bind10_lib_dirs=$bind10_lib_path
+else
+   # If not specified, try some common paths.
+   bind10_lib_dirs="/usr/local/lib /usr/pkg/lib /opt/lib /opt/local/lib"
+fi
+
+# make sure we have buildable libraries
+AC_MSG_CHECKING([for BIND 10 common library])
+BIND10_COMMON_LIB="-lb10-util -lb10-exceptions"
+LDFLAGS="$LDFLAGS $BIND10_LDFLAGS"
+LIBS="$LIBS $BIND10_COMMON_LIB"
+for d in $bind10_lib_dirs
+do
+  LDFLAGS_SAVED="$LDFLAGS"
+  LDFLAGS="$LDFLAGS -L$d"
+  AC_TRY_LINK([
+#include <util/buffer.h>
+],[
+isc::util::OutputBuffer buffer(0);
+], [BIND10_LDFLAGS="-L${d}"])
+  if test "x$BIND10_LDFLAGS" != "x"; then
+     break
+  fi
+  LDFLAGS="$LDFLAGS_SAVED"
+done
+if test "x$BIND10_LDFLAGS" != "x"; then
+  AC_MSG_RESULT(yes)
+else
+  AC_MSG_RESULT(no)
+  AC_MSG_ERROR([unable to find required BIND 10 libraries])
+fi
+
+# restore LIBS once at this point
+LIBS="$LIBS_SAVES"
+
+AC_SUBST(BIND10_LDFLAGS)
+AC_SUBST(BIND10_COMMON_LIB)
+
+# Check per-module BIND 10 libraries
+
+# DNS library
+AC_MSG_CHECKING([for BIND 10 DNS library])
+LIBS="$LIBS $BIND10_COMMON_LIB -lb10-dns++"
+AC_TRY_LINK([
+#include <dns/rrtype.h>
+],[
+isc::dns::RRType rrtype(1);
+], [BIND10_DNS_LIB="-lb10-dns++"
+    AC_MSG_RESULT(yes)],
+   [AC_MSG_RESULT(no)])
+LIBS="$LIBS_SAVES"
+AC_SUBST(BIND10_DNS_LIB)
+
+# Restore other flags
+CPPFLAGS="$CPPFLAGS_SAVED"
+LDFLAGS="$LDFLAGS_SAVES"
+
+AC_LANG_RESTORE
+])dnl AX_ISC_BIND10

src/bin/Makefile.am

@@ -1,4 +1,4 @@
-SUBDIRS = bind10 bindctl cfgmgr ddns loadzone msgq host cmdctl auth xfrin \
+SUBDIRS = bind10 bindctl cfgmgr ddns loadzone msgq cmdctl auth xfrin \
 	xfrout usermgr zonemgr stats tests resolver sockcreator dhcp4 dhcp6 \
 	dbutil sysinfo
 

src/bin/auth/Makefile.am

@@ -55,7 +55,7 @@ b10_auth_SOURCES += auth_config.cc auth_config.h
 b10_auth_SOURCES += command.cc command.h
 b10_auth_SOURCES += common.h common.cc
 b10_auth_SOURCES += statistics.cc statistics.h
-b10_auth_SOURCES += datasrc_configurator.h
+b10_auth_SOURCES += datasrc_config.h datasrc_config.cc
 b10_auth_SOURCES += main.cc
 
 nodist_b10_auth_SOURCES = auth_messages.h auth_messages.cc

src/bin/auth/auth_srv.cc

@@ -69,6 +69,8 @@
 
 using namespace std;
 
+using boost::shared_ptr;
+
 using namespace isc;
 using namespace isc::cc;
 using namespace isc::datasrc;
@@ -264,23 +266,22 @@ public:
     AddressList listen_addresses_;
 
     /// The TSIG keyring
-    const boost::shared_ptr<TSIGKeyRing>* keyring_;
+    const shared_ptr<TSIGKeyRing>* keyring_;
 
-    /// The client list
-    std::map<RRClass, boost::shared_ptr<ConfigurableClientList> >
-        client_lists_;
+    /// The data source client list
+    AuthSrv::DataSrcClientListsPtr datasrc_client_lists_;
 
-    boost::shared_ptr<ConfigurableClientList> getClientList(const RRClass&
-                                                            rrclass)
+    shared_ptr<ConfigurableClientList> getDataSrcClientList(
+        const RRClass& rrclass)
     {
         // TODO: Debug-build only check
         if (!mutex_.locked()) {
             isc_throw(isc::Unexpected, "Not locked!");
         }
-        const std::map<RRClass, boost::shared_ptr<ConfigurableClientList> >::
-            const_iterator it(client_lists_.find(rrclass));
-        if (it == client_lists_.end()) {
-            return (boost::shared_ptr<ConfigurableClientList>());
+        const std::map<RRClass, shared_ptr<ConfigurableClientList> >::
+            const_iterator it(datasrc_client_lists_->find(rrclass));
+        if (it == datasrc_client_lists_->end()) {
+            return (shared_ptr<ConfigurableClientList>());
         } else {
             return (it->second);
         }
@@ -335,6 +336,8 @@ AuthSrvImpl::AuthSrvImpl(AbstractXfroutClient& xfrout_client,
     xfrin_session_(NULL),
     counters_(),
     keyring_(NULL),
+    datasrc_client_lists_(new std::map<RRClass,
+                          shared_ptr<ConfigurableClientList> >()),
     ddns_base_forwarder_(ddns_forwarder),
     ddns_forwarder_(NULL),
     xfrout_connected_(false),
@@ -645,13 +648,13 @@ AuthSrvImpl::processNormalQuery(const IOMessage& io_message, Message& message,
     }
     // Lock the client lists and keep them under the lock until the processing
     // and rendering is done (this is the same mutex as from
-    // AuthSrv::getClientListMutex()).
+    // AuthSrv::getDataSrcClientListMutex()).
     isc::util::thread::Mutex::Locker locker(mutex_);
 
     try {
         const ConstQuestionPtr question = *message.beginQuestion();
-        const boost::shared_ptr<datasrc::ClientList>
-            list(getClientList(question->getClass()));
+        const shared_ptr<datasrc::ClientList>
+            list(getDataSrcClientList(question->getClass()));
         if (list) {
             const RRType& qtype = question->getType();
             const Name& qname = question->getName();
@@ -911,7 +914,7 @@ AuthSrv::setDNSService(isc::asiodns::DNSServiceBase& dnss) {
 }
 
 void
-AuthSrv::setTSIGKeyRing(const boost::shared_ptr<TSIGKeyRing>* keyring) {
+AuthSrv::setTSIGKeyRing(const shared_ptr<TSIGKeyRing>* keyring) {
     impl_->keyring_ = keyring;
 }
 
@@ -930,43 +933,23 @@ AuthSrv::destroyDDNSForwarder() {
     }
 }
 
-void
-AuthSrv::setClientList(const RRClass& rrclass,
-                       const boost::shared_ptr<ConfigurableClientList>& list) {
+AuthSrv::DataSrcClientListsPtr
+AuthSrv::swapDataSrcClientLists(DataSrcClientListsPtr new_lists) {
     // TODO: Debug-build only check
     if (!impl_->mutex_.locked()) {
-        isc_throw(isc::Unexpected, "Not locked");
-    }
-
-    if (list) {
-        impl_->client_lists_[rrclass] = list;
-    } else {
-        impl_->client_lists_.erase(rrclass);
+        isc_throw(isc::Unexpected, "Not locked!");
     }
-}
-boost::shared_ptr<ConfigurableClientList>
-AuthSrv::getClientList(const RRClass& rrclass) {
-    return (impl_->getClientList(rrclass));
+    std::swap(new_lists, impl_->datasrc_client_lists_);
+    return (new_lists);
 }
 
-vector<RRClass>
-AuthSrv::getClientListClasses() const {
-    // TODO: Debug-build only check
-    if (!impl_->mutex_.locked()) {
-        isc_throw(isc::Unexpected, "Not locked");
-    }
-
-    vector<RRClass> result;
-    for (std::map<RRClass, boost::shared_ptr<ConfigurableClientList> >::
-         const_iterator it(impl_->client_lists_.begin());
-         it != impl_->client_lists_.end(); ++it) {
-        result.push_back(it->first);
-    }
-    return (result);
+shared_ptr<ConfigurableClientList>
+AuthSrv::getDataSrcClientList(const RRClass& rrclass) {
+    return (impl_->getDataSrcClientList(rrclass));
 }
 
 util::thread::Mutex&
-AuthSrv::getClientListMutex() const {
+AuthSrv::getDataSrcClientListMutex() const {
     return (impl_->mutex_);
 }
 

src/bin/auth/auth_srv.h

@@ -15,10 +15,11 @@
 #ifndef __AUTH_SRV_H
 #define __AUTH_SRV_H 1
 
-#include <string>
-
 #include <config/ccsession.h>
+
 #include <datasrc/factory.h>
+#include <datasrc/client_list.h>
+
 #include <dns/message.h>
 #include <dns/opcode.h>
 #include <util/buffer.h>
@@ -35,6 +36,11 @@
 #include <server_common/portconfig.h>
 #include <auth/statistics.h>
 
+#include <boost/shared_ptr.hpp>
+
+#include <map>
+#include <string>
+
 namespace isc {
 namespace util {
 namespace io {
@@ -296,31 +302,46 @@ public:
     /// If there was no forwarder yet, this method does nothing.
     void destroyDDNSForwarder();
 
-    /// \brief Sets the currently used list for data sources of given
-    ///     class.
+    /// \brief Shortcut typedef used for swapDataSrcClientLists().
+    typedef boost::shared_ptr<std::map<
+        isc::dns::RRClass, boost::shared_ptr<
+                               isc::datasrc::ConfigurableClientList> > >
+    DataSrcClientListsPtr;
+
+    /// \brief Swap the currently used set of data source client lists with
+    /// given one.
+    ///
+    /// The "set" of lists is actually given in the form of map from
+    /// RRClasses to shared pointers to isc::datasrc::ConfigurableClientList.
+    ///
+    /// This method returns the swapped set of lists, which was previously
+    /// used by the server.
+    ///
+    /// This method is intended to be used by a separate method to update
+    /// the data source configuration "at once".  The caller must hold
+    /// a lock for the mutex object returned by  \c getDataSrcClientListMutex()
+    /// before calling this method.
     ///
-    /// Replaces the internally used client list with a new one. Other
-    /// classes are not changed.
+    /// The ownership of the returned pointer is transferred to the caller.
+    /// The caller is generally expected to release the resources used in
+    /// the old lists.  Note that it could take longer time if some of the
+    /// data source clients contain a large size of in-memory data.
     ///
-    /// \param rrclass The class to modify.
-    /// \param list Shared pointer to the client list. If it is NULL,
-    ///     the list is removed instead.
-    void setClientList(const isc::dns::RRClass& rrclass, const
-                       boost::shared_ptr<isc::datasrc::ConfigurableClientList>&
-                       list);
+    /// The caller can pass a NULL pointer.  This effectively disables
+    /// any data source for the server.
+    ///
+    /// \param new_lists Shared pointer to a new set of data source client
+    /// lists.
+    /// \return The previous set of lists.  It can be NULL.
+    DataSrcClientListsPtr swapDataSrcClientLists(DataSrcClientListsPtr
+                                                 new_lists);
 
     /// \brief Returns the currently used client list for the class.
     ///
     /// \param rrclass The class for which to get the list.
     /// \return The list, or NULL if no list is set for the class.
     boost::shared_ptr<isc::datasrc::ConfigurableClientList>
-        getClientList(const isc::dns::RRClass& rrclass);
-
-    /// \brief Returns a list of classes that have a client list.
-    ///
-    /// \return List of classes for which a non-NULL client list
-    ///     has been set by setClientList.
-    std::vector<isc::dns::RRClass> getClientListClasses() const;
+        getDataSrcClientList(const isc::dns::RRClass& rrclass);
 
     /// \brief Return a mutex for the client lists.
     ///
@@ -331,9 +352,9 @@ public:
     /// is correct:
     /// \code
     /// {
-    ///  Mutex::Locker locker(auth->getClientListMutex());
+    ///  Mutex::Locker locker(auth->getDataSrcClientListMutex());
     ///  boost::shared_ptr<isc::datasrc::ConfigurableClientList>
-    ///    list(auth->getClientList(RRClass::IN()));
+    ///    list(auth->getDataSrcClientList(RRClass::IN()));
     ///  // Do some processing here
     /// }
     /// \endcode
@@ -342,8 +363,8 @@ public:
     /// \code
     /// boost::shared_ptr<isc::datasrc::ConfigurableClientList> list;
     /// {
-    ///     Mutex::Locker locker(auth->getClientListMutex());
-    ///     list = auth->getClientList(RRClass::IN()));
+    ///     Mutex::Locker locker(auth->getDataSrcClientListMutex());
+    ///     list = auth->getDataSrcClientList(RRClass::IN()));
     /// }
     /// // Do some processing here
     /// \endcode
@@ -352,7 +373,7 @@ public:
     ///    (lock) the mutex. It's because locking of the mutex is not really
     ///    a modification of the server object and it is needed to protect the
     ///    lists even on read-only operations.
-    isc::util::thread::Mutex& getClientListMutex() const;
+    isc::util::thread::Mutex& getDataSrcClientListMutex() const;
 
     /// \brief Sets the timeout for incoming TCP connections
     ///

src/bin/auth/benchmarks/Makefile.am

@@ -17,6 +17,7 @@ query_bench_SOURCES += ../auth_srv.h ../auth_srv.cc
 query_bench_SOURCES += ../auth_config.h ../auth_config.cc
 query_bench_SOURCES += ../statistics.h ../statistics.cc
 query_bench_SOURCES += ../auth_log.h ../auth_log.cc
+query_bench_SOURCES += ../datasrc_config.h ../datasrc_config.cc
 
 nodist_query_bench_SOURCES = ../auth_messages.h ../auth_messages.cc
 

src/bin/auth/benchmarks/query_bench.cc

@@ -18,6 +18,8 @@
 #include <bench/benchmark_util.h>
 
 #include <util/buffer.h>
+#include <util/threads/lock.h>
+
 #include <dns/message.h>
 #include <dns/name.h>
 #include <dns/question.h>
@@ -30,7 +32,7 @@
 
 #include <auth/auth_srv.h>
 #include <auth/auth_config.h>
-#include <auth/datasrc_configurator.h>
+#include <auth/datasrc_config.h>
 #include <auth/query.h>
 
 #include <asiodns/asiodns.h>
@@ -125,13 +127,15 @@ public:
                           OutputBuffer& buffer) :
         QueryBenchMark(queries, query_message, buffer)
     {
-        DataSourceConfigurator::testReconfigure(
-            server_.get(),
-            Element::fromJSON("{\"IN\":"
-                              "  [{\"type\": \"sqlite3\","
-                              "    \"params\": {"
-                              "      \"database_file\": \"" +
-                              string(datasrc_file) + "\"}}]}"));
+        isc::util::thread::Mutex::Locker locker(
+                server_->getDataSrcClientListMutex());
+        server_->swapDataSrcClientLists(
+            configureDataSource(
+                Element::fromJSON("{\"IN\":"
+                                  "  [{\"type\": \"sqlite3\","
+                                  "    \"params\": {"
+                                  "      \"database_file\": \"" +
+                                  string(datasrc_file) + "\"}}]}")));
     }
 };
 
@@ -139,19 +143,21 @@ class MemoryQueryBenchMark  : public QueryBenchMark {
 public:
     MemoryQueryBenchMark(const char* const zone_file,
                          const char* const zone_origin,
-                          const BenchQueries& queries,
-                          Message& query_message,
-                          OutputBuffer& buffer) :
+                         const BenchQueries& queries,
+                         Message& query_message,
+                         OutputBuffer& buffer) :
         QueryBenchMark(queries, query_message, buffer)
     {
-        DataSourceConfigurator::testReconfigure(
-            server_.get(),
-            Element::fromJSON("{\"IN\":"
-                              "  [{\"type\": \"MasterFiles\","
-                              "    \"cache-enable\": true, "
-                              "    \"params\": {\"" +
-                              string(zone_origin) + "\": \"" +
-                              string(zone_file) + "\"}}]}"));
+        isc::util::thread::Mutex::Locker locker(
+                server_->getDataSrcClientListMutex());
+        server_->swapDataSrcClientLists(
+            configureDataSource(
+                Element::fromJSON("{\"IN\":"
+                                  "  [{\"type\": \"MasterFiles\","
+                                  "    \"cache-enable\": true, "
+                                  "    \"params\": {\"" +
+                                  string(zone_origin) + "\": \"" +
+                                  string(zone_file) + "\"}}]}")));
     }
 };
 

src/bin/auth/command.cc

@@ -192,9 +192,10 @@ public:
 
         // We're going to work with the client lists. They may be used
         // from a different thread too, protect them.
-        isc::util::thread::Mutex::Locker locker(server.getClientListMutex());
+        isc::util::thread::Mutex::Locker locker(
+            server.getDataSrcClientListMutex());
         const boost::shared_ptr<isc::datasrc::ConfigurableClientList>
-            list(server.getClientList(zone_class));
+            list(server.getDataSrcClientList(zone_class));
 
         if (!list) {
             isc_throw(AuthCommandError, "There's no client list for "

src/bin/auth/datasrc_config.cc

@@ -0,0 +1,25 @@
+// Copyright (C) 2012  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
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// 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.
+
+#include <cc/data.h>
+#include "auth_srv.h"
+#include "datasrc_config.h"
+
+// This is a trivial specialization for the commonly used version.
+// Defined in .cc to avoid accidental creation of multiple copies.
+AuthSrv::DataSrcClientListsPtr
+configureDataSource(const isc::data::ConstElementPtr& config) {
+    return (configureDataSourceGeneric<
+            isc::datasrc::ConfigurableClientList>(config));
+}

src/bin/auth/datasrc_config.h

@@ -0,0 +1,83 @@
+// Copyright (C) 2012  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
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// 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.
+
+#ifndef DATASRC_CONFIG_H
+#define DATASRC_CONFIG_H
+
+#include "auth_srv.h"
+
+#include <cc/data.h>
+#include <datasrc/client_list.h>
+
+#include <boost/shared_ptr.hpp>
+
+#include <utility>
+#include <set>
+
+/// \brief Configure data source client lists
+///
+/// This will hook into the data_sources module configuration and it will
+/// return a new set (in the form of a shared pointer to map) of data source
+/// client lists corresponding to the configuration.
+///
+/// This function is templated. This is simply because of easier testing.
+/// You don't need to pay attention to it, use the configureDataSource
+/// specialization instead.
+///
+/// \note In future we may want to make the reconfiguration more efficient
+/// by only creating newly configured data and just moving the rest from
+/// the running configuration if they are used in the new configuration
+/// without any parameter change.  We could probably do it by passing
+/// the old lists in addition to the new config, but further details are
+/// still to be defined yet.  It will surely require changes in the
+/// data source library, too.  So, right now, we don't introduce the
+/// possibility in the function interface.  If and when we decide to introduce
+/// the optimization, we'll extend the interface.
+///
+/// \param config The configuration value to parse. It is in the form
+///     as an update from the config manager.
+/// \return A map from RR classes to configured lists.
+template<class List>
+boost::shared_ptr<std::map<isc::dns::RRClass,
+                           boost::shared_ptr<List> > > // = ListMap below
+configureDataSourceGeneric(const isc::data::ConstElementPtr& config) {
+    typedef boost::shared_ptr<List> ListPtr;
+    typedef std::map<std::string, isc::data::ConstElementPtr> Map;
+    typedef std::map<isc::dns::RRClass, ListPtr> ListMap;
+
+    boost::shared_ptr<ListMap> new_lists(new ListMap);
+
+    // Go through the configuration and create corresponding list.
+    const Map& map(config->mapValue());
+    for (Map::const_iterator it(map.begin()); it != map.end(); ++it) {
+        const isc::dns::RRClass rrclass(it->first);
+        ListPtr list(new List(rrclass));
+        list->configure(it->second, true);
+        new_lists->insert(std::pair<isc::dns::RRClass, ListPtr>(rrclass,
+                                                                list));
+    }
+
+    return (new_lists);
+}
+
+/// \brief Concrete version of configureDataSource() for the
+///     use with authoritative server implementation.
+AuthSrv::DataSrcClientListsPtr
+configureDataSource(const isc::data::ConstElementPtr& config);
+
+#endif  // DATASRC_CONFIG_H
+
+// Local Variables:
+// mode: c++
+// End:

src/bin/auth/datasrc_configurator.h

@@ -1,226 +0,0 @@
-// Copyright (C) 2012  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
-// copyright notice and this permission notice appear in all copies.
-//
-// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
-// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
-// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
-// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
-// 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.
-
-#ifndef DATASRC_CONFIGURATOR_H
-#define DATASRC_CONFIGURATOR_H
-
-#include "auth_srv.h"
-
-#include <datasrc/client_list.h>
-#include <config/ccsession.h>
-#include <cc/data.h>
-#include <util/threads/lock.h>
-
-#include <set>
-
-/// \brief A class to configure the authoritative server's data source lists
-///
-/// This will hook into the data_sources module configuration and it will
-/// keep the local copy of data source clients in the list in the authoritative
-/// server.
-///
-/// The class is slightly unusual. Due to some technical limitations, the hook
-/// needs to be static method. Therefore it is not possible to create instances
-/// of the class.
-///
-/// Also, the class is a template. This is simply because of easier testing.
-/// You don't need to pay attention to it, use the DataSourceConfigurator
-/// type alias instead.
-template<class Server, class List>
-class DataSourceConfiguratorGeneric {
-private:
-    /// \brief Disallow creation of instances
-    DataSourceConfiguratorGeneric();
-    /// \brief Internal method to hook into the ModuleCCSession
-    ///
-    /// It simply calls reconfigure.
-    static void reconfigureInternal(const std::string&,
-                                    isc::data::ConstElementPtr config,
-                                    const isc::config::ConfigData&)
-    {
-        if (config->contains("classes")) {
-            reconfigure(config->get("classes"));
-        }
-    }
-    static Server* server_;
-    static isc::config::ModuleCCSession* session_;
-    typedef boost::shared_ptr<List> ListPtr;
-public:
-    /// \brief Initializes the class.
-    ///
-    /// This configures which session and server should be used.
-    /// It hooks to the session now and downloads the configuration.
-    /// It is synchronous (it may block for some time).
-    ///
-    /// Note that you need to call cleanup before the server or
-    /// session dies, otherwise it might access them after they
-    /// are destroyed.
-    ///
-    /// \param session The session to hook into and to access the configuration
-    ///     through.
-    /// \param server It is the server to configure.
-    /// \throw isc::InvalidOperation if this is called when already initialized.
-    /// \throw isc::InvalidParameter if any of the parameters is NULL
-    /// \throw isc::config::ModuleCCError if the remote configuration is not
-    ///     available for some reason.
-    static void init(isc::config::ModuleCCSession* session,
-                     Server* server)
-    {
-        if (session == NULL) {
-            isc_throw(isc::InvalidParameter, "The session must not be NULL");
-        }
-        if (server == NULL) {
-            isc_throw(isc::InvalidParameter, "The server must not be NULL");
-        }
-        if (server_ != NULL) {
-            isc_throw(isc::InvalidOperation,
-                      "The configurator is already initialized");
-        }
-        server_ = server;
-        session_ = session;
-        session->addRemoteConfig("data_sources", reconfigureInternal, false);
-    }
-    /// \brief Deinitializes the class.
-    ///
-    /// This detaches from the session and removes the server from internal
-    /// storage. The current configuration in the server is preserved.
-    ///
-    /// This can be called even if it is not initialized currently. You
-    /// can initialize it again after this.
-    static void cleanup() {
-        if (session_ != NULL) {
-            session_->removeRemoteConfig("data_sources");
-        }
-        session_ = NULL;
-        server_ = NULL;
-    }
-    /// \brief Reads new configuration and replaces the old one.
-    ///
-    /// It instructs the server to replace the lists with new ones as needed.
-    /// You don't need to call it directly (but you could, though the benefit
-    /// is unknown and it would be questionable at least). It is called
-    /// automatically on normal updates.
-    ///
-    /// \param config The configuration value to parse. It is in the form
-    ///     as an update from the config manager.
-    /// \throw InvalidOperation if it is called when not initialized.
-    static void reconfigure(const isc::data::ConstElementPtr& config) {
-        if (server_ == NULL) {
-            isc_throw(isc::InvalidOperation,
-                      "Can't reconfigure while not initialized by init()");
-        }
-        // Lock the client lists, we're going to manipulate them.
-        isc::util::thread::Mutex::Locker locker(server_->getClientListMutex());
-        typedef std::map<std::string, isc::data::ConstElementPtr> Map;
-        typedef std::pair<isc::dns::RRClass, ListPtr> RollbackPair;
-        typedef std::pair<isc::dns::RRClass, isc::data::ConstElementPtr>
-            RollbackConfiguration;
-        // Some structures to be able to perform a rollback
-        std::vector<RollbackPair> rollback_sets;
-        std::vector<RollbackConfiguration> rollback_configurations;
-        try {
-            // Get the configuration and current state.
-            const Map& map(config->mapValue());
-            const std::vector<isc::dns::RRClass>
-                activeVector(server_->getClientListClasses());
-            std::set<isc::dns::RRClass> active(activeVector.begin(),
-                                               activeVector.end());
-            // Go through the configuration and change everything.
-            for (Map::const_iterator it(map.begin()); it != map.end(); ++it) {
-                isc::dns::RRClass rrclass(it->first);
-                active.erase(rrclass);
-                ListPtr list(server_->getClientList(rrclass));
-                bool need_set(false);
-                if (list) {
-                    rollback_configurations.
-                        push_back(RollbackConfiguration(rrclass,
-                            list->getConfiguration()));
-                } else {
-                    list.reset(new List(rrclass));
-                    need_set = true;
-                    rollback_sets.push_back(RollbackPair(rrclass, ListPtr()));
-                }
-                list->configure(it->second, true);
-                if (need_set) {
-                    server_->setClientList(rrclass, list);
-                }
-            }
-            // Remove the ones that are not in the configuration.
-            for (std::set<isc::dns::RRClass>::iterator it(active.begin());
-                 it != active.end(); ++it) {
-                // There seems to be no way the setClientList could throw.
-                // But this is just to make sure in case it did to restore
-                // the original.
-                rollback_sets.push_back(
-                    RollbackPair(*it, server_->getClientList(*it)));
-                server_->setClientList(*it, ListPtr());
-            }
-        } catch (...) {
-            // Perform a rollback of the changes. The old configuration should
-            // work.
-            for (typename std::vector<RollbackPair>::const_iterator
-                 it(rollback_sets.begin()); it != rollback_sets.end(); ++it) {
-                server_->setClientList(it->first, it->second);
-            }
-            for (typename std::vector<RollbackConfiguration>::const_iterator
-                 it(rollback_configurations.begin());
-                 it != rollback_configurations.end(); ++it) {
-                server_->getClientList(it->first)->configure(it->second, true);
-            }
-            throw;
-        }
-    }
-    /// \brief Version of reconfigure for easier testing.
-    ///
-    /// This method can be used to reconfigure a server without first
-    /// initializing the configurator. This does not need a session.
-    /// Otherwise, it acts the same as reconfigure.
-    ///
-    /// This is not meant for production code. Do not use there.
-    ///
-    /// \param server The server to configure.
-    /// \param config The config to use.
-    /// \throw isc::InvalidOperation if the configurator is initialized.
-    /// \throw anything that reconfigure does.
-    static void testReconfigure(Server* server,
-                                const isc::data::ConstElementPtr& config)
-    {
-        if (server_ != NULL) {
-            isc_throw(isc::InvalidOperation, "Currently initialized.");
-        }
-        try {
-            server_ = server;
-            reconfigure(config);
-            server_ = NULL;
-        } catch (...) {
-            server_ = NULL;
-            throw;
-        }
-    }
-};
-
-template<class Server, class List>
-isc::config::ModuleCCSession*
-DataSourceConfiguratorGeneric<Server, List>::session_(NULL);
-
-template<class Server, class List>
-Server* DataSourceConfiguratorGeneric<Server, List>::server_(NULL);
-
-/// \brief Concrete version of DataSourceConfiguratorGeneric for the
-///     use in authoritative server.
-typedef DataSourceConfiguratorGeneric<AuthSrv,
-        isc::datasrc::ConfigurableClientList>
-    DataSourceConfigurator;
-
-#endif

src/bin/auth/main.cc

@@ -14,21 +14,11 @@
 
 #include <config.h>
 
-#include <sys/types.h>
-#include <sys/socket.h>
-#include <sys/select.h>
-#include <netdb.h>
-#include <netinet/in.h>
-#include <stdlib.h>
-#include <errno.h>
-
-#include <cassert>
-#include <iostream>
-
 #include <exceptions/exceptions.h>
 
 #include <util/buffer.h>
 #include <util/io/socketsession.h>
+#include <util/threads/lock.h>
 
 #include <dns/message.h>
 #include <dns/messagerenderer.h>
@@ -45,13 +35,26 @@
 #include <auth/command.h>
 #include <auth/auth_srv.h>
 #include <auth/auth_log.h>
-#include <auth/datasrc_configurator.h>
+#include <auth/datasrc_config.h>
 #include <asiodns/asiodns.h>
 #include <asiolink/asiolink.h>
 #include <log/logger_support.h>
 #include <server_common/keyring.h>
 #include <server_common/socket_request.h>
 
+#include <boost/bind.hpp>
+
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <sys/select.h>
+#include <netdb.h>
+#include <netinet/in.h>
+#include <stdlib.h>
+#include <errno.h>
+
+#include <cassert>
+#include <iostream>
+
 using namespace std;
 using namespace isc::asiodns;
 using namespace isc::asiolink;
@@ -70,7 +73,7 @@ namespace {
 /* need global var for config/command handlers.
  * todo: turn this around, and put handlers in the authserver
  * class itself? */
-AuthSrv *auth_server;
+AuthSrv* auth_server;
 
 ConstElementPtr
 my_config_handler(ConstElementPtr new_config) {
@@ -84,6 +87,42 @@ my_command_handler(const string& command, ConstElementPtr args) {
 }
 
 void
+datasrcConfigHandler(AuthSrv* server, bool* first_time,
+                     ModuleCCSession* config_session, const std::string&,
+                     isc::data::ConstElementPtr config,
+                     const isc::config::ConfigData&)
+{
+    assert(server != NULL);
+    if (config->contains("classes")) {
+        AuthSrv::DataSrcClientListsPtr lists;
+
+        if (*first_time) {
+            // HACK: The default is not passed to the handler in the first
+            // callback. This one will get the default (or, current value).
+            // Further updates will work the usual way.
+            assert(config_session != NULL);
+            *first_time = false;
+            lists = configureDataSource(
+                config_session->getRemoteConfigValue("data_sources",
+                                                     "classes"));
+        } else {
+            lists = configureDataSource(config->get("classes"));
+        }
+
+        // Replace the server's lists.  The returned lists will be stored
+        // in a local variable 'lists', and will be destroyed outside of
+        // the temporary block for the lock scope.  That way we can minimize
+        // the range of the critical section.
+        {
+            isc::util::thread::Mutex::Locker locker(
+                server->getDataSrcClientListMutex());
+            lists = server->swapDataSrcClientLists(lists);
+        }
+        // The previous lists are destroyed here.
+    }
+}
+
+void
 usage() {
     cerr << "Usage:  b10-auth [-v]"
          << endl;
@@ -191,13 +230,15 @@ main(int argc, char* argv[]) {
         isc::server_common::initKeyring(*config_session);
         auth_server->setTSIGKeyRing(&isc::server_common::keyring);
 
-        // Start the data source configuration
-        DataSourceConfigurator::init(config_session, auth_server);
-        // HACK: The default is not passed to the handler. This one will
-        // get the default (or, current value). Further updates will work
-        // the usual way.
-        DataSourceConfigurator::reconfigure(
-            config_session->getRemoteConfigValue("data_sources", "classes"));
+        // Start the data source configuration.  We pass first_time and
+        // config_session for the hack described in datasrcConfigHandler.
+        bool first_time = true;
+        config_session->addRemoteConfig("data_sources",
+                                        boost::bind(datasrcConfigHandler,
+                                                    auth_server, &first_time,
+                                                    config_session,
+                                                    _1, _2, _3),
+                                        false);
 
         // Now start asynchronous read.
         config_session->start();
@@ -221,7 +262,10 @@ main(int argc, char* argv[]) {
         xfrin_session->disconnect();
     }
 
-    DataSourceConfigurator::cleanup();
+    // If we haven't registered callback for data sources, this will be just
+    // no-op.
+    config_session->removeRemoteConfig("data_sources");
+
     delete xfrin_session;
     delete config_session;
     delete cc_session;

src/bin/auth/tests/Makefile.am

@@ -42,6 +42,7 @@ run_unittests_SOURCES += ../auth_config.h ../auth_config.cc
 run_unittests_SOURCES += ../command.h ../command.cc
 run_unittests_SOURCES += ../common.h ../common.cc
 run_unittests_SOURCES += ../statistics.h ../statistics.cc
+run_unittests_SOURCES += ../datasrc_config.h ../datasrc_config.cc
 run_unittests_SOURCES += datasrc_util.h datasrc_util.cc
 run_unittests_SOURCES += auth_srv_unittest.cc
 run_unittests_SOURCES += config_unittest.cc
@@ -50,7 +51,7 @@ run_unittests_SOURCES += command_unittest.cc
 run_unittests_SOURCES += common_unittest.cc
 run_unittests_SOURCES += query_unittest.cc
 run_unittests_SOURCES += statistics_unittest.cc
-run_unittests_SOURCES += datasrc_configurator_unittest.cc
+run_unittests_SOURCES += datasrc_config_unittest.cc
 run_unittests_SOURCES += run_unittests.cc
 
 nodist_run_unittests_SOURCES = ../auth_messages.h ../auth_messages.cc

src/bin/auth/tests/auth_srv_unittest.cc

@@ -36,7 +36,7 @@
 #include <auth/command.h>
 #include <auth/common.h>
 #include <auth/statistics.h>
-#include <auth/datasrc_configurator.h>
+#include <auth/datasrc_config.h>
 
 #include <util/unittests/mock_socketsession.h>
 #include <util/threads/lock.h>
@@ -63,6 +63,7 @@
 using namespace std;
 using namespace isc::cc;
 using namespace isc::dns;
+using namespace isc::datasrc;
 using namespace isc::util;
 using namespace isc::util::io::internal;
 using namespace isc::util::unittests;
@@ -90,6 +91,9 @@ const char* const STATIC_DSRC_FILE = DSRC_DIR "/static.zone";
 // a signed example zone.
 const char* const CONFIG_INMEMORY_EXAMPLE = TEST_DATA_DIR "/rfc5155-example.zone.signed";
 
+// shortcut commonly used in tests
+typedef boost::shared_ptr<ConfigurableClientList> ListPtr;
+
 class AuthSrvTest : public SrvTestBase {
 protected:
     AuthSrvTest() :
@@ -722,17 +726,25 @@ TEST_F(AuthSrvTest, notifyWithSessionMessageError) {
 }
 
 void
-updateDatabase(AuthSrv* server, const char* params) {
+installDataSrcClientLists(AuthSrv& server,
+                          AuthSrv::DataSrcClientListsPtr lists)
+{
+    thread::Mutex::Locker locker(server.getDataSrcClientListMutex());
+    server.swapDataSrcClientLists(lists);
+}
+
+void
+updateDatabase(AuthSrv& server, const char* params) {
     const ConstElementPtr config(Element::fromJSON("{"
         "\"IN\": [{"
         "    \"type\": \"sqlite3\","
         "    \"params\": " + string(params) +
         "}]}"));
-    DataSourceConfigurator::testReconfigure(server, config);
+    installDataSrcClientLists(server, configureDataSource(config));
 }
 
 void
-updateInMemory(AuthSrv* server, const char* origin, const char* filename) {
+updateInMemory(AuthSrv& server, const char* origin, const char* filename) {
     const ConstElementPtr config(Element::fromJSON("{"
         "\"IN\": [{"
         "   \"type\": \"MasterFiles\","
@@ -745,17 +757,17 @@ updateInMemory(AuthSrv* server, const char* origin, const char* filename) {
         "   \"type\": \"static\","
         "   \"params\": \"" + string(STATIC_DSRC_FILE) + "\""
         "}]}"));
-    DataSourceConfigurator::testReconfigure(server, config);
+    installDataSrcClientLists(server, configureDataSource(config));
 }
 
 void
-updateBuiltin(AuthSrv* server) {
+updateBuiltin(AuthSrv& server) {
     const ConstElementPtr config(Element::fromJSON("{"
         "\"CH\": [{"
         "   \"type\": \"static\","
         "   \"params\": \"" + string(STATIC_DSRC_FILE) + "\""
         "}]}"));
-    DataSourceConfigurator::testReconfigure(server, config);
+    installDataSrcClientLists(server, configureDataSource(config));
 }
 
 // Try giving the server a TSIG signed request and see it can anwer signed as
@@ -766,7 +778,7 @@ TEST_F(AuthSrvTest, DISABLED_TSIGSigned) { // Needs builtin
 TEST_F(AuthSrvTest, TSIGSigned) {
 #endif
     // Prepare key, the client message, etc
-    updateBuiltin(&server);
+    updateBuiltin(server);
     const TSIGKey key("key:c2VjcmV0Cg==:hmac-sha1");
     TSIGContext context(key);
     UnitTestUtil::createRequestMessage(request_message, opcode, default_qid,
@@ -814,7 +826,7 @@ TEST_F(AuthSrvTest, DISABLED_builtInQueryViaDNSServer) {
 #else
 TEST_F(AuthSrvTest, builtInQueryViaDNSServer) {
 #endif
-    updateBuiltin(&server);
+    updateBuiltin(server);
     UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(),
                                        default_qid, Name("VERSION.BIND."),
                                        RRClass::CH(), RRType::TXT());
@@ -846,7 +858,7 @@ TEST_F(AuthSrvTest, DISABLED_builtInQuery) {
 #else
 TEST_F(AuthSrvTest, builtInQuery) {
 #endif
-    updateBuiltin(&server);
+    updateBuiltin(server);
     UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(),
                                        default_qid, Name("VERSION.BIND."),
                                        RRClass::CH(), RRType::TXT());
@@ -867,7 +879,7 @@ TEST_F(AuthSrvTest, DISABLED_iqueryViaDNSServer) { // Needs builtin
 #else
 TEST_F(AuthSrvTest, iqueryViaDNSServer) { // Needs builtin
 #endif
-    updateBuiltin(&server);
+    updateBuiltin(server);
     createDataFromFile("iquery_fromWire.wire");
     (*server.getDNSLookupProvider())(*io_message, parse_message,
                                      response_message,
@@ -889,7 +901,7 @@ TEST_F(AuthSrvTest, DISABLED_updateConfig) {
 #else
 TEST_F(AuthSrvTest, updateConfig) {
 #endif
-    updateDatabase(&server, CONFIG_TESTDB);
+    updateDatabase(server, CONFIG_TESTDB);
 
     // query for existent data in the installed data source.  The resulting
     // response should have the AA flag on, and have an RR in each answer
@@ -907,7 +919,7 @@ TEST_F(AuthSrvTest, DISABLED_datasourceFail) {
 #else
 TEST_F(AuthSrvTest, datasourceFail) {
 #endif
-    updateDatabase(&server, CONFIG_TESTDB);
+    updateDatabase(server, CONFIG_TESTDB);
 
     // This query will hit a corrupted entry of the data source (the zoneload
     // tool and the data source itself naively accept it).  This will result
@@ -927,10 +939,10 @@ TEST_F(AuthSrvTest, DISABLED_updateConfigFail) {
 TEST_F(AuthSrvTest, updateConfigFail) {
 #endif
     // First, load a valid data source.
-    updateDatabase(&server, CONFIG_TESTDB);
+    updateDatabase(server, CONFIG_TESTDB);
 
     // Next, try to update it with a non-existent one.  This should fail.
-    EXPECT_THROW(updateDatabase(&server, BADCONFIG_TESTDB),
+    EXPECT_THROW(updateDatabase(server, BADCONFIG_TESTDB),
                  isc::datasrc::DataSourceError);
 
     // The original data source should still exist.
@@ -953,7 +965,7 @@ TEST_F(AuthSrvTest, updateWithInMemoryClient) {
         "   \"params\": {},"
         "   \"cache-enable\": true"
         "}]}"));
-    DataSourceConfigurator::testReconfigure(&server, config);
+    installDataSrcClientLists(server, configureDataSource(config));
     // after successful configuration, we should have one (with empty zoneset).
 
     // The memory data source is empty, should return REFUSED rcode.
@@ -974,7 +986,7 @@ TEST_F(AuthSrvTest, queryWithInMemoryClientNoDNSSEC) {
     // query handler class, and confirm it returns no error and a non empty
     // answer section.  Detailed examination on the response content
     // for various types of queries are tested in the query tests.
-    updateInMemory(&server, "example.", CONFIG_INMEMORY_EXAMPLE);
+    updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE);
 
     createDataFromFile("nsec3query_nodnssec_fromWire.wire");
     server.processMessage(*io_message, *parse_message, *response_obuffer,
@@ -993,7 +1005,7 @@ TEST_F(AuthSrvTest, queryWithInMemoryClientDNSSEC) {
     // Similar to the previous test, but the query has the DO bit on.
     // The response should contain RRSIGs, and should have more RRs than
     // the previous case.
-    updateInMemory(&server, "example.", CONFIG_INMEMORY_EXAMPLE);
+    updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE);
 
     createDataFromFile("nsec3query_fromWire.wire");
     server.processMessage(*io_message, *parse_message, *response_obuffer,
@@ -1013,7 +1025,7 @@ TEST_F(AuthSrvTest,
     )
 {
     // Set up the in-memory
-    updateInMemory(&server, "example.", CONFIG_INMEMORY_EXAMPLE);
+    updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE);
 
     // This shouldn't affect the result of class CH query
     UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(),
@@ -1425,13 +1437,16 @@ TEST_F(AuthSrvTest,
     )
 {
     // Set real inmem client to proxy
-    updateInMemory(&server, "example.", CONFIG_INMEMORY_EXAMPLE);
+    updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE);
     {
-        isc::util::thread::Mutex::Locker locker(server.getClientListMutex());
+        isc::util::thread::Mutex::Locker locker(
+            server.getDataSrcClientListMutex());
         boost::shared_ptr<isc::datasrc::ConfigurableClientList>
-            list(new FakeList(server.getClientList(RRClass::IN()), THROW_NEVER,
-                              false));
-        server.setClientList(RRClass::IN(), list);
+            list(new FakeList(server.getDataSrcClientList(RRClass::IN()),
+                              THROW_NEVER, false));
+        AuthSrv::DataSrcClientListsPtr lists(new std::map<RRClass, ListPtr>);
+        lists->insert(pair<RRClass, ListPtr>(RRClass::IN(), list));
+        server.swapDataSrcClientLists(lists);
     }
 
     createDataFromFile("nsec3query_nodnssec_fromWire.wire");
@@ -1450,16 +1465,19 @@ TEST_F(AuthSrvTest,
 // If non null rrset is given, it will be passed to the proxy so it can
 // return some faked response.
 void
-setupThrow(AuthSrv* server, ThrowWhen throw_when, bool isc_exception,
+setupThrow(AuthSrv& server, ThrowWhen throw_when, bool isc_exception,
            ConstRRsetPtr rrset = ConstRRsetPtr())
 {
     updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE);
 
-    isc::util::thread::Mutex::Locker locker(server->getClientListMutex());
+    isc::util::thread::Mutex::Locker locker(
+        server.getDataSrcClientListMutex());
     boost::shared_ptr<isc::datasrc::ConfigurableClientList>
-        list(new FakeList(server->getClientList(RRClass::IN()), throw_when,
-                          isc_exception, rrset));
-    server->setClientList(RRClass::IN(), list);
+        list(new FakeList(server.getDataSrcClientList(RRClass::IN()),
+                          throw_when, isc_exception, rrset));
+    AuthSrv::DataSrcClientListsPtr lists(new std::map<RRClass, ListPtr>);
+    lists->insert(pair<RRClass, ListPtr>(RRClass::IN(), list));
+    server.swapDataSrcClientLists(lists);
 }
 
 TEST_F(AuthSrvTest,
@@ -1482,11 +1500,11 @@ TEST_F(AuthSrvTest,
                                              RRClass::IN(), RRType::TXT());
     for (ThrowWhen* when(throws); *when != THROW_NEVER; ++when) {
         createRequestPacket(request_message, IPPROTO_UDP);
-        setupThrow(&server, *when, true);
+        setupThrow(server, *when, true);
         processAndCheckSERVFAIL();
         // To be sure, check same for non-isc-exceptions
         createRequestPacket(request_message, IPPROTO_UDP);
-        setupThrow(&server, *when, false);
+        setupThrow(server, *when, false);
         processAndCheckSERVFAIL();
     }
 }
@@ -1502,7 +1520,7 @@ TEST_F(AuthSrvTest,
     )
 {
     createDataFromFile("nsec3query_nodnssec_fromWire.wire");
-    setupThrow(&server, THROW_AT_GET_CLASS, true);
+    setupThrow(server, THROW_AT_GET_CLASS, true);
 
     // getClass is not called so it should just answer
     server.processMessage(*io_message, *parse_message, *response_obuffer,
@@ -1526,7 +1544,7 @@ TEST_F(AuthSrvTest,
     ConstRRsetPtr empty_rrset(new RRset(Name("foo.example"),
                                         RRClass::IN(), RRType::TXT(),
                                         RRTTL(0)));
-    setupThrow(&server, THROW_NEVER, true, empty_rrset);
+    setupThrow(server, THROW_NEVER, true, empty_rrset);
 
     // Repeat the query processing two times.  Due to the faked RRset,
     // toWire() should throw, and it should result in SERVFAIL.
@@ -1771,46 +1789,51 @@ TEST_F(AuthSrvTest, clientList) {
     // We need to lock the mutex to make the (get|set)ClientList happy.
     // There's a debug-build only check in them to make sure everything
     // locks them and we call them directly here.
-    isc::util::thread::Mutex::Locker locker(server.getClientListMutex());
+    isc::util::thread::Mutex::Locker locker(
+        server.getDataSrcClientListMutex());
+
+    AuthSrv::DataSrcClientListsPtr lists; // initially empty
+
     // The lists don't exist. Therefore, the list of RRClasses is empty.
-    // We also have no IN list.
-    EXPECT_TRUE(server.getClientListClasses().empty());
-    EXPECT_EQ(boost::shared_ptr<const isc::datasrc::ClientList>(),
-              server.getClientList(RRClass::IN()));
+    EXPECT_TRUE(server.swapDataSrcClientLists(lists)->empty());
+
     // Put something in.
-    const boost::shared_ptr<isc::datasrc::ConfigurableClientList>
-        list(new isc::datasrc::ConfigurableClientList(RRClass::IN()));
-    const boost::shared_ptr<isc::datasrc::ConfigurableClientList>
-        list2(new isc::datasrc::ConfigurableClientList(RRClass::CH()));
-    server.setClientList(RRClass::IN(), list);
-    server.setClientList(RRClass::CH(), list2);
-    // There are two things in the list and they are IN and CH
-    vector<RRClass> classes(server.getClientListClasses());
-    ASSERT_EQ(2, classes.size());
-    EXPECT_EQ(RRClass::IN(), classes[0]);
-    EXPECT_EQ(RRClass::CH(), classes[1]);
+    const ListPtr list(new ConfigurableClientList(RRClass::IN()));
+    const ListPtr list2(new ConfigurableClientList(RRClass::CH()));
+
+    lists.reset(new std::map<RRClass, ListPtr>);
+    lists->insert(pair<RRClass, ListPtr>(RRClass::IN(), list));
+    lists->insert(pair<RRClass, ListPtr>(RRClass::CH(), list2));
+    server.swapDataSrcClientLists(lists);
+
     // And the lists can be retrieved.
-    EXPECT_EQ(list, server.getClientList(RRClass::IN()));
-    EXPECT_EQ(list2, server.getClientList(RRClass::CH()));
-    // Remove one of them
-    server.setClientList(RRClass::CH(),
-        boost::shared_ptr<isc::datasrc::ConfigurableClientList>());
-    // This really got deleted, including the class.
-    classes = server.getClientListClasses();
-    ASSERT_EQ(1, classes.size());
-    EXPECT_EQ(RRClass::IN(), classes[0]);
-    EXPECT_EQ(list, server.getClientList(RRClass::IN()));
+    EXPECT_EQ(list, server.getDataSrcClientList(RRClass::IN()));
+    EXPECT_EQ(list2, server.getDataSrcClientList(RRClass::CH()));
+
+    // Replace the lists with new lists containing only one list.
+    lists.reset(new std::map<RRClass, ListPtr>);
+    lists->insert(pair<RRClass, ListPtr>(RRClass::IN(), list));
+    lists = server.swapDataSrcClientLists(lists);
+
+    // Old one had two lists.  That confirms our swap for IN and CH classes
+    // (i.e., no other entries were there).
+    EXPECT_EQ(2, lists->size());
+
+    // The CH list really got deleted.
+    EXPECT_EQ(list, server.getDataSrcClientList(RRClass::IN()));
+    EXPECT_FALSE(server.getDataSrcClientList(RRClass::CH()));
 }
 
 // We just test the mutex can be locked (exactly once).
 TEST_F(AuthSrvTest, mutex) {
-    isc::util::thread::Mutex::Locker l1(server.getClientListMutex());
+    isc::util::thread::Mutex::Locker l1(server.getDataSrcClientListMutex());
     // TODO: Once we have non-debug build, this one will not work, since
     // we currently use the fact that we can't lock twice from the same
     // thread. In the non-debug mode, this would deadlock.
     // Skip then.
     EXPECT_THROW({
-        isc::util::thread::Mutex::Locker l2(server.getClientListMutex());
+        isc::util::thread::Mutex::Locker l2(
+            server.getDataSrcClientListMutex());
     }, isc::InvalidOperation);
 }
 

src/bin/auth/tests/command_unittest.cc

@@ -16,10 +16,12 @@
 
 #include "datasrc_util.h"
 
+#include <util/threads/lock.h>
+
 #include <auth/auth_srv.h>
 #include <auth/auth_config.h>
 #include <auth/command.h>
-#include <auth/datasrc_configurator.h>
+#include <auth/datasrc_config.h>
 
 #include <dns/name.h>
 #include <dns/rrclass.h>
@@ -174,22 +176,32 @@ TEST_F(AuthCommandTest, shutdownIncorrectPID) {
 // zones, and checks the zones are correctly loaded.
 void
 zoneChecks(AuthSrv& server) {
-    isc::util::thread::Mutex::Locker locker(server.getClientListMutex());
-    EXPECT_EQ(ZoneFinder::SUCCESS, server.getClientList(RRClass::IN())->
+    isc::util::thread::Mutex::Locker locker(
+        server.getDataSrcClientListMutex());
+    EXPECT_EQ(ZoneFinder::SUCCESS, server.getDataSrcClientList(RRClass::IN())->
               find(Name("ns.test1.example")).finder_->
               find(Name("ns.test1.example"), RRType::A())->code);
-    EXPECT_EQ(ZoneFinder::NXRRSET, server.getClientList(RRClass::IN())->
+    EXPECT_EQ(ZoneFinder::NXRRSET, server.getDataSrcClientList(RRClass::IN())->
               find(Name("ns.test1.example")).finder_->
               find(Name("ns.test1.example"), RRType::AAAA())->code);
-    EXPECT_EQ(ZoneFinder::SUCCESS, server.getClientList(RRClass::IN())->
+    EXPECT_EQ(ZoneFinder::SUCCESS, server.getDataSrcClientList(RRClass::IN())->
               find(Name("ns.test2.example")).finder_->
               find(Name("ns.test2.example"), RRType::A())->code);
-    EXPECT_EQ(ZoneFinder::NXRRSET, server.getClientList(RRClass::IN())->
+    EXPECT_EQ(ZoneFinder::NXRRSET, server.getDataSrcClientList(RRClass::IN())->
               find(Name("ns.test2.example")).finder_->
               find(Name("ns.test2.example"), RRType::AAAA())->code);
 }
 
 void
+installDataSrcClientLists(AuthSrv& server,
+                          AuthSrv::DataSrcClientListsPtr lists)
+{
+    isc::util::thread::Mutex::Locker locker(
+        server.getDataSrcClientListMutex());
+    server.swapDataSrcClientLists(lists);
+}
+
+void
 configureZones(AuthSrv& server) {
     ASSERT_EQ(0, system(INSTALL_PROG " -c " TEST_DATA_DIR "/test1.zone.in "
                         TEST_DATA_BUILDDIR "/test1.zone.copied"));
@@ -208,27 +220,29 @@ configureZones(AuthSrv& server) {
         "   \"cache-enable\": true"
         "}]}"));
 
-    DataSourceConfigurator::testReconfigure(&server, config);
+    installDataSrcClientLists(server, configureDataSource(config));
 
     zoneChecks(server);
 }
 
 void
 newZoneChecks(AuthSrv& server) {
-    isc::util::thread::Mutex::Locker locker(server.getClientListMutex());
-    EXPECT_EQ(ZoneFinder::SUCCESS, server.getClientList(RRClass::IN())->
+    isc::util::thread::Mutex::Locker locker(
+        server.getDataSrcClientListMutex());
+    EXPECT_EQ(ZoneFinder::SUCCESS, server.getDataSrcClientList(RRClass::IN())->
               find(Name("ns.test1.example")).finder_->
               find(Name("ns.test1.example"), RRType::A())->code);
     // now test1.example should have ns/AAAA
-    EXPECT_EQ(ZoneFinder::SUCCESS, server.getClientList(RRClass::IN())->
+    EXPECT_EQ(ZoneFinder::SUCCESS, server.getDataSrcClientList(RRClass::IN())->
               find(Name("ns.test1.example")).finder_->
               find(Name("ns.test1.example"), RRType::AAAA())->code);
 
     // test2.example shouldn't change
-    EXPECT_EQ(ZoneFinder::SUCCESS, server.getClientList(RRClass::IN())->
+    EXPECT_EQ(ZoneFinder::SUCCESS, server.getDataSrcClientList(RRClass::IN())->
               find(Name("ns.test2.example")).finder_->
               find(Name("ns.test2.example"), RRType::A())->code);
-    EXPECT_EQ(ZoneFinder::NXRRSET, server.getClientList(RRClass::IN())->
+    EXPECT_EQ(ZoneFinder::NXRRSET,
+              server.getDataSrcClientList(RRClass::IN())->
               find(Name("ns.test2.example")).finder_->
               find(Name("ns.test2.example"), RRType::AAAA())->code);
 }
@@ -271,12 +285,14 @@ TEST_F(AuthCommandTest,
         "    \"cache-enable\": true,"
         "    \"cache-zones\": [\"example.org\"]"
         "}]}"));
-    DataSourceConfigurator::testReconfigure(&server_, config);
+    installDataSrcClientLists(server_, configureDataSource(config));
 
     {
-        isc::util::thread::Mutex::Locker locker(server_.getClientListMutex());
+        isc::util::thread::Mutex::Locker locker(
+            server_.getDataSrcClientListMutex());
         // Check that the A record at www.example.org does not exist
-        EXPECT_EQ(ZoneFinder::NXDOMAIN, server_.getClientList(RRClass::IN())->
+        EXPECT_EQ(ZoneFinder::NXDOMAIN,
+                  server_.getDataSrcClientList(RRClass::IN())->
                   find(Name("example.org")).finder_->
                   find(Name("www.example.org"), RRType::A())->code);
 
@@ -296,7 +312,8 @@ TEST_F(AuthCommandTest,
         sql_updater->addRRset(*rrset);
         sql_updater->commit();
 
-        EXPECT_EQ(ZoneFinder::NXDOMAIN, server_.getClientList(RRClass::IN())->
+        EXPECT_EQ(ZoneFinder::NXDOMAIN,
+                  server_.getDataSrcClientList(RRClass::IN())->
                   find(Name("example.org")).finder_->
                   find(Name("www.example.org"), RRType::A())->code);
     }
@@ -308,9 +325,11 @@ TEST_F(AuthCommandTest,
     checkAnswer(0, "Successful load");
 
     {
-        isc::util::thread::Mutex::Locker locker(server_.getClientListMutex());
+        isc::util::thread::Mutex::Locker locker(
+            server_.getDataSrcClientListMutex());
         // And now it should be present too.
-        EXPECT_EQ(ZoneFinder::SUCCESS, server_.getClientList(RRClass::IN())->
+        EXPECT_EQ(ZoneFinder::SUCCESS,
+                  server_.getDataSrcClientList(RRClass::IN())->
                   find(Name("example.org")).finder_->
                   find(Name("www.example.org"), RRType::A())->code);
     }
@@ -321,9 +340,11 @@ TEST_F(AuthCommandTest,
     checkAnswer(1, "example.com");
 
     {
-        isc::util::thread::Mutex::Locker locker(server_.getClientListMutex());
+        isc::util::thread::Mutex::Locker locker(
+            server_.getDataSrcClientListMutex());
         // The previous zone is not hurt in any way
-        EXPECT_EQ(ZoneFinder::SUCCESS, server_.getClientList(RRClass::IN())->
+        EXPECT_EQ(ZoneFinder::SUCCESS,
+                  server_.getDataSrcClientList(RRClass::IN())->
                   find(Name("example.org")).finder_->
                   find(Name("example.org"), RRType::SOA())->code);
     }
@@ -335,16 +356,18 @@ TEST_F(AuthCommandTest,
         "    \"cache-enable\": true,"
         "    \"cache-zones\": [\"example.com\"]"
         "}]}"));
-    EXPECT_THROW(DataSourceConfigurator::testReconfigure(&server_, config2),
+    EXPECT_THROW(configureDataSource(config2),
                  ConfigurableClientList::ConfigurationError);
 
     result_ = execAuthServerCommand(server_, "loadzone",
         Element::fromJSON("{\"origin\": \"example.com\"}"));
     checkAnswer(1, "Unreadable");
 
-    isc::util::thread::Mutex::Locker locker(server_.getClientListMutex());
+    isc::util::thread::Mutex::Locker locker(
+        server_.getDataSrcClientListMutex());
     // The previous zone is not hurt in any way
-    EXPECT_EQ(ZoneFinder::SUCCESS, server_.getClientList(RRClass::IN())->
+    EXPECT_EQ(ZoneFinder::SUCCESS,
+              server_.getDataSrcClientList(RRClass::IN())->
               find(Name("example.org")).finder_->
               find(Name("example.org"), RRType::SOA())->code);
 }

src/bin/auth/tests/datasrc_configurator_unittest.cc

@@ -12,16 +12,19 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <auth/datasrc_configurator.h>
+#include <auth/datasrc_config.h>
 
 #include <config/tests/fake_session.h>
 #include <config/ccsession.h>
 #include <util/threads/lock.h>
 
 #include <gtest/gtest.h>
-#include <memory>
+
+#include <boost/bind.hpp>
 #include <boost/shared_ptr.hpp>
 
+#include <memory>
+
 using namespace isc;
 using namespace isc::cc;
 using namespace isc::config;
@@ -32,7 +35,7 @@ using namespace boost;
 
 namespace {
 
-class DatasrcConfiguratorTest;
+class DatasrcConfigTest;
 
 class FakeList {
 public:
@@ -57,36 +60,50 @@ private:
 
 typedef shared_ptr<FakeList> ListPtr;
 
-// We use the test fixture as both parameters, this makes it possible
-// to easily fake all needed methods and look that they were called.
-typedef DataSourceConfiguratorGeneric<DatasrcConfiguratorTest,
-        FakeList> Configurator;
+// Forward declaration.  We need precise definition of DatasrcConfigTest
+// to complete this function.
+void
+testConfigureDataSource(DatasrcConfigTest& test,
+                        const isc::data::ConstElementPtr& config);
 
-class DatasrcConfiguratorTest : public ::testing::Test {
+void
+datasrcConfigHandler(DatasrcConfigTest* fake_server, const std::string&,
+                     isc::data::ConstElementPtr config,
+                     const isc::config::ConfigData&)
+{
+    if (config->contains("classes")) {
+        testConfigureDataSource(*fake_server, config->get("classes"));
+    }
+}
+
+class DatasrcConfigTest : public ::testing::Test {
 public:
     // These pretend to be the server
-    ListPtr getClientList(const RRClass& rrclass) {
-        log_ += "get " + rrclass.toText() + "\n";
-        return (lists_[rrclass]);
-    }
-    void setClientList(const RRClass& rrclass, const ListPtr& list) {
-        log_ += "set " + rrclass.toText() + " " +
-            (list ? list->getConf() : "") + "\n";
-        lists_[rrclass] = list;
+    isc::util::thread::Mutex& getDataSrcClientListMutex() const {
+        return (mutex_);
     }
-    vector<RRClass> getClientListClasses() const {
-        vector<RRClass> result;
-        for (std::map<RRClass, ListPtr>::const_iterator it(lists_.begin());
-             it != lists_.end(); ++it) {
-            result.push_back(it->first);
+    void swapDataSrcClientLists(shared_ptr<std::map<dns::RRClass, ListPtr> >
+                                new_lists)
+    {
+        lists_.clear();         // first empty it
+
+        // Record the operation and results.  Note that map elements are
+        // sorted by RRClass, so the ordering should be predictable.
+        for (std::map<dns::RRClass, ListPtr>::const_iterator it =
+                 new_lists->begin();
+             it != new_lists->end();
+             ++it)
+        {
+            const RRClass rrclass = it->first;
+            ListPtr list = it->second;
+            log_ += "set " + rrclass.toText() + " " +
+                (list ? list->getConf() : "") + "\n";
+            lists_[rrclass] = list;
         }
-        return (result);
-    }
-    isc::util::thread::Mutex& getClientListMutex() const {
-        return (mutex_);
     }
+
 protected:
-    DatasrcConfiguratorTest() :
+    DatasrcConfigTest() :
         session(ElementPtr(new ListElement), ElementPtr(new ListElement),
                 ElementPtr(new ListElement)),
         specfile(string(TEST_OWN_DATA_DIR) + "/spec.spec")
@@ -99,25 +116,24 @@ protected:
                                        false));
     }
     void TearDown() {
-        // Make sure no matter what we did, it is cleaned up.
-        Configurator::cleanup();
+        // Make sure no matter what we did, it is cleaned up.  Also check
+        // we really have subscribed to the configuration, and after removing
+        // it we actually cancel it.
+        EXPECT_TRUE(session.haveSubscription("data_sources", "*"));
+        mccs->removeRemoteConfig("data_sources");
+        EXPECT_FALSE(session.haveSubscription("data_sources", "*"));
     }
-    void init(const ElementPtr& config = ElementPtr()) {
+    void SetUp() {
         session.getMessages()->
             add(createAnswer(0,
                              moduleSpecFromFile(string(PLUGIN_DATA_PATH) +
                                                 "/datasrc.spec").
                              getFullSpec()));
-        if (config) {
-            session.getMessages()->add(createAnswer(0, config));
-        } else {
-            session.getMessages()->
-                add(createAnswer(0, ElementPtr(new MapElement)));
-        }
-        Configurator::init(mccs.get(), this);
-    }
-    void SetUp() {
-        init();
+        session.getMessages()->add(createAnswer(0,
+                                                ElementPtr(new MapElement)));
+        mccs->addRemoteConfig("data_sources",
+                              boost::bind(datasrcConfigHandler,
+                                          this, _1, _2, _3), false);
     }
     ElementPtr buildConfig(const string& config) const {
         const ElementPtr internal(Element::fromJSON(config));
@@ -126,14 +142,13 @@ protected:
         return (external);
     }
     void initializeINList() {
-        const ElementPtr
+        const ConstElementPtr
             config(buildConfig("{\"IN\": [{\"type\": \"xxx\"}]}"));
-        session.addMessage(createCommand("config_update", config), "data_sources",
-                           "*");
+        session.addMessage(createCommand("config_update", config),
+                           "data_sources", "*");
         mccs->checkCommand();
-        // Check it called the correct things (check that there's no IN yet and
-        // set a new one.
-        EXPECT_EQ("get IN\nset IN xxx\n", log_);
+        // Check that the passed config is stored.
+        EXPECT_EQ("set IN xxx\n", log_);
         EXPECT_EQ(1, lists_.size());
     }
     FakeSession session;
@@ -144,35 +159,27 @@ protected:
     mutable isc::util::thread::Mutex mutex_;
 };
 
-// Check the initialization (and cleanup)
-TEST_F(DatasrcConfiguratorTest, initialization) {
-    // It can't be initialized again
-    EXPECT_THROW(init(), InvalidOperation);
-    EXPECT_TRUE(session.haveSubscription("data_sources", "*"));
-    // Deinitialize to make the tests reasonable
-    Configurator::cleanup();
-    EXPECT_FALSE(session.haveSubscription("data_sources", "*"));
-    // We can't reconfigure now (not even manually)
-    EXPECT_THROW(Configurator::reconfigure(ElementPtr(new MapElement())),
-                 InvalidOperation);
-    // If one of them is NULL, it does not work
-    EXPECT_THROW(Configurator::init(NULL, this), InvalidParameter);
-    EXPECT_FALSE(session.haveSubscription("data_sources", "*"));
-    EXPECT_THROW(Configurator::init(mccs.get(), NULL), InvalidParameter);
-    EXPECT_FALSE(session.haveSubscription("data_sources", "*"));
-    // But we can initialize it again now
-    EXPECT_NO_THROW(init());
-    EXPECT_TRUE(session.haveSubscription("data_sources", "*"));
+void
+testConfigureDataSource(DatasrcConfigTest& test,
+                        const isc::data::ConstElementPtr& config)
+{
+    // We use customized (faked lists) for the List type.  This makes it
+    // possible to easily look that they were called.
+    shared_ptr<std::map<dns::RRClass, ListPtr> > lists =
+        configureDataSourceGeneric<FakeList>(config);
+    test.swapDataSrcClientLists(lists);
 }
 
 // Push there a configuration with a single list.
-TEST_F(DatasrcConfiguratorTest, createList) {
+TEST_F(DatasrcConfigTest, createList) {
     initializeINList();
 }
 
-TEST_F(DatasrcConfiguratorTest, modifyList) {
-    // First, initialize the list
+TEST_F(DatasrcConfigTest, modifyList) {
+    // First, initialize the list, and confirm the current config
     initializeINList();
+    EXPECT_EQ("xxx", lists_[RRClass::IN()]->getConf());
+
     // And now change the configuration of the list
     const ElementPtr
         config(buildConfig("{\"IN\": [{\"type\": \"yyy\"}]}"));
@@ -180,15 +187,13 @@ TEST_F(DatasrcConfiguratorTest, modifyList) {
                        "*");
     log_ = "";
     mccs->checkCommand();
-    // This one does not set
-    EXPECT_EQ("get IN\n", log_);
-    // But this should contain the yyy configuration
+    // Now the new one should be installed.
     EXPECT_EQ("yyy", lists_[RRClass::IN()]->getConf());
     EXPECT_EQ(1, lists_.size());
 }
 
 // Check we can have multiple lists at once
-TEST_F(DatasrcConfiguratorTest, multiple) {
+TEST_F(DatasrcConfigTest, multiple) {
     const ElementPtr
         config(buildConfig("{\"IN\": [{\"type\": \"yyy\"}], "
                                  "\"CH\": [{\"type\": \"xxx\"}]}"));
@@ -196,7 +201,7 @@ TEST_F(DatasrcConfiguratorTest, multiple) {
                        "*");
     mccs->checkCommand();
     // We have set commands for both classes.
-    EXPECT_EQ("get CH\nset CH xxx\nget IN\nset IN yyy\n", log_);
+    EXPECT_EQ("set IN yyy\nset CH xxx\n", log_);
     // We should have both there
     EXPECT_EQ("yyy", lists_[RRClass::IN()]->getConf());
     EXPECT_EQ("xxx", lists_[RRClass::CH()]->getConf());
@@ -208,7 +213,7 @@ TEST_F(DatasrcConfiguratorTest, multiple) {
 //
 // It's almost like above, but we initialize first with single-list
 // config.
-TEST_F(DatasrcConfiguratorTest, updateAdd) {
+TEST_F(DatasrcConfigTest, updateAdd) {
     initializeINList();
     const ElementPtr
         config(buildConfig("{\"IN\": [{\"type\": \"yyy\"}], "
@@ -217,16 +222,14 @@ TEST_F(DatasrcConfiguratorTest, updateAdd) {
                        "*");
     log_ = "";
     mccs->checkCommand();
-    // The CH is set, IN not
-    EXPECT_EQ("get CH\nset CH xxx\nget IN\n", log_);
-    // But this should contain the yyy configuration
+    EXPECT_EQ("set IN yyy\nset CH xxx\n", log_);
     EXPECT_EQ("xxx", lists_[RRClass::CH()]->getConf());
     EXPECT_EQ("yyy", lists_[RRClass::IN()]->getConf());
     EXPECT_EQ(2, lists_.size());
 }
 
 // We delete a class list in this test.
-TEST_F(DatasrcConfiguratorTest, updateDelete) {
+TEST_F(DatasrcConfigTest, updateDelete) {
     initializeINList();
     const ElementPtr
         config(buildConfig("{}"));
@@ -234,18 +237,18 @@ TEST_F(DatasrcConfiguratorTest, updateDelete) {
                        "*");
     log_ = "";
     mccs->checkCommand();
-    EXPECT_EQ("get IN\nset IN \n", log_);
-    EXPECT_FALSE(lists_[RRClass::IN()]);
-    // In real auth server, the NULL one would be removed. However, we just
-    // store it, so the IN bucket is still in there. This checks there's nothing
-    // else.
-    EXPECT_EQ(1, lists_.size());
+
+    // No operation takes place in the configuration, and the old one is
+    // just dropped
+    EXPECT_EQ("", log_);
+    EXPECT_TRUE(lists_.empty());
 }
 
-// Check that we can rollback an addition if something else fails
-TEST_F(DatasrcConfiguratorTest, rollbackAddition) {
+// Check that broken new configuration doesn't break the running configuration.
+TEST_F(DatasrcConfigTest, brokenConfigForAdd) {
     initializeINList();
-    // The configuration is wrong. However, the CH one will get done first.
+    // The configuration is wrong. However, the CH one will be handled
+    // without an error first.
     const ElementPtr
         config(buildConfig("{\"IN\": [{\"type\": 13}], "
                            "\"CH\": [{\"type\": \"xxx\"}]}"));
@@ -261,41 +264,40 @@ TEST_F(DatasrcConfiguratorTest, rollbackAddition) {
     EXPECT_FALSE(lists_[RRClass::CH()]);
 }
 
-// Check that we can rollback a deletion if something else fails
-TEST_F(DatasrcConfiguratorTest, rollbackDeletion) {
+// Similar to the previous one, but the broken config would delete part of
+// the running config.
+TEST_F(DatasrcConfigTest, brokenConfigForDelete) {
     initializeINList();
     // Put the CH there
     const ElementPtr
         config1(Element::fromJSON("{\"IN\": [{\"type\": \"yyy\"}], "
                                   "\"CH\": [{\"type\": \"xxx\"}]}"));
-    Configurator::reconfigure(config1);
+    testConfigureDataSource(*this, config1);
     const ElementPtr
         config2(Element::fromJSON("{\"IN\": [{\"type\": 13}]}"));
-    // This would delete CH. However, the IN one fails.
-    // As the deletions happen after the additions/settings
-    // and there's no known way to cause an exception during the
-    // deletions, it is not a true rollback, but the result should
-    // be the same.
-    EXPECT_THROW(Configurator::reconfigure(config2), TypeError);
+    // This would delete CH. However, the new config is broken, so it won't
+    // actually apply.
+    EXPECT_THROW(testConfigureDataSource(*this, config2), TypeError);
     EXPECT_EQ("yyy", lists_[RRClass::IN()]->getConf());
     EXPECT_EQ("xxx", lists_[RRClass::CH()]->getConf());
 }
 
-// Check that we can roll back configuration change if something
-// fails later on.
-TEST_F(DatasrcConfiguratorTest, rollbackConfiguration) {
+// Similar to the previous cases, but the broken config would modify the
+// running config of one of the classes.
+TEST_F(DatasrcConfigTest, brokenConfigForModify) {
     initializeINList();
     // Put the CH there
     const ElementPtr
         config1(Element::fromJSON("{\"IN\": [{\"type\": \"yyy\"}], "
                                   "\"CH\": [{\"type\": \"xxx\"}]}"));
-    Configurator::reconfigure(config1);
-    // Now, the CH happens first. But nevertheless, it should be
-    // restored to the previoeus version.
+    testConfigureDataSource(*this, config1);
+    // Now, the CH change will be handled first without an error, then
+    // the change to the IN class will fail, and the none of the changes
+    // will apply.
     const ElementPtr
         config2(Element::fromJSON("{\"IN\": [{\"type\": 13}], "
                                   "\"CH\": [{\"type\": \"yyy\"}]}"));
-    EXPECT_THROW(Configurator::reconfigure(config2), TypeError);
+    EXPECT_THROW(testConfigureDataSource(*this, config2), TypeError);
     EXPECT_EQ("yyy", lists_[RRClass::IN()]->getConf());
     EXPECT_EQ("xxx", lists_[RRClass::CH()]->getConf());
 }

src/bin/bind10/bind10_messages.mes

@@ -141,6 +141,16 @@ it now. The new configuration is printed.
 % BIND10_RECEIVED_SIGNAL received signal %1
 The boss module received the given signal.
 
+% BIND10_RESTART_COMPONENT_SKIPPED Skipped restarting a component %1
+The boss module tried to restart a component after it failed (crashed)
+unexpectedly, but the boss then found that the component had been removed
+from its local configuration of components to run.  This is an unusal
+situation but can happen if the administrator removes the component from
+the configuration after the component's crash and before the restart time.
+The boss module simply skipped restarting that module, and the whole system
+went back to the expected state (except that the crash itself is likely
+to be a bug).
+
 % BIND10_RESURRECTED_PROCESS resurrected %1 (PID %2)
 The given process has been restarted successfully, and is now running
 with the given process id.

src/bin/bind10/bind10_src.py.in

@@ -739,7 +739,7 @@ class BoB:
                 component = self.components.pop(pid)
                 logger.info(BIND10_PROCESS_ENDED, component.name(), pid,
                             exit_status)
-                if component.running() and self.runnable:
+                if component.is_running() and self.runnable:
                     # Tell it it failed. But only if it matters (we are
                     # not shutting down and the component considers itself
                     # to be running.
@@ -771,7 +771,12 @@ class BoB:
         next_restart_time = None
         now = time.time()
         for component in self.components_to_restart:
-            if not component.restart(now):
+            # If the component was removed from the configurator between since
+            # scheduled to restart, just ignore it.  The object will just be
+            # dropped here.
+            if not self._component_configurator.has_component(component):
+                logger.info(BIND10_RESTART_COMPONENT_SKIPPED, component.name())
+            elif not component.restart(now):
                 still_dead.append(component)
                 if next_restart_time is None or\
                    next_restart_time > component.get_restart_time():

src/bin/bind10/tests/bind10_test.py.in

@@ -929,7 +929,14 @@ class MockComponent:
         self.name = lambda: name
         self.pid = lambda: pid
         self.address = lambda: address
+        self.restarted = False
 
+    def get_restart_time(self):
+        return 0                # arbitrary dummy value
+
+    def restart(self, now):
+        self.restarted = True
+        return True
 
 class TestBossCmd(unittest.TestCase):
     def test_ping(self):
@@ -1266,6 +1273,34 @@ class TestBossComponents(unittest.TestCase):
         bob.start_all_components()
         self.__check_extended(self.__param)
 
+    def __setup_restart(self, bob, component):
+        '''Common procedure for restarting a component used below.'''
+        bob.components_to_restart = { component }
+        component.restarted = False
+        bob.restart_processes()
+
+    def test_restart_processes(self):
+        '''Check some behavior on restarting processes.'''
+        bob = MockBob()
+        bob.runnable = True
+        component = MockComponent('test', 53)
+
+        # A component to be restarted will actually be restarted iff it's
+        # in the configurator's configuration.
+        # We bruteforce the configurator internal below; ugly, but the easiest
+        # way for the test.
+        bob._component_configurator._components['test'] = (None, component)
+        self.__setup_restart(bob, component)
+        self.assertTrue(component.restarted)
+        self.assertFalse(component in bob.components_to_restart)
+
+        # Remove the component from the configuration.  It won't be restarted
+        # even if scheduled, nor will remain in the to-be-restarted list.
+        del bob._component_configurator._components['test']
+        self.__setup_restart(bob, component)
+        self.assertFalse(component.restarted)
+        self.assertFalse(component in bob.components_to_restart)
+
 class SocketSrvTest(unittest.TestCase):
     """
     This tests some methods of boss related to the unix domain sockets used

src/bin/dbutil/dbutil.py.in

@@ -193,10 +193,17 @@ UPGRADES = [
             "ALTER TABLE schema_version " +
                 "ADD COLUMN minor INTEGER NOT NULL DEFAULT 0"
         ]
+     },
+
+    {'from': (2, 0), 'to': (2, 1),
+     'statements': [
+            "CREATE INDEX nsec3_byhash_and_rdtype ON nsec3 " +
+                "(hash, rdtype)"
+        ]
     }
 
 # To extend this, leave the above statements in place and add another
-# dictionary to the list.  The "from" version should be (2, 0), the "to"
+# dictionary to the list.  The "from" version should be (2, 1), the "to"
 # version whatever the version the update is to, and the SQL statements are
 # the statements required to perform the upgrade.  This way, the upgrade
 # program will be able to upgrade both a V1.0 and a V2.0 database.

src/bin/dbutil/tests/Makefile.am

@@ -2,6 +2,8 @@ SUBDIRS = . testdata
 
 # Tests of the update script.
 
+noinst_SCRIPTS = dbutil_test.sh
+
 check-local:
 	B10_LOCKFILE_DIR_FROM_BUILD=$(abs_top_builddir) \
 	$(SHELL) $(abs_builddir)/dbutil_test.sh

src/bin/dbutil/tests/dbutil_test.sh.in

@@ -165,7 +165,7 @@ upgrade_ok_test() {
     if [ $? -eq 0 ]
     then
         # Compare schema with the reference
-        get_schema $testdata/v2_0.sqlite3
+        get_schema $testdata/v2_1.sqlite3
         expected_schema=$db_schema
         get_schema $tempfile
         actual_schema=$db_schema
@@ -177,7 +177,7 @@ upgrade_ok_test() {
         fi
 
         # Check the version is set correctly
-        check_version $tempfile "V2.0"
+        check_version $tempfile "V2.1"
 
         # Check that a backup was made
         check_backup $1 $2
@@ -449,7 +449,7 @@ copy_file $testdata/old_v1.sqlite3 $tempfile
 Yes
 .
 passzero $?
-check_version $tempfile "V2.0"
+check_version $tempfile "V2.1"
 rm -f $tempfile $backupfile
 
 echo "13.4 Interactive prompt - no"

src/bin/dbutil/tests/testdata/Makefile.am

@@ -10,3 +10,4 @@ EXTRA_DIST += old_v1.sqlite3
 EXTRA_DIST += README
 EXTRA_DIST += too_many_version.sqlite3
 EXTRA_DIST += v2_0.sqlite3
+EXTRA_DIST += v2_1.sqlite3

src/bin/dhcp4/tests/Makefile.am

@@ -16,6 +16,7 @@ check-local:
 	for pytest in $(PYTESTS) ; do \
 	echo Running test: $$pytest ; \
 	PYTHONPATH=$(COMMON_PYTHON_PATH):$(abs_top_srcdir)/src/bin:$(abs_top_builddir)/src/bin/bind10:$(abs_top_builddir)/src/lib/util/io/.libs \
+	B10_LOCKFILE_DIR_FROM_BUILD=$(abs_top_builddir) \
 	$(LIBRARY_PATH_PLACEHOLDER) \
 		$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
 	done

src/bin/dhcp6/tests/Makefile.am

@@ -15,6 +15,7 @@ check-local:
 	for pytest in $(PYTESTS) ; do \
 	echo Running test: $$pytest ; \
 	$(LIBRARY_PATH_PLACEHOLDER) \
+	B10_LOCKFILE_DIR_FROM_BUILD=$(abs_top_builddir) \
 	PYTHONPATH=$(COMMON_PYTHON_PATH):$(abs_top_srcdir)/src/bin:$(abs_top_builddir)/src/bin/bind10:$(abs_top_builddir)/src/lib/util/io/.libs \
 		$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
 	done

src/bin/host/Makefile.am

@@ -1,37 +0,0 @@
-AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib
-AM_CPPFLAGS += -I$(top_srcdir)/src/lib/dns -I$(top_builddir)/src/lib/dns
-AM_CPPFLAGS += $(BOOST_INCLUDES)
-
-AM_CXXFLAGS = $(B10_CXXFLAGS)
-
-if USE_STATIC_LINK
-AM_LDFLAGS = -static
-endif
-
-CLEANFILES = *.gcno *.gcda
-
-bin_PROGRAMS = b10-host
-b10_host_SOURCES = host.cc
-b10_host_LDADD = $(top_builddir)/src/lib/dns/libb10-dns++.la
-b10_host_LDADD += $(top_builddir)/src/lib/util/libb10-util.la
-b10_host_LDADD += $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
-
-man_MANS = b10-host.1
-DISTCLEANFILES = $(man_MANS)
-EXTRA_DIST = $(man_MANS) b10-host.xml
-
-.PHONY: man
-if GENERATE_DOCS
-
-man: b10-host.1
-
-b10-host.1: b10-host.xml
-	@XSLTPROC@ --novalid --xinclude --nonet -o $@ http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl $(srcdir)/b10-host.xml
-
-else
-
-$(man_MANS):
-	@echo Man generation disabled.  Creating dummy $@.  Configure with --enable-generate-docs to enable it.
-	@echo Man generation disabled.  Remove this file, configure with --enable-generate-docs, and rebuild BIND 10 > $@
-
-endif

src/cppcheck-suppress.lst

@@ -11,3 +11,18 @@ missingInclude
 //
 //    // cppcheck-suppress duplicateExpression
 //    EXPECT_FALSE(small_name < small_name);
+
+// With cppcheck 1.56, there are a number of false positives, which
+// All of these should be checked and hopefully removed after upgrading
+// cppcheck past 1.56
+
+// eraseDereference: This is a known false positive, which has been
+// fixed in the current development version of cppcheck
+eraseDereference
+
+// Unused functions: there suddenly are a lot of unused function errors
+// We could address those by adding for instance early declarations or
+// (unnecessary) header files, but they were all somewhat false positives
+// When we upgrade past 1.56, we should re-check this, and perhaps enable
+// unused-functions again.
+unusedFunction

src/lib/acl/dns.cc

@@ -12,12 +12,6 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <memory>
-#include <string>
-#include <vector>
-
-#include <boost/shared_ptr.hpp>
-
 #include <exceptions/exceptions.h>
 
 #include <dns/name.h>
@@ -31,6 +25,13 @@
 #include <acl/loader.h>
 #include <acl/logic_check.h>
 
+#include <boost/shared_ptr.hpp>
+#include <boost/scoped_ptr.hpp>
+
+#include <memory>
+#include <string>
+#include <vector>
+
 using namespace std;
 using namespace isc::dns;
 using namespace isc::data;
@@ -106,10 +107,12 @@ internal::RequestCheckCreator::create(const string& name,
 
 RequestLoader&
 getRequestLoader() {
-    static RequestLoader* loader(NULL);
-    if (loader == NULL) {
+    // To ensure that the singleton gets destroyed at the end of the
+    // program's lifetime, we put it in a static scoped_ptr.
+    static boost::scoped_ptr<RequestLoader> loader(NULL);
+    if (loader.get() == NULL) {
         // Creator registration may throw, so we first store the new loader
-        // in an auto pointer in order to provide the strong exception
+        // in a second auto pointer in order to provide the strong exception
         // guarantee.
         auto_ptr<RequestLoader> loader_ptr =
             auto_ptr<RequestLoader>(new RequestLoader(REJECT));
@@ -129,7 +132,7 @@ getRequestLoader() {
                 new LogicCreator<AllOfSpec, RequestContext>("ALL")));
 
         // From this point there shouldn't be any exception thrown
-        loader = loader_ptr.release();
+        loader.reset(loader_ptr.release());
     }
 
     return (*loader);

src/lib/cache/tests/rrset_entry_unittest.cc

@@ -50,9 +50,6 @@ class DerivedRRsetEntry: public RRsetEntry {
 public:
     DerivedRRsetEntry(const isc::dns::RRset& rrset, const RRsetTrustLevel& level) : RRsetEntry(rrset, level) {};
 
-    void updateTTLForTest() {
-
-    }
 };
 
 #define TEST_TTL 100

src/lib/cc/data.cc

@@ -413,7 +413,7 @@ from_stringstream_number(std::istream &in, int &pos) {
             isc_throw(JSONError, std::string("Number overflow: ") + number);
         }
     }
-    
+
     if (is_double) {
         return (Element::create(d));
     } else {
@@ -501,7 +501,7 @@ from_stringstream_map(std::istream &in, const std::string& file, int& line,
 
             ConstElementPtr value = Element::fromJSON(in, file, line, pos);
             map->set(key, value);
-            
+
             skip_to(in, file, line, pos, ",}", WHITESPACE);
             c = in.get();
             pos++;
@@ -942,7 +942,7 @@ removeIdentical(ConstElementPtr a, ConstElementPtr b) {
     if (!b) {
         return (result);
     }
-    
+
     if (a->getType() != Element::map || b->getType() != Element::map) {
         isc_throw(TypeError, "Non-map Elements passed to removeIdentical");
     }
@@ -965,7 +965,7 @@ merge(ElementPtr element, ConstElementPtr other) {
         other->getType() != Element::map) {
         isc_throw(TypeError, "merge arguments not MapElements");
     }
-    
+
     const std::map<std::string, ConstElementPtr>& m = other->mapValue();
     for (std::map<std::string, ConstElementPtr>::const_iterator it = m.begin();
          it != m.end() ; ++it) {

src/lib/config/ccsession.cc

@@ -338,7 +338,7 @@ getRelatedLoggers(ConstElementPtr loggers) {
 
     // Now find the wildcard names (the one that start with "*").
     BOOST_FOREACH(ConstElementPtr cur_logger, loggers->listValue()) {
-        std::string cur_name = cur_logger->get("name")->stringValue();
+        const std::string cur_name = cur_logger->get("name")->stringValue();
         // If name is '*', or starts with '*.', replace * with root
         // logger name.
         if (cur_name == "*" || cur_name.length() > 1 &&
@@ -671,9 +671,7 @@ ModuleCCSession::fetchRemoteSpec(const std::string& module, bool is_filename) {
 
 std::string
 ModuleCCSession::addRemoteConfig(const std::string& spec_name,
-                                 void (*handler)(const std::string& module,
-                                                 ConstElementPtr,
-                                                 const ConfigData&),
+                                 RemoteHandler handler,
                                  bool spec_is_filename)
 {
     // First get the module name, specification and default config

src/lib/config/ccsession.h

@@ -283,7 +283,7 @@ public:
      *                  the configuration manager must know it). If
      *                  spec_is_filename is true (the default), then a
      *                  filename is assumed, otherwise a module name.
-     * \param handler The handler function called whenever there's a change.
+     * \param handler The handler functor called whenever there's a change.
      *                Called once initally from this function. May be NULL
      *                if you don't want any handler to be called and you're
      *                fine with requesting the data through
@@ -296,11 +296,11 @@ public:
      * \return The name of the module specified in the given specification
      *         file
      */
+    typedef boost::function<void(const std::string&,
+                                 isc::data::ConstElementPtr,
+                                 const ConfigData&)> RemoteHandler;
     std::string addRemoteConfig(const std::string& spec_name,
-                                void (*handler)(const std::string& module_name,
-                                                isc::data::ConstElementPtr
-                                                update,
-                                                const ConfigData& config_data) = NULL,
+                                RemoteHandler handler = RemoteHandler(),
                                 bool spec_is_filename = true);
 
     /**
@@ -513,9 +513,6 @@ private:
         const std::string& command,
         isc::data::ConstElementPtr args);
 
-    typedef void (*RemoteHandler)(const std::string&,
-                                  isc::data::ConstElementPtr,
-                                  const ConfigData&);
     std::map<std::string, ConfigData> remote_module_configs_;
     std::map<std::string, RemoteHandler> remote_module_handlers_;
 

src/lib/datasrc/memory/domaintree.h

@@ -947,7 +947,7 @@ public:
         PARTIALMATCH, ///< A superdomain node was found
         NOTFOUND,   ///< Not even any superdomain was found
         /// \brief Returned by insert() if a node of the name already exists
-        ALREADYEXISTS,
+        ALREADYEXISTS
     };
 
     /// \brief Allocate and construct \c DomainTree
@@ -1080,55 +1080,25 @@ public:
     ///    of it. In that case, node parameter is left intact.
     //@{
 
-    /// \brief Simple find.
+    /// \brief Simple find
     ///
     /// Acts as described in the \ref find section.
     Result find(const isc::dns::Name& name,
-                DomainTreeNode<T>** node) const {
-        DomainTreeNodeChain<T> node_path;
-        const isc::dns::LabelSequence ls(name);
-        return (find<void*>(ls, node, node_path, NULL, NULL));
-    }
-
-    /// \brief Simple find returning immutable node.
-    ///
-    /// Acts as described in the \ref find section, but returns immutable node
-    /// pointer.
-    Result find(const isc::dns::Name& name,
                 const DomainTreeNode<T>** node) const {
         DomainTreeNodeChain<T> node_path;
-        DomainTreeNode<T> *target_node = NULL;
         const isc::dns::LabelSequence ls(name);
-        Result ret = (find<void*>(ls, &target_node, node_path, NULL, NULL));
-        if (ret != NOTFOUND) {
-            *node = target_node;
-        }
+        Result ret = (find<void*>(ls, node, node_path, NULL, NULL));
         return (ret);
     }
 
     /// \brief Simple find, with node_path tracking
     ///
     /// Acts as described in the \ref find section.
-    Result find(const isc::dns::Name& name, DomainTreeNode<T>** node,
-                DomainTreeNodeChain<T>& node_path) const
-    {
-        const isc::dns::LabelSequence ls(name);
-        return (find<void*>(ls, node, node_path, NULL, NULL));
-    }
-
-    /// \brief Simple find returning immutable node, with node_path tracking
-    ///
-    /// Acts as described in the \ref find section, but returns immutable node
-    /// pointer.
     Result find(const isc::dns::Name& name, const DomainTreeNode<T>** node,
                 DomainTreeNodeChain<T>& node_path) const
     {
-        DomainTreeNode<T> *target_node = NULL;
         const isc::dns::LabelSequence ls(name);
-        Result ret = (find<void*>(ls, &target_node, node_path, NULL, NULL));
-        if (ret != NOTFOUND) {
-            *node = target_node;
-        }
+        Result ret = (find<void*>(ls, node, node_path, NULL, NULL));
         return (ret);
     }
 
@@ -1143,13 +1113,9 @@ public:
                 bool (*callback)(const DomainTreeNode<T>&, CBARG),
                 CBARG callback_arg) const
     {
-        DomainTreeNode<T>* target_node = NULL;
         const isc::dns::LabelSequence ls(name);
-        Result ret = find(ls, &target_node, node_path, callback,
+        Result ret = find(ls, node, node_path, callback,
                           callback_arg);
-        if (ret != NOTFOUND) {
-            *node = target_node;
-        }
         return (ret);
     }
 
@@ -1229,30 +1195,10 @@ public:
     ///     \c true, it returns immediately with the current node.
     template <typename CBARG>
     Result find(const isc::dns::LabelSequence& target_labels_orig,
-                DomainTreeNode<T>** node,
-                DomainTreeNodeChain<T>& node_path,
-                bool (*callback)(const DomainTreeNode<T>&, CBARG),
-                CBARG callback_arg) const;
-
-    /// \brief Simple find returning immutable node.
-    ///
-    /// Acts as described in the \ref find section, but returns immutable
-    /// node pointer.
-    template <typename CBARG>
-    Result find(const isc::dns::LabelSequence& target_labels,
                 const DomainTreeNode<T>** node,
                 DomainTreeNodeChain<T>& node_path,
                 bool (*callback)(const DomainTreeNode<T>&, CBARG),
-                CBARG callback_arg) const
-    {
-        DomainTreeNode<T>* target_node = NULL;
-        Result ret = find(target_labels, &target_node, node_path,
-                          callback, callback_arg);
-        if (ret != NOTFOUND) {
-            *node = target_node;
-        }
-        return (ret);
-    }
+                CBARG callback_arg) const;
     //@}
 
     /// \brief return the next bigger node in DNSSEC order from a given node
@@ -1515,7 +1461,7 @@ template <typename T>
 template <typename CBARG>
 typename DomainTree<T>::Result
 DomainTree<T>::find(const isc::dns::LabelSequence& target_labels_orig,
-                    DomainTreeNode<T>** target,
+                    const DomainTreeNode<T>** target,
                     DomainTreeNodeChain<T>& node_path,
                     bool (*callback)(const DomainTreeNode<T>&, CBARG),
                     CBARG callback_arg) const
@@ -1526,11 +1472,11 @@ DomainTree<T>::find(const isc::dns::LabelSequence& target_labels_orig,
                   " and label sequence");
     }
 
-    DomainTreeNode<T>* node;
+    const DomainTreeNode<T>* node;
 
     if (!node_path.isEmpty()) {
         // Get the top node in the node chain
-        node = const_cast<DomainTreeNode<T>*>(node_path.top());
+        node = node_path.top();
         // Start searching from its down pointer
         node = node->getDown();
     } else {

src/lib/datasrc/memory/memory_client.cc

@@ -627,22 +627,20 @@ InMemoryClient::InMemoryClientImpl::load(
     // node must point to a valid node now
     assert(node != NULL);
 
-    std::string* tstr = node->setData(new std::string(filename));
+    const std::string* tstr = node->setData(new std::string(filename));
     delete tstr;
 
-    ZoneTable::AddResult result(zone_table_->addZone(mem_sgmt_, rrclass_,
-                                                     zone_name));
+    const ZoneTable::AddResult result(zone_table_->addZone(mem_sgmt_, rrclass_,
+                                                           zone_name,
+                                                           holder.release()));
     if (result.code == result::SUCCESS) {
         // Only increment the zone count if the zone doesn't already
         // exist.
         ++zone_count_;
     }
-
-    ZoneTable::FindResult fr(zone_table_->setZoneData(zone_name,
-                                                      holder.release()));
-    assert(fr.code == result::SUCCESS);
-    if (fr.zone_data != NULL) {
-        ZoneData::destroy(mem_sgmt_, fr.zone_data, rrclass_);
+    // Destroy the old instance of the zone if there was any
+    if (result.zone_data != NULL) {
+        ZoneData::destroy(mem_sgmt_, result.zone_data, rrclass_);
     }
 
     return (result.code);
@@ -732,9 +730,9 @@ InMemoryClient::load(const isc::dns::Name& zone_name,
 
 const std::string
 InMemoryClient::getFileName(const isc::dns::Name& zone_name) const {
-    FileNameNode* node(NULL);
-    FileNameTree::Result result = impl_->file_name_tree_->find(zone_name,
-                                                               &node);
+    const FileNameNode* node(NULL);
+    const FileNameTree::Result result = impl_->file_name_tree_->find(zone_name,
+                                                                     &node);
     if (result == FileNameTree::EXACTMATCH) {
         return (*node->getData());
     } else {
@@ -742,24 +740,6 @@ InMemoryClient::getFileName(const isc::dns::Name& zone_name) const {
     }
 }
 
-result::Result
-InMemoryClient::add(const isc::dns::Name& zone_name,
-                    const ConstRRsetPtr& rrset)
-{
-    const ZoneTable::FindResult result =
-        impl_->zone_table_->findZone(zone_name);
-    if (result.code != result::SUCCESS) {
-        isc_throw(DataSourceError, "No such zone: " + zone_name.toText());
-    }
-
-    const ConstRRsetPtr sig_rrset =
-        rrset ? rrset->getRRsig() : ConstRRsetPtr();
-    impl_->add(rrset, sig_rrset, zone_name, *result.zone_data);
-
-    // add() doesn't allow duplicate add, so we always return SUCCESS.
-    return (result::SUCCESS);
-}
-
 namespace {
 
 class MemoryIterator : public ZoneIterator {

src/lib/datasrc/memory/memory_client.h

@@ -139,35 +139,6 @@ public:
     /// zone from a file before.
     const std::string getFileName(const isc::dns::Name& zone_name) const;
 
-    /// \brief Inserts an rrset into the zone.
-    ///
-    /// It puts another RRset into the zone.
-    ///
-    /// In the current implementation, this method doesn't allow an existing
-    /// RRset to be updated or overridden.  So the caller must make sure that
-    /// all RRs of the same type and name must be given in the form of a
-    /// single RRset.  The current implementation will also require that
-    /// when an RRSIG is added, the RRset to be covered has already been
-    /// added.  These restrictions are probably too strict when this data
-    /// source accepts various forms of input, so they should be revisited
-    /// later.
-    ///
-    /// Except for NullRRset and OutOfZone, this method does not guarantee
-    /// strong exception safety (it is currently not needed, if it is needed
-    /// in future, it should be implemented).
-    ///
-    /// \throw NullRRset \c rrset is a NULL pointer.
-    /// \throw OutOfZone The owner name of \c rrset is outside of the
-    /// origin of the zone.
-    /// \throw AddError Other general errors.
-    /// \throw Others This method might throw standard allocation exceptions.
-    ///
-    /// \param rrset The set to add.
-    /// \return SUCCESS or EXIST (if an rrset for given name and type already
-    ///    exists).
-    result::Result add(const isc::dns::Name& zone_name,
-                       const isc::dns::ConstRRsetPtr& rrset);
-
     /// \brief RRset is NULL exception.
     ///
     /// This is thrown if the provided RRset parameter is NULL.

src/lib/datasrc/memory/zone_finder.cc

@@ -428,7 +428,7 @@ FindNodeResult findNode(const ZoneData& zone_data,
                         ZoneFinder::FindOptions options,
                         bool out_of_zone_ok = false)
 {
-    ZoneNode* node = NULL;
+    const ZoneNode* node = NULL;
     FindState state((options & ZoneFinder::FIND_GLUE_OK) != 0);
 
     const ZoneTree& tree(zone_data.getZoneTree());

src/lib/datasrc/memory/zone_table.cc

@@ -69,17 +69,13 @@ ZoneTable::destroy(util::MemorySegment& mem_sgmt, ZoneTable* ztable,
 
 ZoneTable::AddResult
 ZoneTable::addZone(util::MemorySegment& mem_sgmt, RRClass zone_class,
-                   const Name& zone_name)
+                   const Name& zone_name, ZoneData* content)
 {
-    // Create a new ZoneData instance first.  If the specified name already
-    // exists in the table, the new data will soon be destroyed, but we want
-    // to make sure if this allocation fails the tree won't be changed to
-    // provide as strong guarantee as possible.  In practice, we generally
-    // expect the caller tries to add a zone only when it's a new one, so
-    // this should be a minor concern.
-    SegmentObjectHolder<ZoneData, RRClass> holder(
-        mem_sgmt, ZoneData::create(mem_sgmt, zone_name), zone_class);
-
+    if (content == NULL) {
+        isc_throw(isc::BadValue, "Zone content must not be NULL");
+    }
+    SegmentObjectHolder<ZoneData, RRClass> holder(mem_sgmt, content,
+                                                  zone_class);
     // Get the node where we put the zone
     ZoneTableNode* node(NULL);
     switch (zones_->insert(mem_sgmt, zone_name, &node)) {
@@ -94,18 +90,18 @@ ZoneTable::addZone(util::MemorySegment& mem_sgmt, RRClass zone_class,
     // Can Not Happen
     assert(node != NULL);
 
-    // Is it empty? We either just created it or it might be nonterminal
-    if (node->isEmpty()) {
-        node->setData(holder.get());
-        return (AddResult(result::SUCCESS, holder.release()));
-    } else { // There's something there already
-        return (AddResult(result::EXIST, node->getData()));
+    // We can release now, setData never throws
+    ZoneData* old = node->setData(holder.release());
+    if (old != NULL) {
+        return (AddResult(result::EXIST, old));
+    } else {
+        return (AddResult(result::SUCCESS, NULL));
     }
 }
 
 ZoneTable::FindResult
 ZoneTable::findZone(const Name& name) const {
-    ZoneTableNode* node(NULL);
+    const ZoneTableNode* node(NULL);
     result::Result my_result;
 
     // Translate the return codes
@@ -132,20 +128,6 @@ ZoneTable::findZone(const Name& name) const {
     return (FindResult(my_result, node->getData()));
 }
 
-ZoneTable::FindResult
-ZoneTable::setZoneData(const Name& name, ZoneData* data)
-{
-    ZoneTableNode* node(NULL);
-
-    ZoneTableTree::Result result(zones_->find(name, &node));
-
-    if (result != ZoneTableTree::EXACTMATCH) {
-        return (FindResult(result::NOTFOUND, NULL));
-    } else {
-        return (FindResult(result::SUCCESS, node->setData(data)));
-    }
-}
-
 } // end of namespace memory
 } // end of namespace datasrc
 } // end of namespace isc

src/lib/datasrc/memory/zone_table.h

@@ -74,23 +74,23 @@ private:
     typedef DomainTreeNode<ZoneData> ZoneTableNode;
 
 public:
-    /// \brief Result data of addZone() method.
-    struct AddResult {
-        AddResult(result::Result param_code, ZoneData* param_zone_data) :
-            code(param_code), zone_data(param_zone_data)
-        {}
-        const result::Result code;
-        ZoneData* const zone_data;
-    };
+     /// \brief Result data of addZone() method.
+     struct AddResult {
+         AddResult(result::Result param_code, ZoneData* param_zone_data) :
+             code(param_code), zone_data(param_zone_data)
+         {}
+         const result::Result code;
+         ZoneData* const zone_data;
+     };
 
     /// \brief Result data of findZone() method.
     struct FindResult {
         FindResult(result::Result param_code,
-                   ZoneData* param_zone_data) :
+                   const ZoneData* param_zone_data) :
             code(param_code), zone_data(param_zone_data)
         {}
         const result::Result code;
-        ZoneData* const zone_data;
+        const ZoneData* const zone_data;
     };
 
 private:
@@ -140,30 +140,29 @@ public:
 
     /// Add a new zone to the \c ZoneTable.
     ///
-    /// This method creates a new \c ZoneData for the given zone name and
-    /// holds it in the internal table.  The newly created zone data will be
-    /// returned via the \c zone_data member of the return value.  If the given
-    /// zone name already exists in the table, a new data object won't be
-    /// created; instead, the existing corresponding data will be returned.
-    ///
-    /// The zone table keeps the ownership of the created zone data; the
-    /// caller must not try to destroy it directly.  (We'll eventually
-    /// add an interface to delete specific zone data from the table).
+    /// This method adds a given zone data to the internal table.
     ///
     /// \throw std::bad_alloc Internal resource allocation fails.
     ///
     /// \param mem_sgmt The \c MemorySegment to allocate zone data to be
-    /// created.  It must be the same segment that was used to create
-    /// the zone table at the time of create().
+    ///     created.  It must be the same segment that was used to create
+    ///     the zone table at the time of create().
     /// \param zone_name The name of the zone to be added.
     /// \param zone_class The RR class of the zone.  It must be the RR class
-    /// that is supposed to be associated to the zone table.
+    ///     that is supposed to be associated to the zone table.
+    /// \param content This one should hold the zone content (the ZoneData).
+    ///     The ownership is passed onto the zone table. Must not be null.
+    ///     Must correspond to the name and class and must be allocated from
+    ///     mem_sgmt.
     /// \return \c result::SUCCESS If the zone is successfully
-    /// added to the zone table.
-    /// \return \c result::EXIST The zone table already contains
-    /// zone of the same origin.
-    AddResult addZone(util::MemorySegment& mem_sgmt, dns::RRClass zone_class,
-                      const dns::Name& zone_name);
+    ///     added to the zone table.
+    /// \return \c result::EXIST The zone table already contained
+    ///     zone of the same origin. The old data is replaced and returned
+    ///     inside the result.
+    AddResult addZone(util::MemorySegment& mem_sgmt,
+                      dns::RRClass zone_class,
+                      const dns::Name& zone_name,
+                      ZoneData* content);
 
     /// Find a zone that best matches the given name in the \c ZoneTable.
     ///
@@ -185,16 +184,6 @@ public:
     /// \return A \c FindResult object enclosing the search result (see above).
     FindResult findZone(const isc::dns::Name& name) const;
 
-    /// Override the ZoneData for a node (zone) in the zone tree.
-    ///
-    /// \throw none
-    ///
-    /// \param name A domain name for which the zone data is set.
-    /// \param data The new zone data to set.
-    /// \return A \c FindResult object containing the old data if the
-    /// zone was found.
-    FindResult setZoneData(const isc::dns::Name& name, ZoneData* data);
-
 private:
     boost::interprocess::offset_ptr<ZoneTableTree> zones_;
 };

src/lib/datasrc/sqlite3_accessor.cc

@@ -44,7 +44,7 @@ namespace {
 // program may not be taking advantage of features (possibly performance
 // improvements) added to the database.
 const int SQLITE_SCHEMA_MAJOR_VERSION = 2;
-const int SQLITE_SCHEMA_MINOR_VERSION = 0;
+const int SQLITE_SCHEMA_MINOR_VERSION = 1;
 }
 
 namespace isc {
@@ -65,19 +65,20 @@ enum StatementID {
     DEL_ZONE_RECORDS = 6,
     ADD_RECORD = 7,
     DEL_RECORD = 8,
-    ITERATE = 9,
-    FIND_PREVIOUS = 10,
-    ADD_RECORD_DIFF = 11,
-    LOW_DIFF_ID = 12,
-    HIGH_DIFF_ID = 13,
-    DIFF_RECS = 14,
-    NSEC3 = 15,
-    NSEC3_PREVIOUS = 16,
-    NSEC3_LAST = 17,
-    ADD_NSEC3_RECORD = 18,
-    DEL_ZONE_NSEC3_RECORDS = 19,
-    DEL_NSEC3_RECORD = 20,
-    NUM_STATEMENTS = 21
+    ITERATE_RECORDS = 9,
+    ITERATE_NSEC3 = 10,
+    FIND_PREVIOUS = 11,
+    ADD_RECORD_DIFF = 12,
+    LOW_DIFF_ID = 13,
+    HIGH_DIFF_ID = 14,
+    DIFF_RECS = 15,
+    NSEC3 = 16,
+    NSEC3_PREVIOUS = 17,
+    NSEC3_LAST = 18,
+    ADD_NSEC3_RECORD = 19,
+    DEL_ZONE_NSEC3_RECORDS = 20,
+    DEL_NSEC3_RECORD = 21,
+    NUM_STATEMENTS = 22
 };
 
 const char* const text_statements[NUM_STATEMENTS] = {
@@ -102,19 +103,17 @@ const char* const text_statements[NUM_STATEMENTS] = {
         "VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7)",
     "DELETE FROM records WHERE zone_id=?1 AND name=?2 " // DEL_RECORD
         "AND rdtype=?3 AND rdata=?4",
-    // The following iterates the whole zone. As the NSEC3 records
-    // (and corresponding RRSIGs) live in separate table, we need to
-    // take both of them. As the RRSIGs are for NSEC3s in the other
-    // table, we can easily hardcode the sigtype.
-    //
-    // The extra column is so we can order it by rname. This is to
-    // preserve the previous order, mostly for tests.
-    // TODO: Is it possible to get rid of the ordering?
-    "SELECT rdtype, ttl, sigtype, rdata, name, rname FROM records " // ITERATE
-        "WHERE zone_id = ?1 "
-        "UNION "
-        "SELECT rdtype, ttl, \"NSEC3\", rdata, owner, owner FROM nsec3 "
-        "WHERE zone_id = ?1 ORDER by rname, rdtype",
+
+    // ITERATE_RECORDS:
+    // The following iterates the whole zone in the records table.
+    "SELECT rdtype, ttl, sigtype, rdata, name FROM records "
+        "WHERE zone_id = ?1 ORDER BY rname, rdtype",
+
+    // ITERATE_NSEC3:
+    // The following iterates the whole zone in the nsec3 table. As the
+    // RRSIGs are for NSEC3s, we can hardcode the sigtype.
+    "SELECT rdtype, ttl, \"NSEC3\", rdata, owner FROM nsec3 "
+        "WHERE zone_id = ?1 ORDER BY hash, rdtype",
     /*
      * This one looks for previous name with NSEC record. It is done by
      * using the reversed name. The NSEC is checked because we need to
@@ -332,7 +331,7 @@ public:
 const char* const SCHEMA_LIST[] = {
     "CREATE TABLE schema_version (version INTEGER NOT NULL, "
         "minor INTEGER NOT NULL DEFAULT 0)",
-    "INSERT INTO schema_version VALUES (2, 0)",
+    "INSERT INTO schema_version VALUES (2, 1)",
     "CREATE TABLE zones (id INTEGER PRIMARY KEY, "
     "name TEXT NOT NULL COLLATE NOCASE, "
     "rdclass TEXT NOT NULL COLLATE NOCASE DEFAULT 'IN', "
@@ -358,6 +357,7 @@ const char* const SCHEMA_LIST[] = {
         "ttl INTEGER NOT NULL, rdtype TEXT NOT NULL COLLATE NOCASE, "
         "rdata TEXT NOT NULL)",
     "CREATE INDEX nsec3_byhash ON nsec3 (hash)",
+    "CREATE INDEX nsec3_byhash_and_rdtype ON nsec3 (hash, rdtype)",
     "CREATE TABLE diffs (id INTEGER PRIMARY KEY, "
         "zone_id INTEGER NOT NULL, "
         "version INTEGER NOT NULL, "
@@ -642,11 +642,21 @@ public:
         iterator_type_(ITT_ALL),
         accessor_(accessor),
         statement_(NULL),
+        statement2_(NULL),
+        rc_(SQLITE_OK),
+        rc2_(SQLITE_OK),
         name_("")
     {
-        // We create the statement now and then just keep getting data from it
+        // We create the statements now and then just keep getting data
+        // from them.
         statement_ = prepare(accessor->dbparameters_->db_,
-                             text_statements[ITERATE]);
+                             text_statements[ITERATE_NSEC3]);
+        bindZoneId(id);
+
+        std::swap(statement_, statement2_);
+
+        statement_ = prepare(accessor->dbparameters_->db_,
+                             text_statements[ITERATE_RECORDS]);
         bindZoneId(id);
     }
 
@@ -665,6 +675,9 @@ public:
         iterator_type_(qtype == QT_NSEC3 ? ITT_NSEC3 : ITT_NAME),
         accessor_(accessor),
         statement_(NULL),
+        statement2_(NULL),
+        rc_(SQLITE_OK),
+        rc2_(SQLITE_OK),
         name_(name)
     {
         // Choose the statement text depending on the query type, and
@@ -703,29 +716,35 @@ public:
         // If there's another row, get it
         // If finalize has been called (e.g. when previous getNext() got
         // SQLITE_DONE), directly return false
-        if (statement_ == NULL) {
-            return false;
-        }
-        const int rc(sqlite3_step(statement_));
-        if (rc == SQLITE_ROW) {
-            // For both types, we copy the first four columns
-            copyColumn(data, TYPE_COLUMN);
-            copyColumn(data, TTL_COLUMN);
-            // The NSEC3 lookup does not provide the SIGTYPE, it is not
-            // necessary and not contained in the table.
-            if (iterator_type_ != ITT_NSEC3) {
-                copyColumn(data, SIGTYPE_COLUMN);
+        while (statement_ != NULL) {
+            rc_ = sqlite3_step(statement_);
+            if (rc_ == SQLITE_ROW) {
+                // For both types, we copy the first four columns
+                copyColumn(data, TYPE_COLUMN);
+                copyColumn(data, TTL_COLUMN);
+                // The NSEC3 lookup does not provide the SIGTYPE, it is not
+                // necessary and not contained in the table.
+                if (iterator_type_ != ITT_NSEC3) {
+                    copyColumn(data, SIGTYPE_COLUMN);
+                }
+                copyColumn(data, RDATA_COLUMN);
+                // Only copy Name if we are iterating over every record
+                if (iterator_type_ == ITT_ALL) {
+                    copyColumn(data, NAME_COLUMN);
+                }
+                return (true);
+            } else if (rc_ != SQLITE_DONE) {
+                isc_throw(DataSourceError,
+                          "Unexpected failure in sqlite3_step: " <<
+                          sqlite3_errmsg(accessor_->dbparameters_->db_));
             }
-            copyColumn(data, RDATA_COLUMN);
-            // Only copy Name if we are iterating over every record
-            if (iterator_type_ == ITT_ALL) {
-                copyColumn(data, NAME_COLUMN);
+            // We are done with statement_. If statement2_ has not been
+            // used yet, try that one now.
+            if ((statement2_ == NULL) || (rc2_ != SQLITE_OK)) {
+                break;
             }
-            return (true);
-        } else if (rc != SQLITE_DONE) {
-            isc_throw(DataSourceError,
-                      "Unexpected failure in sqlite3_step: " <<
-                      sqlite3_errmsg(accessor_->dbparameters_->db_));
+            std::swap(statement_, statement2_);
+            std::swap(rc_, rc2_);
         }
         finalize();
         return (false);
@@ -771,13 +790,22 @@ private:
     }
 
     void finalize() {
-        sqlite3_finalize(statement_);
-        statement_ = NULL;
+        if (statement_ != NULL) {
+             sqlite3_finalize(statement_);
+             statement_ = NULL;
+        }
+        if (statement2_ != NULL) {
+             sqlite3_finalize(statement2_);
+             statement2_ = NULL;
+        }
     }
 
     const IteratorType iterator_type_;
     boost::shared_ptr<const SQLite3Accessor> accessor_;
     sqlite3_stmt* statement_;
+    sqlite3_stmt* statement2_;
+    int rc_;
+    int rc2_;
     const std::string name_;
 };
 

src/lib/datasrc/tests/memory/Makefile.am

@@ -39,6 +39,7 @@ run_unittests_LDFLAGS  = $(AM_LDFLAGS)  $(GTEST_LDFLAGS)
 
 run_unittests_LDADD = $(top_builddir)/src/lib/datasrc/libb10-datasrc.la
 run_unittests_LDADD += $(top_builddir)/src/lib/dns/libb10-dns++.la
+run_unittests_LDADD += $(top_builddir)/src/lib/cc/libb10-cc.la
 run_unittests_LDADD += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la
 run_unittests_LDADD += $(top_builddir)/src/lib/util/libb10-util.la
 run_unittests_LDADD += $(top_builddir)/src/lib/testutils/libb10-testutils.la

src/lib/datasrc/tests/memory/domaintree_unittest.cc

@@ -256,8 +256,8 @@ TEST_F(DomainTreeTest, subTreeRoot) {
 
     // "g.h" is not a subtree root
     EXPECT_EQ(TestDomainTree::EXACTMATCH,
-              dtree_expose_empty_node.find(Name("g.h"), &dtnode));
-    EXPECT_FALSE(dtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
+              dtree_expose_empty_node.find(Name("g.h"), &cdtnode));
+    EXPECT_FALSE(cdtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
 
     // fission the node "g.h"
     EXPECT_EQ(TestDomainTree::ALREADYEXISTS,
@@ -270,8 +270,8 @@ TEST_F(DomainTreeTest, subTreeRoot) {
 
     // "g.h" should be a subtree root now.
     EXPECT_EQ(TestDomainTree::EXACTMATCH,
-              dtree_expose_empty_node.find(Name("g.h"), &dtnode));
-    EXPECT_TRUE(dtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
+              dtree_expose_empty_node.find(Name("g.h"), &cdtnode));
+    EXPECT_TRUE(cdtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
 }
 
 TEST_F(DomainTreeTest, additionalNodeFission) {
@@ -286,8 +286,8 @@ TEST_F(DomainTreeTest, additionalNodeFission) {
 
     // "t.0" is not a subtree root
     EXPECT_EQ(TestDomainTree::EXACTMATCH,
-              dtree_expose_empty_node.find(Name("t.0"), &dtnode));
-    EXPECT_FALSE(dtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
+              dtree_expose_empty_node.find(Name("t.0"), &cdtnode));
+    EXPECT_FALSE(cdtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
 
     // fission the node "t.0"
     EXPECT_EQ(TestDomainTree::ALREADYEXISTS,
@@ -300,8 +300,8 @@ TEST_F(DomainTreeTest, additionalNodeFission) {
 
     // "t.0" should be a subtree root now.
     EXPECT_EQ(TestDomainTree::EXACTMATCH,
-              dtree_expose_empty_node.find(Name("t.0"), &dtnode));
-    EXPECT_TRUE(dtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
+              dtree_expose_empty_node.find(Name("t.0"), &cdtnode));
+    EXPECT_TRUE(cdtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
 }
 
 TEST_F(DomainTreeTest, findName) {
@@ -328,10 +328,10 @@ TEST_F(DomainTreeTest, findName) {
     EXPECT_EQ(TestDomainTree::PARTIALMATCH,
               dtree_expose_empty_node.find(Name("m.d.e.f"), &cdtnode));
 
-    // find dtnode
+    // find cdtnode
     EXPECT_EQ(TestDomainTree::EXACTMATCH, dtree.find(Name("q.w.y.d.e.f"),
-                                                   &dtnode));
-    EXPECT_EQ(Name("q"), dtnode->getName());
+                                                   &cdtnode));
+    EXPECT_EQ(Name("q"), cdtnode->getName());
 }
 
 TEST_F(DomainTreeTest, findError) {
@@ -411,11 +411,12 @@ performCallbackTest(TestDomainTree& dtree,
                                                         Name("example"),
                                                         &parentdtnode));
     // the child/parent nodes shouldn't "inherit" the callback flag.
-    // "dtnode" may be invalid due to the insertion, so we need to re-find
-    // it.
+    // "dtnode" should still validly point to "callback.example", but we
+    // explicitly confirm it.
     EXPECT_EQ(TestDomainTree::EXACTMATCH, dtree.find(Name("callback.example"),
-                                                   &dtnode));
-    EXPECT_TRUE(dtnode->getFlag(TestDomainTreeNode::FLAG_CALLBACK));
+                                                     &cdtnode));
+    ASSERT_EQ(dtnode, cdtnode);
+    EXPECT_TRUE(cdtnode->getFlag(TestDomainTreeNode::FLAG_CALLBACK));
     EXPECT_FALSE(subdtnode->getFlag(TestDomainTreeNode::FLAG_CALLBACK));
     EXPECT_FALSE(parentdtnode->getFlag(TestDomainTreeNode::FLAG_CALLBACK));
 

src/lib/datasrc/tests/memory/memory_client_unittest.cc

@@ -543,29 +543,6 @@ TEST_F(MemoryClientTest, loadRRSIGs) {
     EXPECT_EQ(1, client_->getZoneCount());
 }
 
-TEST_F(MemoryClientTest, loadRRSIGsRdataMixedCoveredTypes) {
-    client_->load(Name("example.org"),
-                  TEST_DATA_DIR "/example.org-rrsigs.zone");
-
-    RRsetPtr rrset(new RRset(Name("example.org"),
-                             RRClass::IN(), RRType::A(), RRTTL(3600)));
-    rrset->addRdata(in::A("192.0.2.1"));
-    rrset->addRdata(in::A("192.0.2.2"));
-
-    RRsetPtr rrsig(new RRset(Name("example.org"), zclass_,
-                             RRType::RRSIG(), RRTTL(300)));
-    rrsig->addRdata(generic::RRSIG("A 5 3 3600 20000101000000 20000201000000 "
-                                   "12345 example.org. FAKEFAKEFAKE"));
-    rrsig->addRdata(generic::RRSIG("NS 5 3 3600 20000101000000 20000201000000 "
-                                   "54321 example.org. FAKEFAKEFAKEFAKE"));
-    rrset->addRRsig(rrsig);
-
-    EXPECT_THROW(client_->add(Name("example.org"), rrset),
-                 InMemoryClient::AddError);
-
-    // Teardown checks for memory segment leaks
-}
-
 TEST_F(MemoryClientTest, getZoneCount) {
     EXPECT_EQ(0, client_->getZoneCount());
     client_->load(Name("example.org"), TEST_DATA_DIR "/example.org-empty.zone");
@@ -655,75 +632,6 @@ TEST_F(MemoryClientTest, getIteratorGetSOAThrowsNotImplemented) {
     EXPECT_THROW(iterator->getSOA(), isc::NotImplemented);
 }
 
-TEST_F(MemoryClientTest, addRRsetToNonExistentZoneThrows) {
-    // The zone "example.org" doesn't exist, so we can't add an RRset to
-    // it.
-    RRsetPtr rrset_a(new RRset(Name("example.org"), RRClass::IN(), RRType::A(),
-                               RRTTL(300)));
-    rrset_a->addRdata(rdata::in::A("192.0.2.1"));
-    EXPECT_THROW(client_->add(Name("example.org"), rrset_a), DataSourceError);
-}
-
-TEST_F(MemoryClientTest, addOutOfZoneThrows) {
-    // Out of zone names should throw.
-    client_->load(Name("example.org"),
-                  TEST_DATA_DIR "/example.org-empty.zone");
-
-    RRsetPtr rrset_a(new RRset(Name("a.example.com"),
-                               RRClass::IN(), RRType::A(), RRTTL(300)));
-    rrset_a->addRdata(rdata::in::A("192.0.2.1"));
-
-    EXPECT_THROW(client_->add(Name("example.org"), rrset_a),
-                 OutOfZone);
-    // Teardown checks for memory segment leaks
-}
-
-TEST_F(MemoryClientTest, addNullRRsetThrows) {
-    client_->load(Name("example.org"),
-                  TEST_DATA_DIR "/example.org-rrsigs.zone");
-
-    EXPECT_THROW(client_->add(Name("example.org"), ConstRRsetPtr()),
-                 InMemoryClient::NullRRset);
-
-    // Teardown checks for memory segment leaks
-}
-
-TEST_F(MemoryClientTest, addEmptyRRsetThrows) {
-    client_->load(Name("example.org"),
-                  TEST_DATA_DIR "/example.org-rrsigs.zone");
-
-    RRsetPtr rrset_a(new RRset(Name("example.org"), RRClass::IN(), RRType::A(),
-                               RRTTL(300)));
-    EXPECT_THROW(client_->add(Name("example.org"), rrset_a),
-                 InMemoryClient::AddError);
-
-    // Teardown checks for memory segment leaks
-}
-
-TEST_F(MemoryClientTest, add) {
-    client_->load(Name("example.org"), TEST_DATA_DIR "/example.org-empty.zone");
-
-    // Add another RRset
-    RRsetPtr rrset_a(new RRset(Name("example.org"), RRClass::IN(), RRType::A(),
-                               RRTTL(300)));
-    rrset_a->addRdata(rdata::in::A("192.0.2.1"));
-    client_->add(Name("example.org"), rrset_a);
-
-    ZoneIteratorPtr iterator(client_->getIterator(Name("example.org")));
-
-    // First we have the SOA
-    ConstRRsetPtr rrset(iterator->getNextRRset());
-    EXPECT_TRUE(rrset);
-    EXPECT_EQ(RRType::A(), rrset->getType());
-
-    rrset = iterator->getNextRRset();
-    EXPECT_TRUE(rrset);
-    EXPECT_EQ(RRType::SOA(), rrset->getType());
-
-    // There's nothing else in this zone
-    EXPECT_EQ(ConstRRsetPtr(), iterator->getNextRRset());
-}
-
 TEST_F(MemoryClientTest, findZoneData) {
     client_->load(Name("example.org"),
                   TEST_DATA_DIR "/example.org-rrsigs.zone");
@@ -774,4 +682,16 @@ TEST_F(MemoryClientTest, getJournalReaderNotImplemented) {
     EXPECT_THROW(client_->getJournalReader(Name("."), 0, 0),
                  isc::NotImplemented);
 }
+
+// TODO (upon merge of #2268): Re-add (and modify not to need
+// InMemoryClient::add) the tests removed in
+// 7a628baa1a158b5837d6f383e10b30542d2ac59b. Maybe some of them
+// are really not needed.
+//
+// * MemoryClientTest::loadRRSIGsRdataMixedCoveredTypes
+// * MemoryClientTest::addRRsetToNonExistentZoneThrows
+// * MemoryClientTest::addOutOfZoneThrows
+// * MemoryClientTest::addNullRRsetThrows
+// * MemoryClientTest::addEmptyRRsetThrows
+// * MemoryClientTest::add
 }

src/lib/datasrc/tests/memory/zone_data_unittest.cc

@@ -108,7 +108,7 @@ void
 checkFindRdataSet(const ZoneTree& tree, const Name& name, RRType type,
                   const RdataSet* expected_set)
 {
-    ZoneNode* node = NULL;
+    const ZoneNode* node = NULL;
     tree.find(name, &node);
     ASSERT_NE(static_cast<ZoneNode*>(NULL), node);
     EXPECT_EQ(expected_set, RdataSet::find(node->getData(), type));

src/lib/datasrc/tests/memory/zone_table_unittest.cc

@@ -22,6 +22,7 @@
 #include <datasrc/result.h>
 #include <datasrc/memory/zone_data.h>
 #include <datasrc/memory/zone_table.h>
+#include <datasrc/memory/segment_object_holder.h>
 
 #include <gtest/gtest.h>
 
@@ -30,6 +31,7 @@
 using namespace isc::dns;
 using namespace isc::datasrc;
 using namespace isc::datasrc::memory;
+using namespace isc::datasrc::memory::detail;
 
 namespace {
 // Memory segment specified for tests.  It normally behaves like a "local"
@@ -87,46 +89,89 @@ TEST_F(ZoneTableTest, create) {
 }
 
 TEST_F(ZoneTableTest, addZone) {
+    // It doesn't accept empty (NULL) zones
+    EXPECT_THROW(zone_table->addZone(mem_sgmt_, zclass_, zname1, NULL),
+                 isc::BadValue);
+
+    SegmentObjectHolder<ZoneData, RRClass> holder1(
+        mem_sgmt_, ZoneData::create(mem_sgmt_, zname1), zclass_);
+    const ZoneData* data1(holder1.get());
     // Normal successful case.
-    const ZoneTable::AddResult result1 =
-        zone_table->addZone(mem_sgmt_, zclass_, zname1);
+    const ZoneTable::AddResult result1(zone_table->addZone(mem_sgmt_, zclass_,
+                                                           zname1,
+                                                           holder1.release()));
     EXPECT_EQ(result::SUCCESS, result1.code);
+    EXPECT_EQ(static_cast<const ZoneData*>(NULL), result1.zone_data);
+    // It got released by it
+    EXPECT_EQ(static_cast<const ZoneData*>(NULL), holder1.get());
 
     // Duplicate add doesn't replace the existing data.
-    EXPECT_EQ(result::EXIST, zone_table->addZone(mem_sgmt_, zclass_,
-                                                 zname1).code);
-    EXPECT_EQ(result1.zone_data,
-              zone_table->addZone(mem_sgmt_, zclass_, zname1).zone_data);
+    SegmentObjectHolder<ZoneData, RRClass> holder2(
+        mem_sgmt_, ZoneData::create(mem_sgmt_, zname1), zclass_);
+    const ZoneTable::AddResult result2(zone_table->addZone(mem_sgmt_, zclass_,
+                                                           zname1,
+                                                           holder2.release()));
+    EXPECT_EQ(result::EXIST, result2.code);
+    // The old one gets out
+    EXPECT_EQ(data1, result2.zone_data);
+    // It releases this one even when we replace the old zone
+    EXPECT_EQ(static_cast<const ZoneData*>(NULL), holder2.get());
+    // We need to release the old one manually
+    ZoneData::destroy(mem_sgmt_, result2.zone_data, zclass_);
+
+    SegmentObjectHolder<ZoneData, RRClass> holder3(
+        mem_sgmt_, ZoneData::create(mem_sgmt_, Name("EXAMPLE.COM")),
+                                    zclass_);
     // names are compared in a case insensitive manner.
-    EXPECT_EQ(result::EXIST, zone_table->addZone(mem_sgmt_, zclass_,
-                                                 Name("EXAMPLE.COM")).code);
+    const ZoneTable::AddResult result3(zone_table->addZone(mem_sgmt_, zclass_,
+                                                           Name("EXAMPLE.COM"),
+                                                           holder3.release()));
+    EXPECT_EQ(result::EXIST, result3.code);
+    ZoneData::destroy(mem_sgmt_, result3.zone_data, zclass_);
     // Add some more different ones.  Should just succeed.
+    SegmentObjectHolder<ZoneData, RRClass> holder4(
+        mem_sgmt_, ZoneData::create(mem_sgmt_, zname2), zclass_);
     EXPECT_EQ(result::SUCCESS,
-              zone_table->addZone(mem_sgmt_, zclass_, zname2).code);
+              zone_table->addZone(mem_sgmt_, zclass_, zname2,
+                                  holder4.release()).code);
+    SegmentObjectHolder<ZoneData, RRClass> holder5(
+        mem_sgmt_, ZoneData::create(mem_sgmt_, zname3), zclass_);
     EXPECT_EQ(result::SUCCESS,
-              zone_table->addZone(mem_sgmt_, zclass_, zname3).code);
+              zone_table->addZone(mem_sgmt_, zclass_, zname3,
+                                  holder5.release()).code);
 
     // Have the memory segment throw an exception in extending the internal
     // tree.  It still shouldn't cause memory leak (which would be detected
     // in TearDown()).
-    mem_sgmt_.setThrowCount(2);
-    EXPECT_THROW(zone_table->addZone(mem_sgmt_, zclass_, Name("example.org")),
+    SegmentObjectHolder<ZoneData, RRClass> holder6(
+        mem_sgmt_, ZoneData::create(mem_sgmt_, Name("example.org")), zclass_);
+    mem_sgmt_.setThrowCount(1);
+    EXPECT_THROW(zone_table->addZone(mem_sgmt_, zclass_, Name("example.org"),
+                                     holder6.release()),
                  std::bad_alloc);
 }
 
 TEST_F(ZoneTableTest, findZone) {
-    const ZoneTable::AddResult add_result1 =
-        zone_table->addZone(mem_sgmt_, zclass_, zname1);
-    EXPECT_EQ(result::SUCCESS, add_result1.code);
+    SegmentObjectHolder<ZoneData, RRClass> holder1(
+        mem_sgmt_, ZoneData::create(mem_sgmt_, zname1), zclass_);
+    ZoneData* zone_data = holder1.get();
+    EXPECT_EQ(result::SUCCESS, zone_table->addZone(mem_sgmt_, zclass_, zname1,
+                                                   holder1.release()).code);
+    SegmentObjectHolder<ZoneData, RRClass> holder2(
+        mem_sgmt_, ZoneData::create(mem_sgmt_, zname2), zclass_);
     EXPECT_EQ(result::SUCCESS,
-              zone_table->addZone(mem_sgmt_, zclass_, zname2).code);
+              zone_table->addZone(mem_sgmt_, zclass_, zname2,
+                                  holder2.release()).code);
+    SegmentObjectHolder<ZoneData, RRClass> holder3(
+        mem_sgmt_, ZoneData::create(mem_sgmt_, zname3), zclass_);
     EXPECT_EQ(result::SUCCESS,
-              zone_table->addZone(mem_sgmt_, zclass_, zname3).code);
+              zone_table->addZone(mem_sgmt_, zclass_, zname3,
+                                  holder3.release()).code);
 
     const ZoneTable::FindResult find_result1 =
         zone_table->findZone(Name("example.com"));
     EXPECT_EQ(result::SUCCESS, find_result1.code);
-    EXPECT_EQ(add_result1.zone_data, find_result1.zone_data);
+    EXPECT_EQ(zone_data, find_result1.zone_data);
 
     EXPECT_EQ(result::NOTFOUND,
               zone_table->findZone(Name("example.org")).code);
@@ -137,14 +182,17 @@ TEST_F(ZoneTableTest, findZone) {
     // and the code should be PARTIALMATCH.
     EXPECT_EQ(result::PARTIALMATCH,
               zone_table->findZone(Name("www.example.com")).code);
-    EXPECT_EQ(add_result1.zone_data,
+    EXPECT_EQ(zone_data,
               zone_table->findZone(Name("www.example.com")).zone_data);
 
     // make sure the partial match is indeed the longest match by adding
     // a zone with a shorter origin and query again.
+    SegmentObjectHolder<ZoneData, RRClass> holder4(
+        mem_sgmt_, ZoneData::create(mem_sgmt_, Name("com")), zclass_);
     EXPECT_EQ(result::SUCCESS, zone_table->addZone(mem_sgmt_, zclass_,
-                                                   Name("com")).code);
-    EXPECT_EQ(add_result1.zone_data,
+                                                   Name("com"),
+                                                   holder4.release()).code);
+    EXPECT_EQ(zone_data,
               zone_table->findZone(Name("www.example.com")).zone_data);
 }
 }

src/lib/datasrc/tests/sqlite3_accessor_unittest.cc

@@ -191,6 +191,11 @@ TEST_F(SQLite3AccessorTest, iterator) {
     checkRR(context, "sub.example.org.", "3600", "NS", "ns.sub.example.org.");
     checkRR(context, "ns.sub.example.org.", "3600", "A", "192.0.2.101");
     checkRR(context, "www.example.org.", "3600", "A", "192.0.2.1");
+    checkRR(context, "ns3.example.org.", "3600", "NSEC3",
+            "1 1 12 aabbccdd 2T7B4G4VSA5SMI47K61MV5BV1A22BOJR A RRSIG");
+    checkRR(context, "ns3.example.org.", "3600", "RRSIG",
+            "NSEC3 5 3 3600 20000101000000 20000201000000 "
+            "12345 ns3.example.org. FAKEFAKEFAKE");
 
     // Check there's no other
     EXPECT_FALSE(context->getNext(data));
@@ -665,16 +670,16 @@ TEST_F(SQLite3Create, creationtest) {
 TEST_F(SQLite3Create, emptytest) {
     ASSERT_FALSE(isReadable(SQLITE_NEW_DBFILE));
 
-    // open one manualle
+    // open one manually
     sqlite3* db;
     ASSERT_EQ(SQLITE_OK, sqlite3_open(SQLITE_NEW_DBFILE, &db));
 
-    // empty, but not locked, so creating it now should work
+    // empty, but not locked, so creating another accessor should work
     SQLite3Accessor accessor2(SQLITE_NEW_DBFILE, "IN");
 
     sqlite3_close(db);
 
-    // should work now that we closed it
+    // should still work now that we closed it
     SQLite3Accessor accessor3(SQLITE_NEW_DBFILE, "IN");
 }
 
@@ -692,8 +697,10 @@ TEST_F(SQLite3Create, lockedtest) {
 
     sqlite3_exec(db, "ROLLBACK TRANSACTION", NULL, NULL, NULL);
 
-    // should work now that we closed it
+    // should work now that the transaction has been rolled back
     SQLite3Accessor accessor3(SQLITE_NEW_DBFILE, "IN");
+
+    ASSERT_EQ(SQLITE_OK, sqlite3_close(db));
 }
 
 TEST_F(SQLite3AccessorTest, clone) {

src/lib/dhcp/tests/.gitignore

@@ -1 +1,2 @@
 /libdhcp++_unittests
+/libdhcpsrv_unittests

src/lib/dhcp/tests/iface_mgr_unittest.cc

@@ -1096,7 +1096,7 @@ void parse_ifconfig(const std::string& textFile, IfaceMgr::IfaceCollection& ifac
             addr = addr.substr(0, addr.find_first_of(" "));
             IOAddress a(addr);
             iface->addAddress(a);
-        } else if(line.find("Metric")) {
+        } else if(line.find("Metric") != string::npos) {
             // flags
             if (line.find("UP") != string::npos) {
                 iface->flag_up_ = true;

src/lib/dns/Makefile.am

@@ -149,11 +149,11 @@ libdns___include_HEADERS = \
 	messagerenderer.h \
 	name.h \
 	question.h \
+	opcode.h \
 	rcode.h \
 	rdata.h \
 	rrparamregistry.h \
 	rrset.h \
-	rrsetlist.h \
 	rrttl.h \
 	tsigkey.h
 # Purposely not installing these headers:

src/lib/dns/benchmarks/message_renderer_bench.cc

@@ -39,7 +39,7 @@ public:
         renderer_(NULL),
         names_(names)
     {}
-    MessageRendererBenchMark() {
+    ~MessageRendererBenchMark() {
         delete renderer_;
     }
     unsigned int run() {

src/lib/dns/benchmarks/rdatarender_bench.cc

@@ -42,9 +42,10 @@ template <typename T>
 class RdataRenderBenchMark {
 public:
     RdataRenderBenchMark(const vector<T>& dataset) :
-        dataset_(dataset)
+        dataset_(dataset),
+        renderer_(NULL)
     {}
-    RdataRenderBenchMark() {
+    ~RdataRenderBenchMark() {
         delete renderer_;
     }
     unsigned int run() {

src/lib/dns/python/edns_python.cc

@@ -269,7 +269,6 @@ EDNS_createFromRR(const s_EDNS* null_self, PyObject* args) {
     const PyObject* rrtype;
     const PyObject* rrttl;
     const PyObject* rdata;
-    s_EDNS* edns_obj = NULL;
 
     assert(null_self == NULL);
 
@@ -277,7 +276,7 @@ EDNS_createFromRR(const s_EDNS* null_self, PyObject* args) {
                          &rrclass_type, &rrclass, &rrtype_type, &rrtype,
                          &rrttl_type, &rrttl, &rdata_type, &rdata)) {
         uint8_t extended_rcode;
-        edns_obj = PyObject_New(s_EDNS, &edns_type);
+        s_EDNS* edns_obj = PyObject_New(s_EDNS, &edns_type);
         if (edns_obj == NULL) {
             return (NULL);
         }

src/lib/dns/rdata/generic/afsdb_18.cc

@@ -57,7 +57,6 @@ AFSDB::AFSDB(const std::string& afsdb_str) :
     try {
         const uint32_t subtype = tokenToNum<int32_t, 16>(getToken(iss));
         const Name servername(getToken(iss));
-        string server;
 
         if (!iss.eof()) {
             isc_throw(InvalidRdataText, "Unexpected input for AFSDB"

src/lib/dns/tests/labelsequence_unittest.cc

@@ -860,8 +860,12 @@ TEST_F(LabelSequenceTest, badDeserialize) {
     const uint8_t toomany_offsets[] = { Name::MAX_LABELS + 1 };
     EXPECT_THROW(LabelSequence ls(toomany_offsets), isc::BadValue);
 
-    // exceed MAX_LABEL_LEN
-    const uint8_t offsets_toolonglabel[] = { 2, 0, 64 };
+    // (second) offset does not match actual label length
+    const uint8_t offsets_wrongoffset[] = { 2, 0, 64, 1 };
+    EXPECT_THROW(LabelSequence ls(offsets_wrongoffset), isc::BadValue);
+
+    // offset matches, but exceeds MAX_LABEL_LEN
+    const uint8_t offsets_toolonglabel[] = { 2, 0, 64, 64 };
     EXPECT_THROW(LabelSequence ls(offsets_toolonglabel), isc::BadValue);
 
     // Inconsistent data: an offset is lower than the previous offset

src/lib/dns/tests/rrparamregistry_unittest.cc

@@ -37,6 +37,7 @@ protected:
     {
         ostringstream oss1;
         oss1 << test_class_code;
+        // cppcheck-suppress useInitializationList
         test_class_unknown_str = "CLASS" + oss1.str();
 
         ostringstream oss2;
@@ -60,7 +61,7 @@ protected:
 
     // we assume class/type numbers are officially unassigned.  If not we'll
     // need to update the test cases.
-    static const uint16_t test_class_code = 65533; 
+    static const uint16_t test_class_code = 65533;
     static const uint16_t test_type_code = 65534;
     static const string test_class_str;
     static const string test_type_str;
@@ -77,7 +78,7 @@ TEST_F(RRParamRegistryTest, addRemove) {
 
     // the first removal attempt should succeed
     EXPECT_TRUE(RRParamRegistry::getRegistry().removeType(test_type_code));
-    // then toText() should treat it as an "unknown" 
+    // then toText() should treat it as an "unknown"
     EXPECT_EQ(test_type_unknown_str, RRType(test_type_code).toText());
     // attempt of removing non-existent mapping should result in 'false'
     EXPECT_FALSE(RRParamRegistry::getRegistry().removeType(test_type_code));

src/lib/log/compiler/message.cc

@@ -109,11 +109,13 @@ currentTime() {
     // Get a text representation of the current time.
     time_t curtime;
     time(&curtime);
-    char* buffer = ctime(&curtime);
+    struct tm* timeinfo;
+    timeinfo = localtime(&curtime);
 
-    // Convert to string and strip out the trailing newline
-    string current_time = buffer;
-    return (isc::util::str::trim(current_time));
+    char buffer[80];
+    strftime(buffer, 80, "%a %b %d %Y %H:%M", timeinfo);
+
+    return (std::string(buffer));
 }
 
 

src/lib/log/tests/logger_example.cc

@@ -118,7 +118,6 @@ int main(int argc, char** argv) {
     LoggerSpecification cur_spec(ROOT_NAME);// Current specification
     OutputOption        cur_opt;            // Current output option
     vector<LoggerSpecification> loggers;    // Set of logger specifications
-    vector<OutputOption>        options;    // Output options for logger
     std::string                 severity;   // Severity set for logger
 
     // Initialize logging system - set the root logger name.

src/lib/python/isc/bind10/component.py

@@ -45,6 +45,7 @@ COMPONENT_RESTART_DELAY = 10
 
 STATE_DEAD = 'dead'
 STATE_STOPPED = 'stopped'
+STATE_RESTARTING = 'restarting'
 STATE_RUNNING = 'running'
 
 def get_signame(signal_number):
@@ -68,6 +69,7 @@ class BaseComponent:
       explicitly).
     - Running - after start() was called, it started successfully and is
       now running.
+    - Restarting - the component failed (crashed) and is waiting for a restart
     - Dead - it failed and can not be resurrected.
 
     Init
@@ -79,11 +81,11 @@ class BaseComponent:
                     |            |                |
                     |failure     | failed()       |
                     |            |                |
-                    v            |                |
+                    v            |                | start()/restart()
                     +<-----------+                |
                     |                             |
                     |  kind == dispensable or kind|== needed and failed late
-                    +-----------------------------+
+                    +-----------------------> Restarting
                     |
                     | kind == core or kind == needed and it failed too soon
                     v
@@ -163,7 +165,7 @@ class BaseComponent:
         """
         if self.__state == STATE_DEAD:
             raise ValueError("Can't resurrect already dead component")
-        if self.running():
+        if self.is_running():
             raise ValueError("Can't start already running component")
         logger.info(BIND10_COMPONENT_START, self.name())
         self.__state = STATE_RUNNING
@@ -188,7 +190,7 @@ class BaseComponent:
         """
         # This is not tested. It talks with the outher world, which is out
         # of scope of unittests.
-        if not self.running():
+        if not self.is_running():
             raise ValueError("Can't stop a component which is not running")
         logger.info(BIND10_COMPONENT_STOP, self.name())
         self.__state = STATE_STOPPED
@@ -234,9 +236,9 @@ class BaseComponent:
 
         logger.error(BIND10_COMPONENT_FAILED, self.name(), self.pid(),
                      exit_str)
-        if not self.running():
+        if not self.is_running():
             raise ValueError("Can't fail component that isn't running")
-        self.__state = STATE_STOPPED
+        self.__state = STATE_RESTARTING # tentatively set, maybe changed to DEAD
         self._failed_internal()
         # If it is a core component or the needed component failed to start
         # (including it stopped really soon)
@@ -284,7 +286,7 @@ class BaseComponent:
         else:
             return False
 
-    def running(self):
+    def is_running(self):
         """
         Informs if the component is currently running. It assumes the failed
         is called whenever the component really fails and there might be some
@@ -296,6 +298,15 @@ class BaseComponent:
         """
         return self.__state == STATE_RUNNING
 
+    def is_restarting(self):
+        """Informs if the component has failed and is waiting for a restart.
+
+        Unlike the case of is_running(), if this returns True it always means
+        the corresponding process has died and not yet restarted.
+
+        """
+        return self.__state == STATE_RESTARTING
+
     def _start_internal(self):
         """
         This method does the actual starting of a process. You need to override
@@ -560,6 +571,13 @@ class Configurator:
         self._running = False
         self.__reconfigure_internal(self._components, {})
 
+    def has_component(self, component):
+        '''Return if a specified component is configured.'''
+        # Values of self._components are tuples of (config, component).
+        # Extract the components of the tuples and see if the given one
+        # is included.
+        return component in map(lambda x: x[1], self._components.values())
+
     def reconfigure(self, configuration):
         """
         Changes configuration from the current one to the provided. It
@@ -591,7 +609,7 @@ class Configurator:
         for cname in old.keys():
             if cname not in new:
                 component = self._components[cname][1]
-                if component.running():
+                if component.is_running() or component.is_restarting():
                     plan.append({
                         'command': STOP_CMD,
                         'component': component,
@@ -674,7 +692,7 @@ class Configurator:
                     self._components[task['name']] = (task['config'],
                                                       component)
                 elif command == STOP_CMD:
-                    if component.running():
+                    if component.is_running():
                         component.stop()
                     del self._components[task['name']]
                 else:

src/lib/python/isc/bind10/tests/component_test.py

@@ -191,7 +191,8 @@ class ComponentTests(BossUtils, unittest.TestCase):
         self.assertFalse(self.__start_called)
         self.assertFalse(self.__stop_called)
         self.assertFalse(self.__failed_called)
-        self.assertFalse(component.running())
+        self.assertFalse(component.is_running())
+        self.assertFalse(component.is_restarting())
         # We can't stop or fail the component yet
         self.assertRaises(ValueError, component.stop)
         self.assertRaises(ValueError, component.failed, 1)
@@ -204,7 +205,8 @@ class ComponentTests(BossUtils, unittest.TestCase):
         self.assertTrue(self.__start_called)
         self.assertFalse(self.__stop_called)
         self.assertFalse(self.__failed_called)
-        self.assertTrue(component.running())
+        self.assertTrue(component.is_running())
+        self.assertFalse(component.is_restarting())
 
     def __check_dead(self, component):
         """
@@ -215,7 +217,8 @@ class ComponentTests(BossUtils, unittest.TestCase):
         self.assertFalse(self.__stop_called)
         self.assertTrue(self.__failed_called)
         self.assertEqual(1, self._exitcode)
-        self.assertFalse(component.running())
+        self.assertFalse(component.is_running())
+        self.assertFalse(component.is_restarting())
         # Surely it can't be stopped when already dead
         self.assertRaises(ValueError, component.stop)
         # Nor started
@@ -234,7 +237,8 @@ class ComponentTests(BossUtils, unittest.TestCase):
         self.assertTrue(self.__start_called)
         self.assertFalse(self.__stop_called)
         self.assertTrue(self.__failed_called)
-        self.assertTrue(component.running())
+        self.assertTrue(component.is_running())
+        self.assertFalse(component.is_restarting())
         # Check it can't be started again
         self.assertRaises(ValueError, component.start)
 
@@ -246,7 +250,8 @@ class ComponentTests(BossUtils, unittest.TestCase):
         self.assertTrue(self.__start_called)
         self.assertFalse(self.__stop_called)
         self.assertTrue(self.__failed_called)
-        self.assertFalse(component.running())
+        self.assertFalse(component.is_running())
+        self.assertTrue(component.is_restarting())
 
     def __do_start_stop(self, kind):
         """
@@ -270,7 +275,8 @@ class ComponentTests(BossUtils, unittest.TestCase):
         self.assertTrue(self.__start_called)
         self.assertTrue(self.__stop_called)
         self.assertFalse(self.__failed_called)
-        self.assertFalse(component.running())
+        self.assertFalse(component.is_running())
+        self.assertFalse(component.is_restarting())
         # Check it can't be stopped twice
         self.assertRaises(ValueError, component.stop)
         # Or failed
@@ -553,10 +559,10 @@ class ComponentTests(BossUtils, unittest.TestCase):
         self.assertIsNone(component.pid())
         self.assertEqual(['hello'], component._params)
         self.assertEqual('Address', component._address)
-        self.assertFalse(component.running())
+        self.assertFalse(component.is_running())
         self.assertEqual({}, self.__registered_processes)
         component.start()
-        self.assertTrue(component.running())
+        self.assertTrue(component.is_running())
         # Some versions of unittest miss assertIsInstance
         self.assertTrue(isinstance(component._procinfo, TestProcInfo))
         self.assertEqual(42, component.pid())
@@ -580,11 +586,11 @@ class ComponentTests(BossUtils, unittest.TestCase):
         """
         component = Component('component', self, 'needed', 'Address')
         component.start()
-        self.assertTrue(component.running())
+        self.assertTrue(component.is_running())
         self.assertEqual('component', self.__start_simple_params)
         component.pid = lambda: 42
         component.stop()
-        self.assertFalse(component.running())
+        self.assertFalse(component.is_running())
         self.assertEqual(('component', 'Address', 42),
                          self.__stop_process_params)
 
@@ -609,7 +615,7 @@ class ComponentTests(BossUtils, unittest.TestCase):
         component = Component('component', self, 'needed', 'Address',
                               [], ProcInfo)
         component.start()
-        self.assertTrue(component.running())
+        self.assertTrue(component.is_running())
         component.kill()
         self.assertTrue(process.terminated)
         self.assertFalse(process.killed)
@@ -872,12 +878,13 @@ class ConfiguratorTest(BossUtils, unittest.TestCase):
             'priority': 6,
             'special': 'test',
             'process': 'additional',
-            'kind': 'needed'
+            'kind': 'dispensable' # need to be dispensable so it could restart
         }
         self.log = []
         plan = configurator._build_plan(self.__build_components(self.__core),
                                         bigger)
-        self.assertEqual([('additional', 'init'), ('additional', 'needed')],
+        self.assertEqual([('additional', 'init'),
+                          ('additional', 'dispensable')],
                          self.log)
         self.assertEqual(1, len(plan))
         self.assertTrue('component' in plan[0])
@@ -888,15 +895,29 @@ class ConfiguratorTest(BossUtils, unittest.TestCase):
         # Now remove the one component again
         # We run the plan so the component is wired into internal structures
         configurator._run_plan(plan)
-        self.log = []
-        plan = configurator._build_plan(self.__build_components(bigger),
-                                        self.__core)
-        self.assertEqual([], self.log)
-        self.assertEqual([{
-            'command': 'stop',
-            'name': 'additional',
-            'component': component
-        }], plan)
+        # We should have the 'additional' component in the configurator.
+        self.assertTrue(configurator.has_component(component))
+        for count in [0, 1]:    # repeat two times, see below
+            self.log = []
+            plan = configurator._build_plan(self.__build_components(bigger),
+                                            self.__core)
+            self.assertEqual([], self.log)
+            self.assertEqual([{
+                        'command': 'stop',
+                        'name': 'additional',
+                        'component': component
+                        }], plan)
+
+            if count is 0:
+                # We then emulate unexpected failure of the component (but
+                # before it restarts).  It shouldn't confuse the
+                # configurator in the second phase of the test
+                component.failed(0)
+        # Run the plan, confirm the specified component is gone.
+        configurator._run_plan(plan)
+        self.assertFalse(configurator.has_component(component))
+        # There shouldn't be any other object that has the same name
+        self.assertFalse('additional' in configurator._components)
 
         # We want to switch a component. So, prepare the configurator so it
         # holds one

src/lib/python/isc/datasrc/sqlite3_ds.py

@@ -25,7 +25,7 @@ RR_RDATA_INDEX = 7
 
 # Current major and minor versions of schema
 SCHEMA_MAJOR_VERSION = 2
-SCHEMA_MINOR_VERSION = 0
+SCHEMA_MINOR_VERSION = 1
 
 class Sqlite3DSError(Exception):
     """ Define exceptions."""
@@ -81,6 +81,7 @@ def create(cur):
                     rdtype TEXT NOT NULL COLLATE NOCASE,
                     rdata TEXT NOT NULL)""")
         cur.execute("CREATE INDEX nsec3_byhash ON nsec3 (hash)")
+        cur.execute("CREATE INDEX nsec3_byhash_and_rdtype ON nsec3 (hash, rdtype)")
         cur.execute("""CREATE TABLE diffs (id INTEGER PRIMARY KEY,
                     zone_id INTEGER NOT NULL,
                     version INTEGER NOT NULL,

src/lib/resolve/recursive_query.cc

@@ -176,7 +176,7 @@ class RunningQuery : public IOFetch::Callback {
 class ResolverNSASCallback : public isc::nsas::AddressRequestCallback {
 public:
     ResolverNSASCallback(RunningQuery* rq) : rq_(rq) {}
-    
+
     void success(const isc::nsas::NameserverAddress& address) {
         // Success callback, send query to found namesever
         LOG_DEBUG(isc::resolve::logger, RESLIB_DBG_CB, RESLIB_RUNQ_SUCCESS)
@@ -184,7 +184,7 @@ public:
         rq_->nsasCallbackCalled();
         rq_->sendTo(address);
     }
-    
+
     void unreachable() {
         // Nameservers unreachable: drop query or send servfail?
         LOG_DEBUG(isc::resolve::logger, RESLIB_DBG_CB, RESLIB_RUNQ_FAIL);
@@ -261,7 +261,7 @@ private:
     bool done_;
 
     // If we have a client timeout, we call back with a failure message,
-    // but we do not stop yet. We use this variable to make sure we 
+    // but we do not stop yet. We use this variable to make sure we
     // don't call back a second time later
     bool callback_called_;
 
@@ -270,7 +270,7 @@ private:
 
     // Reference to our cache
     isc::cache::ResolverCache& cache_;
-    
+
     // the 'current' zone we are in (i.e.) we start out at the root,
     // and for each delegation this gets updated with the zone the
     // delegation points to.
@@ -278,7 +278,7 @@ private:
     // of the call we use it in take a string, we need update those
     // too).
     std::string cur_zone_;
-    
+
     // This is the handler we pass on to the NSAS; it is called when
     // the NSAS has an address for us to query
     boost::shared_ptr<ResolverNSASCallback> nsas_callback_;
@@ -295,7 +295,7 @@ private:
 
     // The moment in time we sent a query to the nameserver above.
     struct timeval current_ns_qsent_time;
-    
+
     // RunningQuery deletes itself when it is done. In order for us
     // to do this safely, we must make sure that there are no events
     // that might call back to it. There are two types of events in
@@ -365,7 +365,7 @@ private:
             io_.get_io_service().post(query);
         }
     }
-    
+
     // 'general' send, ask the NSAS to give us an address.
     void send(IOFetch::Protocol protocol = IOFetch::UDP, bool edns = true) {
         protocol_ = protocol;   // Store protocol being used for this
@@ -397,7 +397,7 @@ private:
             nsas_.lookup(cur_zone_, question_.getClass(), nsas_callback_);
         }
     }
-    
+
     // Called by our NSAS callback handler so we know we do not have
     // an outstanding NSAS call anymore.
     void nsasCallbackCalled() {
@@ -422,13 +422,13 @@ private:
         // here (classify() will set it when it walks through
         // the cname chain to verify it).
         Name cname_target(question_.getName());
-        
+
         isc::resolve::ResponseClassifier::Category category =
             isc::resolve::ResponseClassifier::classify(
                 question_, incoming, cname_target, cname_count_);
 
         bool found_ns = false;
-            
+
         switch (category) {
         case isc::resolve::ResponseClassifier::ANSWER:
         case isc::resolve::ResponseClassifier::ANSWERCNAME:
@@ -569,7 +569,7 @@ private:
                 // SERVFAIL if we get FORMERR instead
             }
             goto SERVFAIL;
-            
+
         default:
 SERVFAIL:
             // Some error in received packet it.  Report it and return SERVFAIL
@@ -718,7 +718,7 @@ public:
             ++outstanding_events_;
             lookup_timer.async_wait(boost::bind(&RunningQuery::lookupTimeout, this));
         }
-        
+
         // Setup the timer to send an answer (client_timeout)
         if (client_timeout >= 0) {
             client_timer.expires_from_now(
@@ -726,7 +726,7 @@ public:
             ++outstanding_events_;
             client_timer.async_wait(boost::bind(&RunningQuery::clientTimeout, this));
         }
-        
+
         doLookup();
     }
 
@@ -741,7 +741,7 @@ public:
         --outstanding_events_;
         stop();
     }
-    
+
     // called if we have a client timeout; if our callback has
     // not been called, call it now. But do not stop.
     void clientTimeout() {
@@ -810,7 +810,7 @@ public:
         // XXX is this the place for TCP retry?
         assert(outstanding_events_ > 0);
         --outstanding_events_;
-        
+
         if (!done_ && result != IOFetch::TIME_OUT) {
             // we got an answer
 
@@ -890,7 +890,7 @@ public:
             stop();
         }
     }
-    
+
     // Clear the answer parts of answer_message, and set the rcode
     // to servfail
     void makeSERVFAIL() {
@@ -1096,7 +1096,7 @@ RecursiveQuery::resolve(const QuestionPtr& question,
         // Message found, return that
         LOG_DEBUG(isc::resolve::logger, RESLIB_DBG_CACHE, RESLIB_RECQ_CACHE_FIND)
                   .arg(questionText(*question)).arg(1);
-        
+
         // TODO: err, should cache set rcode as well?
         answer_message->setRcode(Rcode::NOERROR());
         callback->success(answer_message);
@@ -1146,11 +1146,11 @@ RecursiveQuery::resolve(const Question& question,
     // TODO: general 'prepareinitialanswer'
     answer_message->setOpcode(isc::dns::Opcode::QUERY());
     answer_message->addQuestion(question);
-    
+
     // First try to see if we have something cached in the messagecache
     LOG_DEBUG(isc::resolve::logger, RESLIB_DBG_TRACE, RESLIB_RESOLVE)
               .arg(questionText(question)).arg(2);
-    
+
     if (cache_.lookup(question.getName(), question.getType(),
                       question.getClass(), *answer_message) &&
         answer_message->getRRCount(Message::SECTION_ANSWER) > 0) {
@@ -1181,7 +1181,7 @@ RecursiveQuery::resolve(const Question& question,
             // delete itself when it is done
             LOG_DEBUG(isc::resolve::logger, RESLIB_DBG_TRACE, RESLIB_RECQ_CACHE_NO_FIND)
                       .arg(questionText(question)).arg(2);
-            new RunningQuery(io, question, answer_message, 
+            new RunningQuery(io, question, answer_message,
                              test_server_, buffer, crs, query_timeout_,
                              client_timeout_, lookup_timeout_, retries_,
                              nsas_, cache_, rtt_recorder_);

src/lib/resolve/tests/recursive_query_unittest.cc

@@ -218,7 +218,7 @@ protected:
     }
 
     // Receive a UDP packet from a mock server; used for testing
-    // recursive lookup.  The caller must place a RecursiveQuery 
+    // recursive lookup.  The caller must place a RecursiveQuery
     // on the IO Service queue before running this routine.
     void recvUDP(const int family, void* buffer, size_t& size) {
         ScopedAddrInfo sai(resolveAddress(family, IPPROTO_UDP, true));
@@ -267,7 +267,7 @@ protected:
         if (ret < 0) {
             isc_throw(IOError, "recvfrom failed: " << strerror(errno));
         }
-        
+
         // Pass the message size back via the size parameter
         size = ret;
     }
@@ -693,37 +693,6 @@ createTestSocket() {
     return (sock.release());
 }
 
-int
-setSocketTimeout(int sock, size_t tv_sec, size_t tv_usec) {
-    const struct timeval timeo = { tv_sec, tv_usec };
-    int recv_options = 0;
-    if (setsockopt(sock, SOL_SOCKET, SO_RCVTIMEO, &timeo, sizeof(timeo))) {
-        if (errno == ENOPROTOOPT) { // see RecursiveQueryTest::recvUDP()
-            recv_options = MSG_DONTWAIT;
-        } else {
-            isc_throw(IOError, "set RCVTIMEO failed: " << strerror(errno));
-        }
-    }
-    return (recv_options);
-}
-
-// try to read from the socket max time
-// *num is incremented for every succesfull read
-// returns true if it can read max times, false otherwise
-bool tryRead(int sock, int recv_options, size_t max, int* num) {
-    size_t i = 0;
-    do {
-        char inbuff[512];
-        if (recv(sock, inbuff, sizeof(inbuff), recv_options) < 0) {
-            return false;
-        } else {
-            ++i;
-            ++*num;
-        }
-    } while (i < max);
-    return true;
-}
-
 // Mock resolver callback for testing forward query.
 class MockResolverCallback : public isc::resolve::ResolverInterface::Callback {
 public:
@@ -904,7 +873,7 @@ TEST_F(RecursiveQueryTest, lowtimeouts) {
 TEST_F(RecursiveQueryTest, DISABLED_recursiveSendOk) {
     setDNSService(true, false);
     bool done;
-    
+
     MockServerStop server(io_service_, &done);
     vector<pair<string, uint16_t> > empty_vector;
     RecursiveQuery rq(*dns_service_, *nsas_, cache_, empty_vector,
@@ -930,7 +899,7 @@ TEST_F(RecursiveQueryTest, DISABLED_recursiveSendOk) {
 TEST_F(RecursiveQueryTest, DISABLED_recursiveSendNXDOMAIN) {
     setDNSService(true, false);
     bool done;
-    
+
     MockServerStop server(io_service_, &done);
     vector<pair<string, uint16_t> > empty_vector;
     RecursiveQuery rq(*dns_service_, *nsas_, cache_, empty_vector,

src/lib/resolve/tests/recursive_query_unittest_2.cc

@@ -76,14 +76,17 @@ using namespace std;
 /// directed to one or other of the "servers" in the RecursiveQueryTest2 class,
 /// regardless of the glue returned in referrals.
 
+namespace {
+const char* const TEST_ADDRESS = "127.0.0.1"; ///< Servers are on this address
+const uint16_t TEST_PORT = 5301;              ///< ... and this port
+const size_t BUFFER_SIZE = 1024;              ///< For all buffers
+const char* const WWW_EXAMPLE_ORG = "192.0.2.254";
+                                              ///< Address of www.example.org
+} //end anonymous namespace
+
 namespace isc {
 namespace asiodns {
 
-const std::string TEST_ADDRESS = "127.0.0.1";   ///< Servers are on this address
-const uint16_t TEST_PORT = 5301;                ///< ... and this port
-const size_t BUFFER_SIZE = 1024;                ///< For all buffers
-const char* WWW_EXAMPLE_ORG = "192.0.2.254";    ///< Address of www.example.org
-
 // As the test is fairly long and complex, debugging "print" statements have
 // been left in although they are disabled.  Set the following to "true" to
 // enable them.

src/lib/resolve/tests/recursive_query_unittest_3.cc

@@ -69,15 +69,16 @@ using namespace std;
 /// By using the "test_server_" element of RecursiveQuery, all queries are
 /// directed to one or other of the "servers" in the RecursiveQueryTest3 class.
 
-namespace isc {
-namespace asiodns {
-
-const std::string TEST_ADDRESS3 = "127.0.0.1"; 
-                                               ///< Servers are on this address
+namespace {
+const char* const TEST_ADDRESS3 = "127.0.0.1"; ///< Servers are on this address
 const uint16_t TEST_PORT3 = 5303;              ///< ... and this port
-const size_t BUFFER_SIZE = 1024;              ///< For all buffers
+const size_t BUFFER_SIZE = 1024;               ///< For all buffers
 
-const std::string DUMMY_ADDR3 = "1.2.3.4";     ///< address to return as A
+const char* const DUMMY_ADDR3 = "1.2.3.4";     ///< address to return as A
+} // end anonymous namespace
+
+namespace isc {
+namespace asiodns {
 
 class MockResolver3 : public isc::resolve::ResolverInterface {
 public:

src/lib/server_common/tests/socket_requestor_test.cc

@@ -410,7 +410,8 @@ private:
             isc_throw(Unexpected,
                       "mkstemp() created a filename too long for sun_path");
         }
-        strncpy(socket_address.sun_path, path_, len);
+        strncpy(socket_address.sun_path, path_,
+                sizeof(socket_address.sun_path));
 #ifdef HAVE_SA_LEN
         socket_address.sun_len = len;
 #endif
@@ -542,7 +543,6 @@ TEST_F(SocketRequestorTest, testSocketPassing) {
         EXPECT_EQ("foo", socket_id.second);
         EXPECT_EQ(0, close(socket_id.first));
     }
-
     // Create a second socket server, to test that multiple different
     // domains sockets would work as well (even though we don't actually
     // use that feature)

src/lib/util/Makefile.am

@@ -33,3 +33,6 @@ EXTRA_DIST = python/pycppwrapper_util.h
 
 libb10_util_la_LIBADD = $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
 CLEANFILES = *.gcno *.gcda
+
+libb10_util_includedir = $(includedir)/$(PACKAGE_NAME)/util
+libb10_util_include_HEADERS = buffer.h

src/lib/util/hash/sha1.cc

@@ -48,7 +48,7 @@
  *      merchantability of this software or the suitability of this
  *      software for any particular purpose.  It is provided "as is"
  *      without express or implied warranty of any kind.
- *      
+ *
  */
 #include <util/hash/sha1.h>
 
@@ -79,7 +79,7 @@ SHA_Parity(const uint32_t x, const uint32_t y, const uint32_t z) {
     return ((x) ^ (y) ^ (z));
 }
 
-static inline int 
+static inline int
 SHA1CircularShift(uint8_t bits, uint32_t word) {
     return ((word << bits) | (word >> (32 - bits)));
 }

src/lib/util/io/fd_share.cc

@@ -142,6 +142,7 @@ send_fd(const int sock, const int fd) {
     if (msghdr.msg_control == NULL) {
         return (FD_OTHER_ERROR);
     }
+    std::memset(msghdr.msg_control, 0, msghdr.msg_controllen);
 
     struct cmsghdr* cmsg = CMSG_FIRSTHDR(&msghdr);
     cmsg->cmsg_len = cmsg_len(sizeof(int));

src/lib/util/tests/fd_tests.cc

@@ -34,7 +34,9 @@ class FDTest : public ::testing::Test {
             // We do not care what is inside, we just need it to be the same
             data(new unsigned char[TEST_DATA_SIZE]),
             buffer(NULL)
-        { }
+        {
+            memset(data, 0, TEST_DATA_SIZE);
+        }
         ~ FDTest() {
             delete[] data;
             delete[] buffer;

src/lib/util/tests/lru_list_unittest.cc

@@ -168,7 +168,7 @@ protected:
         entry7_(new TestEntry("eta", 1))
     {}
 
-    virtual ~LruListTest() 
+    virtual ~LruListTest()
     {}
 
     boost::shared_ptr<TestEntry>    entry1_;
@@ -355,7 +355,7 @@ TEST_F(LruListTest, Dropped) {
     lru.add(entry5_);
     EXPECT_NE(0, (entry2_->getCode() & 0x8000));
 
-    // Delete an entry and check that the handler does not run. 
+    // Delete an entry and check that the handler does not run.
     EXPECT_EQ(0, (entry3_->getCode() & 0x8000));
     lru.remove(entry3_);
     EXPECT_EQ(0, (entry3_->getCode() & 0x8000));
@@ -386,7 +386,7 @@ TEST_F(LruListTest, Clear) {
     EXPECT_NE(0, (entry1_->getCode() & 0x8000));
     EXPECT_NE(0, (entry2_->getCode() & 0x8000));
     EXPECT_NE(0, (entry3_->getCode() & 0x8000));
- 
+
     EXPECT_EQ(0, lru.size());
 }
 

src/lib/util/threads/tests/.gitignore

@@ -0,0 +1 @@
+/run_unittests

tests/tools/perfdhcp/templates/.gitignore

@@ -0,0 +1,5 @@
+/test1.hex
+/test2.hex
+/test3.hex
+/test4.hex
+/test5.hex