Merge branch 'master' into work/pylog

ChangeLog

@@ -1,20 +1,58 @@
-249.    [func]      jerry
+256.	[bug]		jerry
+	src/bin/xfrin: update xfrin to check TSIG before other part of
+	incoming message.
+	(Trac955, git 261450e93af0b0406178e9ef121f81e721e0855c)
+
+255.	[func]		zhang likun
+	src/lib/cache:  remove empty code in lib/cache and the corresponding
+	suppression rule in	src/cppcheck-suppress.lst.
+	(Trac639, git 4f714bac4547d0a025afd314c309ca5cb603e212)
+
+254.	[bug]		jinmei
+	b10-xfrout: failed to send notifies over IPv6 correctly.
+	(Trac964, git 3255c92714737bb461fb67012376788530f16e40)
+
+253.    [func]		jelte
+	Add configuration options for logging through the virtual module
+	Logging.
+	(Trac 736, git 9fa2a95177265905408c51d13c96e752b14a0824)
+
+252.    [func]      	stephen
+	Add syslog as destination for logging.
+	(Trac976, git 31a30f5485859fd3df2839fc309d836e3206546e)
+
+251.	[bug]*		jinmei
+	Make sure bindctl private files are non readable to anyone except
+	the owner or users in the same group.  Note that if BIND 10 is run
+	with changing the user, this change means that the file owner or
+	group will have to be adjusted.  Also note that this change is
+	only effective for a fresh install; if these files already exist,
+	their permissions must be adjusted by hand (if necessary).
+	(Trac870, git 461fc3cb6ebabc9f3fa5213749956467a14ebfd4)
+
+250.    [bug]           ocean
+	src/lib/util/encode, in some conditions, the DecodeNormalizer's
+	iterator may reach the end() and when later being dereferenced
+	it will cause crash on some platform.
+	(Trac838, git 83e33ec80c0c6485d8b116b13045b3488071770f)
+
+249.    [func]      	jerry
 	xfrout: add support for TSIG verification.
 	(Trac816, git 3b2040e2af2f8139c1c319a2cbc429035d93f217)
 
-248.    [func]      stephen
+248.    [func]      	stephen
 	Add file and stderr as destinations for logging.
 	(Trac555, git 38b3546867425bd64dbc5920111a843a3330646b)
 
-247.    [func]      jelte
+247.    [func]      	jelte
 	Upstream queries from the resolver now set EDNS0 buffer size.
 	(Trac834, git 48e10c2530fe52c9bde6197db07674a851aa0f5d)
 
-246.    [func]      stephen
+246.    [func]      	stephen
 	Implement logging using log4cplus (http://log4cplus.sourceforge.net)
 	(Trac899, git 31d3f525dc01638aecae460cb4bc2040c9e4df10)
 
-245.    [func]      vorner
+245.    [func]      	vorner
 	Authoritative server can now sign the answers using TSIG
 	(configured in tsig_keys/keys, list of strings like
 	"name:<base64-secret>:sha1-hmac"). It doesn't use them for

configure.ac

@@ -827,6 +827,8 @@ AC_CONFIG_FILES([Makefile
                  src/lib/util/unittests/Makefile
                  src/lib/util/pyunittests/Makefile
                  src/lib/util/tests/Makefile
+                 src/lib/acl/Makefile
+                 src/lib/acl/tests/Makefile
                  tests/Makefile
                  tests/system/Makefile
                  tests/tools/Makefile
@@ -891,6 +893,7 @@ AC_OUTPUT([doc/version.ent
            src/lib/cc/session_config.h.pre
            src/lib/cc/tests/session_unittests_config.h
            src/lib/log/tests/console_test.sh
+           src/lib/log/tests/destination_test.sh
            src/lib/log/tests/local_file_test.sh
            src/lib/log/tests/severity_test.sh
            src/lib/log/tests/tempdir.h
@@ -922,6 +925,7 @@ AC_OUTPUT([doc/version.ent
            chmod +x src/lib/dns/tests/testdata/gen-wiredata.py
            chmod +x src/lib/log/tests/local_file_test.sh
            chmod +x src/lib/log/tests/console_test.sh
+           chmod +x src/lib/log/tests/destination_test.sh
            chmod +x src/lib/log/tests/severity_test.sh
            chmod +x src/lib/util/python/mkpywrapper.py
            chmod +x src/lib/python/isc/log/tests/log_console.py

doc/Doxyfile

@@ -573,8 +573,8 @@ INPUT                  = ../src/lib/cc ../src/lib/config \
     ../src/bin/auth ../src/bin/resolver ../src/lib/bench \
     ../src/lib/log ../src/lib/asiolink/ ../src/lib/nsas \
     ../src/lib/testutils ../src/lib/cache ../src/lib/server_common/ \
-    ../src/bin/sockcreator/ ../src/lib/util/
-    ../src/lib/resolve
+    ../src/bin/sockcreator/ ../src/lib/util/ \
+    ../src/lib/resolve ../src/lib/acl
 
 # This tag can be used to specify the character encoding of the source files
 # that doxygen parses. Internally doxygen uses the UTF-8 encoding, which is

src/bin/auth/tests/run_unittests.cc

@@ -13,6 +13,7 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <gtest/gtest.h>
+#include <log/logger_support.h>
 #include <util/unittests/run_all.h>
 
 #include <dns/tests/unittest_util.h>
@@ -22,6 +23,7 @@ main(int argc, char* argv[]) {
     ::testing::InitGoogleTest(&argc, argv);
     isc::UnitTestUtil::addDataPath(TEST_DATA_DIR);
     isc::UnitTestUtil::addDataPath(TEST_DATA_BUILDDIR);
+    isc::log::initLogger();
 
     return (isc::util::unittests::run_all());
 }

src/bin/cfgmgr/plugins/Makefile.am

@@ -1,5 +1,6 @@
 SUBDIRS = tests
 EXTRA_DIST = README tsig_keys.py tsig_keys.spec
+EXTRA_DIST += logging.spec b10logging.py
 
 config_plugindir = @prefix@/share/@PACKAGE@/config_plugins
 config_plugin_DATA = tsig_keys.py tsig_keys.spec

src/bin/cfgmgr/plugins/b10logging.py

@@ -0,0 +1,96 @@
+# Copyright (C) 2011  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and 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 INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM 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.
+
+# This is the configuration plugin for logging options
+# The name is 'b10logging' because logging.py is an existing module
+#
+# For a technical background, see
+# http://bind10.isc.org/wiki/LoggingCppApiDesign
+#
+
+from isc.config.module_spec import module_spec_from_file
+from isc.util.file import path_search
+from bind10_config import PLUGIN_PATHS
+spec = module_spec_from_file(path_search('logging.spec', PLUGIN_PATHS))
+
+ALLOWED_SEVERITIES = [ 'default',
+                       'debug',
+                       'info',
+                       'warn',
+                       'error',
+                       'fatal',
+                       'none' ]
+ALLOWED_DESTINATIONS = [ 'console',
+                         'file',
+                         'syslog' ]
+ALLOWED_STREAMS = [ 'stdout',
+                    'stderr' ]
+
+def check(config):
+    # Check the data layout first
+    errors=[]
+    if not spec.validate_config(False, config, errors):
+        return ' '.join(errors)
+    # The 'layout' is ok, now check for specific values
+    if 'loggers' in config:
+        for logger in config['loggers']:
+            # name should always be present
+            name = logger['name']
+
+            if 'severity' in logger and\
+               logger['severity'].lower() not in ALLOWED_SEVERITIES:
+                errors.append("bad severity value for logger " + name +
+                              ": " + logger['severity'])
+            if 'output_options' in logger:
+                for output_option in logger['output_options']:
+                    if 'destination' in output_option:
+                        destination = output_option['destination'].lower()
+                        if destination not in ALLOWED_DESTINATIONS:
+                            errors.append("bad destination for logger " +
+                            name + ": " + output_option['destination'])
+                        else:
+                            # if left to default, output is stdout, and
+                            # it will not show in the updated config,
+                            # so 1. we only need to check it if present,
+                            # and 2. if destination is changed, so should
+                            # output. So first check checks 'in', and the
+                            # others 'not in' for 'output'
+                            if destination == "console" and\
+                               'output' in output_option and\
+                               output_option['output'] not in ALLOWED_STREAMS:
+                                errors.append("bad output for logger " + name +
+                                              ": " + output_option['stream'] +
+                                              ", must be stdout or stderr")
+                            elif destination == "file" and\
+                                 'output' not in output_option or\
+                                 output_option['output'] == "":
+                                    errors.append("destination set to file but "
+                                                  "output not set to any "
+                                                  "filename for logger "
+                                                  + name)
+                            elif destination == "syslog" and\
+                                 'output' not in output_option or\
+                                 output_option['output'] == "":
+                                    errors.append("destination set to syslog but "
+                                                  "output not set to any facility"
+                                                  " for logger " + name)
+
+    if errors:
+        return ', '.join(errors)
+    return None
+
+def load():
+    return (spec, check)
+

src/bin/cfgmgr/plugins/logging.spec

@@ -0,0 +1,81 @@
+{
+    "module_spec": {
+        "module_name": "Logging",
+        "module_description": "Logging options",
+        "config_data": [
+            {
+                "item_name": "loggers",
+                "item_type": "list",
+                "item_optional": false,
+                "item_default": [],
+                "list_item_spec": {
+                  "item_name": "logger",
+                  "item_type": "map",
+                  "item_optional": false,
+                  "item_default": {},
+                  "map_item_spec": [
+                  {  "item_name": "name",
+                     "item_type": "string",
+                     "item_optional": false,
+                     "item_default": ""
+                  },
+                  {  "item_name": "severity",
+                     "item_type": "string",
+                     "item_optional": false,
+                     "item_default": "INFO"
+                  },
+                  {  "item_name": "debuglevel",
+                     "item_type": "integer",
+                     "item_optional": false,
+                     "item_default": 0
+                  },
+                  {  "item_name": "additive",
+                     "item_type": "boolean",
+                     "item_optional": false,
+                     "item_default": false
+                  },
+                  { "item_name": "output_options",
+                    "item_type": "list",
+                    "item_optional": false,
+                    "item_default": [],
+                    "list_item_spec": {
+                      "item_name": "output_option",
+                      "item_type": "map",
+                      "item_optional": false,
+                      "item_default": {},
+                      "map_item_spec": [
+                      { "item_name": "destination",
+                        "item_type": "string",
+                        "item_optional": false,
+                        "item_default": "console"
+                      },
+                      { "item_name": "output",
+                        "item_type": "string",
+                        "item_optional": false,
+                        "item_default": "stdout"
+                      },
+                      { "item_name": "flush",
+                        "item_type": "boolean",
+                        "item_optional": false,
+                        "item_default": false
+                      },
+                      { "item_name": "maxsize",
+                        "item_type": "integer",
+                        "item_optional": false,
+                        "item_default": 0
+                      },
+                      { "item_name": "maxver",
+                        "item_type": "integer",
+                        "item_optional": false,
+                        "item_default": 0
+                      }
+                      ]
+                    }
+                  }
+                  ]
+                }
+            }
+        ],
+        "commands": []
+    }
+}

src/bin/cfgmgr/plugins/tests/tsig_keys_test.py

@@ -86,7 +86,7 @@ class TSigKeysTest(unittest.TestCase):
         self.assertEqual("TSIG: Invalid TSIG key string: invalid.key",
                          tsig_keys.check({'keys': ['invalid.key']}))
         self.assertEqual(
-            "TSIG: attempt to decode a value not in base64 char set",
+            "TSIG: Unexpected end of input in BASE decoder",
             tsig_keys.check({'keys': ['invalid.key:123']}))
 
     def test_bad_format(self):

src/bin/cmdctl/Makefile.am

@@ -40,12 +40,13 @@ b10-cmdctl: cmdctl.py
 
 if INSTALL_CONFIGURATIONS
 
-# TODO: permissions handled later
+# Below we intentionally use ${INSTALL} -m 640 instead of $(INSTALL_DATA)
+# because these file will contain sensitive information.
 install-data-local:
 	$(mkinstalldirs) $(DESTDIR)/@sysconfdir@/@PACKAGE@   
 	for f in $(CMDCTL_CONFIGURATIONS) ; do	\
 	  if test ! -f $(DESTDIR)$(sysconfdir)/@PACKAGE@/$$f; then	\
-	    $(INSTALL_DATA) $(srcdir)/$$f $(DESTDIR)$(sysconfdir)/@PACKAGE@/ ;	\
+	    ${INSTALL} -m 640 $(srcdir)/$$f $(DESTDIR)$(sysconfdir)/@PACKAGE@/ ;	\
 	  fi ;	\
 	done
 

src/bin/resolver/main.cc

@@ -208,7 +208,8 @@ main(int argc, char* argv[]) {
         cc_session = new Session(io_service.get_io_service());
         config_session = new ModuleCCSession(specfile, *cc_session,
                                              my_config_handler,
-                                             my_command_handler);
+                                             my_command_handler,
+                                             true, true);
         LOG_DEBUG(resolver_logger, RESOLVER_DBG_INIT, RESOLVER_CONFIGCHAN);
 
         // FIXME: This does not belong here, but inside Boss

src/bin/resolver/tests/Makefile.am

@@ -34,9 +34,7 @@ run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 run_unittests_LDFLAGS  = $(AM_LDFLAGS)  $(GTEST_LDFLAGS)
 
 run_unittests_LDADD  = $(GTEST_LDADD)
-run_unittests_LDADD += $(SQLITE_LIBS)
 run_unittests_LDADD += $(top_builddir)/src/lib/testutils/libtestutils.la
-run_unittests_LDADD += $(top_builddir)/src/lib/datasrc/libdatasrc.la
 run_unittests_LDADD += $(top_builddir)/src/lib/dns/libdns++.la
 run_unittests_LDADD += $(top_builddir)/src/lib/asiodns/libasiodns.la
 run_unittests_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la

src/bin/resolver/tests/run_unittests.cc

@@ -13,6 +13,7 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <gtest/gtest.h>
+#include <log/logger_support.h>
 #include <util/unittests/run_all.h>
 
 #include <dns/tests/unittest_util.h>
@@ -22,6 +23,7 @@ main(int argc, char* argv[]) {
     ::testing::InitGoogleTest(&argc, argv);
     isc::UnitTestUtil::addDataPath(TEST_DATA_DIR);
     isc::UnitTestUtil::addDataPath(TEST_DATA_BUILDDIR);
+    isc::log::initLogger();
 
     return (isc::util::unittests::run_all());
 }

src/bin/xfrin/tests/xfrin_test.py

@@ -15,6 +15,7 @@
 
 import unittest
 import socket
+import io
 from isc.testutils.tsigctx_mock import MockTSIGContext
 from xfrin import *
 
@@ -78,7 +79,7 @@ class MockXfrin(Xfrin):
 
     def _get_db_file(self):
         pass
-    
+
     def _cc_check_command(self):
         self._shutdown_event.set()
         if MockXfrin.check_command_hook:
@@ -207,6 +208,18 @@ class TestXfrinConnection(unittest.TestCase):
         mock_ctx.error = error
         return mock_ctx
 
+    def __match_exception(self, expected_exception, expected_msg, expression):
+        # This helper method is a higher-granularity version of assertRaises().
+        # If it's not sufficient to check the exception class (e.g., when
+        # the same type of exceptions can be thrown from many places), this
+        # method can be used to check it with the exception argument.
+        try:
+            expression()
+        except expected_exception as ex:
+            self.assertEqual(str(ex), expected_msg)
+        else:
+            self.assertFalse('exception is expected, but not raised')
+
     def test_close(self):
         # we shouldn't be using the global asyncore map.
         self.assertEqual(len(asyncore.socket_map), 0)
@@ -293,6 +306,31 @@ class TestXfrinConnection(unittest.TestCase):
         self.conn.reply_data = self.conn.create_response_data(bad_qid = True)
         self.assertRaises(XfrinException, self._handle_xfrin_response)
 
+    def test_response_error_code_bad_sig(self):
+        self.conn._tsig_key = TSIG_KEY
+        self.conn._tsig_ctx_creator = \
+            lambda key: self.__create_mock_tsig(key, TSIGError.BAD_SIG)
+        self.conn._send_query(RRType.AXFR())
+        self.conn.reply_data = self.conn.create_response_data(
+                rcode=Rcode.SERVFAIL())
+        # xfrin should check TSIG before other part of incoming message
+        # validate log message for XfrinException
+        self.__match_exception(XfrinException,
+                               "TSIG verify fail: BADSIG",
+                               self._handle_xfrin_response)
+
+    def test_response_bad_qid_bad_key(self):
+        self.conn._tsig_key = TSIG_KEY
+        self.conn._tsig_ctx_creator = \
+            lambda key: self.__create_mock_tsig(key, TSIGError.BAD_KEY)
+        self.conn._send_query(RRType.AXFR())
+        self.conn.reply_data = self.conn.create_response_data(bad_qid=True)
+        # xfrin should check TSIG before other part of incoming message
+        # validate log message for XfrinException
+        self.__match_exception(XfrinException,
+                               "TSIG verify fail: BADKEY",
+                               self._handle_xfrin_response)
+
     def test_response_non_response(self):
         self.conn._send_query(RRType.AXFR())
         self.conn.reply_data = self.conn.create_response_data(response = False)
@@ -337,6 +375,18 @@ class TestXfrinConnection(unittest.TestCase):
         self.conn.response_generator = self._create_soa_response_data
         self.assertRaises(XfrinException, self.conn._check_soa_serial)
 
+    def test_soacheck_bad_qid_bad_sig(self):
+        self.conn._tsig_key = TSIG_KEY
+        self.conn._tsig_ctx_creator = \
+            lambda key: self.__create_mock_tsig(key, TSIGError.BAD_SIG)
+        self.soa_response_params['bad_qid'] = True
+        self.conn.response_generator = self._create_soa_response_data
+        # xfrin should check TSIG before other part of incoming message
+        # validate log message for XfrinException
+        self.__match_exception(XfrinException,
+                               "TSIG verify fail: BADSIG",
+                               self.conn._check_soa_serial)
+
     def test_soacheck_non_response(self):
         self.soa_response_params['response'] = False
         self.conn.response_generator = self._create_soa_response_data

src/bin/xfrin/xfrin.py.in

@@ -243,13 +243,13 @@ class XfrinConnection(asyncore.dispatcher):
         msg = Message(Message.PARSE)
         msg.from_wire(soa_response)
 
+        # TSIG related checks, including an unexpected signed response
+        self._check_response_tsig(msg, soa_response)
+
         # perform some minimal level validation.  It's an open issue how
         # strict we should be (see the comment in _check_response_header())
         self._check_response_header(msg)
 
-        # TSIG related checks, including an unexpected signed response
-        self._check_response_tsig(msg, soa_response)
-
         # TODO, need select soa record from data source then compare the two
         # serial, current just return OK, since this function hasn't been used
         # now.
@@ -311,7 +311,7 @@ class XfrinConnection(asyncore.dispatcher):
             raise XfrinException('error response: %s' % msg_rcode.to_text())
 
         if not msg.get_header_flag(Message.HEADERFLAG_QR):
-            raise XfrinException('response is not a response ')
+            raise XfrinException('response is not a response')
 
         if msg.get_qid() != self._query_id:
             raise XfrinException('bad query id')
@@ -362,11 +362,13 @@ class XfrinConnection(asyncore.dispatcher):
             recvdata = self._get_request_response(msg_len)
             msg = Message(Message.PARSE)
             msg.from_wire(recvdata)
-            self._check_response_status(msg)
 
             # TSIG related checks, including an unexpected signed response
             self._check_response_tsig(msg, recvdata)
 
+            # Perform response status validation
+            self._check_response_status(msg)
+
             answer_section = msg.get_section(Message.SECTION_ANSWER)
             for rr in self._handle_answer_section(answer_section):
                 yield rr

src/cppcheck-suppress.lst

@@ -4,11 +4,6 @@ debug
 missingInclude
 // This is a template, and should be excluded from the check
 unreadVariable:src/lib/dns/rdata/template.cc:60
-// These three trigger warnings due to the incomplete implementation.  This is
-// our problem, but we need to suppress the warnings for now.
-functionConst:src/lib/cache/resolver_cache.h
-functionConst:src/lib/cache/message_cache.h
-functionConst:src/lib/cache/rrset_cache.h
 // Intentional self assignment tests.  Suppress warning about them.
 selfAssignment:src/lib/dns/tests/name_unittest.cc:293
 selfAssignment:src/lib/dns/tests/rdata_unittest.cc:228

src/lib/Makefile.am

@@ -1,3 +1,3 @@
 SUBDIRS = exceptions util log cryptolink dns cc config python xfr \
           bench asiolink asiodns nsas cache resolve testutils datasrc \
-          server_common
+          server_common acl

src/lib/acl/Makefile.am

@@ -0,0 +1,4 @@
+SUBDIRS = tests
+
+# TODO: Once we have some cc file we are able to compile, create the library.
+# For now, we have only header files, not creating empty library.

src/lib/acl/check.h

@@ -0,0 +1,195 @@
+// Copyright (C) 2011  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 ACL_CHECK_H
+#define ACL_CHECK_H
+
+#include <vector>
+#include <typeinfo>
+#include <sstream>
+
+namespace isc {
+namespace acl {
+
+/**
+ * \brief ACL check base class.
+ *
+ * It is intended that all ACL checks are inherited (maybe indirectly) from
+ * this base class. This will allow us to define new types of checks without
+ * changing any of the code that is using it and with the correct
+ * implementation even without changing the thing that parses configuration
+ * and creates instances of the checks.
+ *
+ * It is implemented as a template. This allows easy reuse of the code for
+ * checking of different types of things (packets of different protocols, etc).
+ * We'll implement the loader and compound checks as templates as well (
+ * and just make sure they are instantiated for each type of thing we want
+ * to check). While most of concrete checks will be specific for one protocol
+ * (or whatever the entity we check is), it makes sense to implement some of
+ * these as templates as well (for example the IP address check, for whatever
+ * context that contains member called ip and has the right methods).
+ *
+ * The Context carries whatever information might be checked for that protocol
+ * (eg. the packet, information where it came from, to what port, ...).
+ */
+template<typename Context> class Check {
+protected:
+    /// \brief Constructor.
+    ///
+    /// Just to make sure this thing is not directly instantiated.
+    Check() { }
+public:
+    /**
+     * \brief The check itself.
+     *
+     * The actual check will be performed here. Every concrete child class
+     * will reimplement it and decide based on the context passed if it
+     * matches.
+     *
+     * The caller should expect this method can throw. The list of exceptions
+     * isn't restricted, as we don't know what kind of checks will be needed.
+     * An exception should be considered as it is impossible to check the
+     * condition. It should lead to either blackholing the packet or returning
+     * some 500-like error (ServFail).
+     *
+     * \param context The thing we are trying to match against this check.
+     * \return true if the context satisfies the check, false otherwise.
+     */
+    virtual bool matches(const Context& context) const = 0;
+
+    /**
+     * \brief Cost for unknown cost estimate.
+     *
+     * This indicates that the estimate for cost is not provided. This
+     * is arbitrary large value, meaning "somehow longish time". To be
+     * on the safe side, we guess more and be just happily suprirised
+     * if it turns out to run faster.
+     */
+    static const unsigned UNKNOWN_COST;
+
+    /**
+     * \brief The expected cost of single match.
+     *
+     * This is here to provide some kind of cost information to optimising
+     * routines. It is in units without any real size, just bigger number
+     * means the check takes longer time. It is expected to be linear scale.
+     * It doesn't need to be exact, but better accuracy might lead to better
+     * optimisations. As of writing this, no optimisations exist yet, but
+     * are expected to exist in future.
+     *
+     * The default is UNKNOWN_COST.
+     */
+    virtual unsigned cost() const {
+        return (UNKNOWN_COST);
+    }
+
+    /// \brief Virtual destructor, as we're virtual
+    virtual ~ Check() { }
+
+    /**
+     * \brief Conversion to text.
+     *
+     * This is meant for debugging purposes, it doesn't have to
+     * serialise the whole information stored in this Check.
+     *
+     * If the check is compound, it should not include the subexpressions
+     * (while we're able to build whatever treeish representation using
+     * CompoundCheck::subexpressions, we're not able to separate them
+     * automatically, as this may produce any kind of free-form string).
+     */
+    virtual std::string toText() const {
+        std::stringstream output;
+        output << typeid(*this).name() << "@" << this;
+        return (output.rdbuf()->str());
+    }
+};
+
+// This seems to be the propper way for static template members
+template<typename Context> const unsigned Check<Context>::UNKNOWN_COST = 10000;
+
+/**
+ * \brief Base class for compound checks.
+ *
+ * While some checks will be a match against some property of the information
+ * passed (eg. the sender's IP address must be in some range), others will
+ * combine results of more checks together to get their own. This is base class
+ * for the second type, allowing listing of the subexpressions (mostly for
+ * debugging purposes to print the whole tree of matches and possible future
+ * optimisations which would like to crawl the expression tree).
+ */
+template<typename Context> class CompoundCheck : public Check<Context> {
+public:
+    /// \brief Abbreviated name for list of subexpressions
+    typedef std::vector<const Check<Context>*> Checks;
+
+    /**
+     * \brief Get the list of subexpressions.
+     *
+     * The result contains pointers to the all subexpressions this check holds
+     * (and therefore might call during its own match() function).
+     *
+     * Using shared pointers looks an overkill here. All the checks must be
+     * alive for the whole life of this one and this check will hold their
+     * ownership. Therefore the only thing the caller needs to do is to make
+     * sure this check is not deleted while it's still using the ones from the
+     * result.
+     *
+     * This method must not throw except for the standard allocation exceptions
+     * to allocate the result.
+     */
+    virtual Checks getSubexpressions() const = 0;
+
+    /**
+     * \brief If the result depends only on results of subexpressions.
+     *
+     * Some optimisations might use the fact that a compound expression is
+     * a function of results of its subexpressions (subchecks) only. But
+     * some compound checks might want to look into the provided context in
+     * their match() as well as looking at the results of the subexpressions.
+     *
+     * This function informs the optimisation routines if it is safe to use
+     * these optimisations.
+     *
+     * \return true if the check depends only on results of subexpressions
+     *    only, false if it examines the context itself as well.
+     * \note The default implementation returns true, as it is expected to
+     *    be the case in majority of cases.
+     */
+    virtual bool pure() const { return (true); }
+
+    /**
+     * \brief Default compound cost function.
+     *
+     * It is simply sum of all subexpressions, as an expected upper bound
+     * on the cost. This expects that the combining itself is cheap relatively
+     * to the checks performed by the subexpressions. In most cases, this
+     * should be good enough, but it can be reimplemented in situations
+     * where most of the subexpressions will be avoided in usual situations.
+     * Replacing the default of 10000 from Check.
+     */
+    virtual unsigned cost() const {
+        Checks checks(getSubexpressions());
+        unsigned result(0);
+        for (typename Checks::const_iterator i(checks.begin());
+             i != checks.end(); ++ i) {
+            result += (*i)->cost();
+        }
+        return (result);
+    }
+};
+
+}
+}
+
+#endif

src/lib/acl/tests/Makefile.am

@@ -0,0 +1,15 @@
+AM_CPPFLAGS = -I$(top_builddir)/src/lib -I$(top_srcdir)/src/lib
+
+TESTS =
+if HAVE_GTEST
+TESTS += run_unittests
+run_unittests_SOURCES = run_unittests.cc
+run_unittests_SOURCES += check_test.cc
+run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
+run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)
+
+run_unittests_LDADD = $(GTEST_LDADD)
+run_unittests_LDADD += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la
+endif
+
+noinst_PROGRAMS = $(TESTS)

src/lib/acl/tests/check_test.cc

@@ -0,0 +1,70 @@
+// Copyright (C) 2011  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 <gtest/gtest.h>
+#include <acl/check.h>
+
+using namespace isc::acl;
+
+namespace {
+
+// This test has two function. For one, it checks the default implementations
+// do what they should and it makes sure the template actually compiles
+// (as templates are syntax-checked upon instantiation).
+
+// This is a test check that just passes the boolean it gets.
+class Pass : public Check<bool> {
+public:
+    virtual bool matches(const bool& value) const { return (value); }
+};
+
+// This is a simple test compound check. It contains two Pass checks
+// and passes result of the first one.
+
+class First : public CompoundCheck<bool> {
+public:
+    // The internal checks are public, so we can check the addresses
+    Pass first, second;
+    virtual Checks getSubexpressions() const {
+        Checks result;
+        result.push_back(&first);
+        result.push_back(&second);
+        return (result);
+    }
+    virtual bool matches(const bool& value) const {
+        return (first.matches(value));
+    }
+};
+
+TEST(Check, defaultCheckValues) {
+    Pass p;
+    EXPECT_EQ(Check<bool>::UNKNOWN_COST, p.cost());
+    EXPECT_TRUE(p.matches(true));
+    EXPECT_FALSE(p.matches(false));
+    // The exact text is compiler dependant, but we check it returns something
+    // and can be compiled
+    EXPECT_FALSE(p.toText().empty());
+}
+
+TEST(Check, defaultCompoundValues) {
+    First f;
+    EXPECT_EQ(2 * Check<bool>::UNKNOWN_COST, f.cost());
+    EXPECT_TRUE(f.pure());
+    First::Checks c(f.getSubexpressions());
+    ASSERT_EQ(2, c.size());
+    EXPECT_EQ(&f.first, c[0]);
+    EXPECT_EQ(&f.second, c[1]);
+}
+
+}

src/lib/acl/tests/run_unittests.cc

@@ -0,0 +1,23 @@
+// Copyright (C) 2011  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 <gtest/gtest.h>
+#include <util/unittests/run_all.h>
+
+int
+main(int argc, char* argv[]) {
+    ::testing::InitGoogleTest(&argc, argv);
+    return (isc::util::unittests::run_all());
+}
+

src/lib/asiodns/tests/Makefile.am

@@ -25,7 +25,6 @@ run_unittests_SOURCES += io_fetch_unittest.cc
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 
 run_unittests_LDADD  = $(GTEST_LDADD)
-run_unittests_LDADD += $(SQLITE_LIBS)
 run_unittests_LDADD += $(top_builddir)/src/lib/dns/libdns++.la
 run_unittests_LDADD += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la
 run_unittests_LDADD += $(top_builddir)/src/lib/util/libutil.la

src/lib/asiodns/tests/run_unittests.cc

@@ -15,14 +15,14 @@
 #include <gtest/gtest.h>
 #include <util/unittests/run_all.h>
 
-#include <log/root_logger_name.h>
+#include <log/logger_manager.h>
 #include <dns/tests/unittest_util.h>
 
 int
 main(int argc, char* argv[])
 {
     ::testing::InitGoogleTest(&argc, argv);         // Initialize Google test
-    isc::log::setRootLoggerName("unittest");        // Set a root logger name
+    isc::log::LoggerManager::init("unittest");      // Set a root logger name
     isc::UnitTestUtil::addDataPath(TEST_DATA_DIR);  // Add location of test data
 
     return (isc::util::unittests::run_all());

src/lib/asiolink/tests/Makefile.am

@@ -34,7 +34,6 @@ run_unittests_SOURCES += udp_socket_unittest.cc
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 
 run_unittests_LDADD  = $(GTEST_LDADD)
-run_unittests_LDADD += $(SQLITE_LIBS)
 run_unittests_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la
 run_unittests_LDADD += $(top_builddir)/src/lib/log/liblog.la
 run_unittests_LDADD += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la

src/lib/asiolink/tests/run_unittests.cc

@@ -14,15 +14,12 @@
 
 #include <gtest/gtest.h>
 #include <util/unittests/run_all.h>
-
-#include <log/root_logger_name.h>
-#include <dns/tests/unittest_util.h>
+#include <log/logger_manager.h>
 
 int
 main(int argc, char* argv[])
 {
     ::testing::InitGoogleTest(&argc, argv);         // Initialize Google test
-    isc::log::setRootLoggerName("unittest");        // Set a root logger name
-
+    isc::log::LoggerManager::init("unittest");      // Set a root logger name
     return (isc::util::unittests::run_all());
 }

src/lib/cache/TODO

@@ -12,7 +12,8 @@
 * When the rrset beging updated is an NS rrset, NSAS should be updated
   together.
 * Share the NXDOMAIN info between different type queries. current implementation
-  can only cache for the type that user quired, for example, if user query A 
+  can only cache for the type that user queried, for example, if user query A
   record of a.example. and the server replied with NXDOMAIN, this should be
   cached for all the types queries of a.example.
-
+* Add the interfaces for resizing and serialization (loading and dumping) to
+  cache.

src/lib/cache/message_cache.cc

@@ -97,24 +97,6 @@ MessageCache::update(const Message& msg) {
     return (message_table_.add(msg_entry, entry_key, true));
 }
 
-#if 0
-void
-MessageCache::dump(const std::string&) {
-    //TODO
-}
-
-void
-MessageCache::load(const std::string&) {
-    //TODO
-}
-
-bool
-MessageCache::resize(uint32_t) {
-    //TODO
-    return (true);
-}
-#endif
-
 } // namespace cache
 } // namespace isc
 

src/lib/cache/message_cache.h

@@ -30,6 +30,8 @@ namespace cache {
 /// The object of MessageCache represents the cache for class-specific
 /// messages.
 ///
+/// \todo The message cache class should provide the interfaces for
+///       loading, dumping and resizing.
 class MessageCache {
 // Noncopyable
 private:
@@ -64,20 +66,6 @@ public:
     /// If the message doesn't exist in the cache, it will be added
     /// directly.
     bool update(const isc::dns::Message& msg);
-
-#if 0
-    /// \brief Dump the message cache to specified file.
-    /// \todo It should can be dumped to one configured database.
-    void dump(const std::string& file_name);
-
-    /// \brief Load the cache from one file.
-    /// \todo It should can be loaded from one configured database.
-    void load(const std::string& file_name);
-
-    /// \brief Resize the size of message cache in runtime.
-    bool resize(uint32_t size);
-#endif
-
 protected:
     /// \brief Get the hash key for the message entry in the cache.
     /// \param name query name of the message.

src/lib/cache/resolver_cache.cc

@@ -227,16 +227,6 @@ ResolverCache::update(const isc::dns::ConstRRsetPtr& rrset_ptr) {
     }
 }
 
-void
-ResolverCache::dump(const std::string&) {
-    //TODO
-}
-
-void
-ResolverCache::load(const std::string&) {
-    //TODO
-}
-
 ResolverClassCache*
 ResolverCache::getClassCache(const isc::dns::RRClass& cache_class) const {
     for (int i = 0; i < class_caches_.size(); ++i) {

src/lib/cache/resolver_cache.h

@@ -76,6 +76,9 @@ public:
 ///
 /// \note Public interaction with the cache should be through ResolverCache,
 /// not directly with this one. (TODO: make this private/hidden/local to the .cc?)
+///
+/// \todo The resolver cache class should provide the interfaces for
+///       loading, dumping and resizing.
 class ResolverClassCache {
 public:
     /// \brief Default Constructor.
@@ -300,23 +303,6 @@ public:
     ///
     bool update(const isc::dns::ConstRRsetPtr& rrset_ptr);
 
-    /// \name Cache Serialization
-    //@{
-    /// \brief Dump the cache content to one file.
-    ///
-    /// \param file_name file to write to
-    ///
-    /// \todo It should can be dumped to one configured database.
-    void dump(const std::string& file_name);
-
-    /// \brief Load the cache from one file.
-    ///
-    /// \param file to load from
-    ///
-    /// \todo It should can be loaded from one configured database.
-    void load(const std::string& file_name);
-    //@}
-
 private:
     /// \brief Returns the class-specific subcache
     ///

src/lib/cache/rrset_cache.cc

@@ -79,24 +79,6 @@ RRsetCache::update(const isc::dns::RRset& rrset, const RRsetTrustLevel& level) {
     return (entry_ptr);
 }
 
-#if 0
-void
-RRsetCache::dump(const std::string&) {
-    //TODO
-}
-
-void
-RRsetCache::load(const std::string&) {
-    //TODO
-}
-
-bool
-RRsetCache::resize(uint32_t) {
-    //TODO
-    return (true);
-}
-#endif
-
 } // namespace cache
 } // namespace isc
 

src/lib/cache/rrset_cache.h

@@ -30,6 +30,9 @@ class RRsetEntry;
 /// \brief RRset Cache
 /// The object of RRsetCache represented the cache for class-specific
 /// RRsets.
+///
+/// \todo The rrset cache class should provide the interfaces for
+///       loading, dumping and resizing.
 class RRsetCache{
     ///
     /// \name Constructors and Destructor
@@ -73,28 +76,6 @@ public:
     RRsetEntryPtr update(const isc::dns::RRset& rrset,
                          const RRsetTrustLevel& level);
 
-#if 0
-    /// \brief Dump the rrset cache to specified file.
-    ///
-    /// \param file_name The file to write to
-    ///
-    /// \todo It should can be dumped to one configured database.
-    void dump(const std::string& file_name);
-
-    /// \brief Load the cache from one file.
-    ///
-    /// \param file_name The file to read from
-    ///
-    /// \todo It should can be loaded from one configured database.
-    void load(const std::string& file_name);
-
-    /// \brief Resize the size of rrset cache in runtime.
-    ///
-    /// \param The size to resize to
-    /// \return true
-    bool resize(uint32_t size);
-#endif
-
     /// \short Protected memebers, so they can be accessed by tests.
 protected:
     uint16_t class_; // The class of the rrset cache.

src/lib/config/ccsession.cc

@@ -35,6 +35,10 @@
 #include <config/config_log.h>
 #include <config/ccsession.h>
 
+#include <log/logger_support.h>
+#include <log/logger_specification.h>
+#include <log/logger_manager.h>
+
 using namespace std;
 
 using isc::data::Element;
@@ -151,6 +155,115 @@ parseCommand(ConstElementPtr& arg, ConstElementPtr command) {
     }
 }
 
+namespace {
+// Temporary workaround functions for missing functionality in
+// getValue() (main problem described in ticket #993)
+// This returns either the value set for the given relative id,
+// or its default value
+// (intentially defined here so this interface does not get
+// included in ConfigData as it is)
+ConstElementPtr getValueOrDefault(ConstElementPtr config_part,
+                                  const std::string& relative_id,
+                                  const ConfigData& config_data,
+                                  const std::string& full_id) {
+    if (config_part->contains(relative_id)) {
+        return config_part->get(relative_id);
+    } else {
+        return config_data.getDefaultValue(full_id);
+    }
+}
+
+// Reads a output_option subelement of a logger configuration,
+// and sets the values thereing to the given OutputOption struct,
+// or defaults values if they are not provided (from config_data).
+void
+readOutputOptionConf(isc::log::OutputOption& output_option,
+                     ConstElementPtr output_option_el,
+                     const ConfigData& config_data)
+{
+    ConstElementPtr destination_el = getValueOrDefault(output_option_el,
+                                    "destination", config_data,
+                                    "loggers/output_options/destination");
+    output_option.destination = isc::log::getDestination(destination_el->stringValue());
+    ConstElementPtr output_el = getValueOrDefault(output_option_el,
+                                    "output", config_data,
+                                    "loggers/output_options/output");
+    if (output_option.destination == isc::log::OutputOption::DEST_CONSOLE) {
+        output_option.stream = isc::log::getStream(output_el->stringValue());
+    } else if (output_option.destination == isc::log::OutputOption::DEST_FILE) {
+        output_option.filename = output_el->stringValue();
+    } else if (output_option.destination == isc::log::OutputOption::DEST_SYSLOG) {
+        output_option.facility = output_el->stringValue();
+    }
+    output_option.flush = getValueOrDefault(output_option_el,
+                              "flush", config_data,
+                              "loggers/output_options/flush")->boolValue();
+    output_option.maxsize = getValueOrDefault(output_option_el,
+                                "maxsize", config_data,
+                                "loggers/output_options/maxsize")->intValue();
+    output_option.maxver = getValueOrDefault(output_option_el,
+                               "maxver", config_data,
+                               "loggers/output_options/maxver")->intValue();
+}
+
+// Reads a full 'loggers' configuration, and adds the loggers therein
+// to the given vector, fills in blanks with defaults from config_data
+void
+readLoggersConf(std::vector<isc::log::LoggerSpecification>& specs,
+                ConstElementPtr logger,
+                const ConfigData& config_data)
+{
+    const std::string lname = logger->get("name")->stringValue();
+    ConstElementPtr severity_el = getValueOrDefault(logger,
+                                      "severity", config_data,
+                                      "loggers/severity");
+    isc::log::Severity severity = isc::log::getSeverity(
+                                      severity_el->stringValue());
+    int dbg_level = getValueOrDefault(logger, "debuglevel",
+                                      config_data,
+                                      "loggers/debuglevel")->intValue();
+    bool additive = getValueOrDefault(logger, "additive", config_data,
+                                      "loggers/additive")->boolValue();
+
+    isc::log::LoggerSpecification logger_spec(
+        lname, severity, dbg_level, additive
+    );
+
+    if (logger->contains("output_options")) {
+        BOOST_FOREACH(ConstElementPtr output_option_el,
+                      logger->get("output_options")->listValue()) {
+            // create outputoptions
+            isc::log::OutputOption output_option;
+            readOutputOptionConf(output_option,
+                                 output_option_el,
+                                 config_data);
+            logger_spec.addOutputOption(output_option);
+        }
+    }
+
+    specs.push_back(logger_spec);
+}
+
+} // end anonymous namespace
+
+void
+my_logconfig_handler(const std::string&n, ConstElementPtr new_config, const ConfigData& config_data) {
+    config_data.getModuleSpec().validateConfig(new_config, true);
+
+    std::vector<isc::log::LoggerSpecification> specs;
+
+    if (new_config->contains("loggers")) {
+        BOOST_FOREACH(ConstElementPtr logger,
+                      new_config->get("loggers")->listValue()) {
+            readLoggersConf(specs, logger, config_data);
+        }
+    }
+
+    isc::log::LoggerManager logger_manager;
+    logger_manager.process(specs.begin(), specs.end());
+}
+
+
 ModuleSpec
 ModuleCCSession::readModuleSpecification(const std::string& filename) {
     std::ifstream file;
@@ -193,7 +306,8 @@ ModuleCCSession::ModuleCCSession(
         isc::data::ConstElementPtr new_config),
     isc::data::ConstElementPtr(*command_handler)(
         const std::string& command, isc::data::ConstElementPtr args),
-    bool start_immediately
+    bool start_immediately,
+    bool handle_logging
     ) :
     started_(false),
     session_(session)
@@ -207,10 +321,8 @@ ModuleCCSession::ModuleCCSession(
 
     session_.establish(NULL);
     session_.subscribe(module_name_, "*");
-    //session_.subscribe("Boss", "*");
-    //session_.subscribe("statistics", "*");
-    // send the data specification
 
+    // send the data specification
     ConstElementPtr spec_msg = createCommand("module_spec",
                                              module_specification_.getFullSpec());
     unsigned int seq = session_.group_sendmsg(spec_msg, "ConfigManager");
@@ -239,9 +351,15 @@ ModuleCCSession::ModuleCCSession(
         }
     }
 
+    // Keep track of logging settings automatically
+    if (handle_logging) {
+        addRemoteConfig("Logging", my_logconfig_handler, false);
+    }
+
     if (start_immediately) {
         start();
     }
+
 }
 
 void
@@ -361,6 +479,11 @@ ModuleCCSession::checkCommand() {
             }
         } catch (const CCSessionError& re) {
             LOG_ERROR(config_logger, CONFIG_CCSESSION_MSG).arg(re.what());
+        } catch (const std::exception& stde) {
+            // No matter what unexpected error happens, we do not want
+            // to crash because of an incoming event, so we log the
+            // exception and continue to run
+            LOG_ERROR(config_logger, CONFIG_CCSESSION_MSG_INTERNAL).arg(stde.what());
         }
         if (!isNull(answer)) {
             session_.reply(routing, answer);
@@ -382,9 +505,7 @@ ModuleCCSession::fetchRemoteSpec(const std::string& module, bool is_filename) {
         ConstElementPtr cmd(createCommand("get_module_spec",
                             Element::fromJSON("{\"module_name\": \"" + module +
                                               "\"}")));
-        const unsigned int seq = session_.group_sendmsg(cmd, "ConfigManager");
-
-        // Wait for the answer
+        unsigned int seq = session_.group_sendmsg(cmd, "ConfigManager");
         ConstElementPtr env, answer;
         session_.group_recvmsg(env, answer, false, seq);
         int rcode;
@@ -407,7 +528,8 @@ 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),
+                                                 ConstElementPtr,
+                                                 const ConfigData&),
                                  bool spec_is_filename)
 {
     // First get the module name, specification and default config
@@ -439,7 +561,7 @@ ModuleCCSession::addRemoteConfig(const std::string& spec_name,
     remote_module_configs_[module_name] = rmod_config;
     if (handler) {
         remote_module_handlers_[module_name] = handler;
-        handler(module_name, local_config);
+        handler(module_name, local_config, rmod_config);
     }
 
     // Make sure we get updates in future
@@ -487,7 +609,7 @@ ModuleCCSession::updateRemoteConfig(const std::string& module_name,
         std::map<std::string, RemoteHandler>::iterator hit =
             remote_module_handlers_.find(module_name);
         if (hit != remote_module_handlers_.end()) {
-            hit->second(module_name, new_config);
+            hit->second(module_name, new_config, it->second);
         }
     }
 }

src/lib/config/ccsession.h

@@ -161,6 +161,7 @@ public:
      * configuration of the local module needs to be updated.
      * This must refer to a valid object of a concrete derived class of
      * AbstractSession without establishing the session.
+     *
      * Note: the design decision on who is responsible for establishing the
      * session is in flux, and may change in near future.
      *
@@ -171,11 +172,14 @@ public:
      * @param command_handler A callback function pointer to be called when
      * a control command from a remote agent needs to be performed on the
      * local module.
-     * @start_immediately If true (default), start listening to new commands
+     * @param start_immediately If true (default), start listening to new commands
      * and configuration changes asynchronously at the end of the constructor;
      * if false, it will be delayed until the start() method is explicitly
      * called. (This is a short term workaround for an initialization trouble.
      * We'll need to develop a cleaner solution, and then remove this knob)
+     * @param handle_logging If true, the ModuleCCSession will automatically
+     * take care of logging configuration through the virtual Logging config
+     * module.
      */
     ModuleCCSession(const std::string& spec_file_name,
                     isc::cc::AbstractSession& session,
@@ -184,7 +188,8 @@ public:
                     isc::data::ConstElementPtr(*command_handler)(
                         const std::string& command,
                         isc::data::ConstElementPtr args) = NULL,
-                    bool start_immediately = true
+                    bool start_immediately = true,
+                    bool handle_logging = false
                     );
 
     /// Start receiving new commands and configuration changes asynchronously.
@@ -283,7 +288,8 @@ public:
     std::string addRemoteConfig(const std::string& spec_name,
                                 void (*handler)(const std::string& module_name,
                                                 isc::data::ConstElementPtr
-                                                update) = NULL,
+                                                update,
+                                                const ConfigData& config_data) = NULL,
                                 bool spec_is_filename = true);
 
     /**
@@ -337,7 +343,8 @@ private:
         isc::data::ConstElementPtr args);
 
     typedef void (*RemoteHandler)(const std::string&,
-                                  isc::data::ConstElementPtr);
+                                  isc::data::ConstElementPtr,
+                                  const ConfigData&);
     std::map<std::string, ConfigData> remote_module_configs_;
     std::map<std::string, RemoteHandler> remote_module_handlers_;
 

src/lib/config/config_data.cc

@@ -21,6 +21,63 @@
 
 using namespace isc::data;
 
+namespace {
+
+// Returns the '_spec' part of a list or map specification (recursively,
+// i.e. if it is a list of lists or maps, will return the spec of the
+// inner-most list or map).
+//
+// \param spec_part the list or map specification (part)
+// \return the value of spec_part's "list_item_spec" or "map_item_spec",
+//         or the original spec_part, if it is not a MapElement or does
+//         not contain "list_item_spec" or "map_item_spec"
+ConstElementPtr findListOrMapSubSpec(ConstElementPtr spec_part) {
+    while (spec_part->getType() == Element::map &&
+           (spec_part->contains("list_item_spec") ||
+            spec_part->contains("map_item_spec"))) {
+        if (spec_part->contains("list_item_spec")) {
+            spec_part = spec_part->get("list_item_spec");
+        } else {
+            spec_part = spec_part->get("map_item_spec");
+        }
+    }
+    return spec_part;
+}
+
+// Returns a specific Element in a given specification ListElement
+//
+// \exception DataNotFoundError if the given identifier does not
+// point to an existing element. Since we are dealing with the
+// specification here, and not the config data itself, this should
+// not happen, and is a code bug.
+//
+// \param spec_part ListElement to find the element in
+// \param id_part the name of the element to find (must match the value
+//                "item_name" in the list item
+// \param id_full the full identifier id_part is a part of, this is
+//                used to better report any errors
+ConstElementPtr findItemInSpecList(ConstElementPtr spec_part,
+                                   const std::string& id_part,
+                                   const std::string& id_full)
+{
+    bool found = false;
+    BOOST_FOREACH(ConstElementPtr list_el, spec_part->listValue()) {
+        if (list_el->getType() == Element::map &&
+            list_el->contains("item_name") &&
+            list_el->get("item_name")->stringValue() == id_part) {
+            spec_part = list_el;
+            found = true;
+        }
+    }
+    if (!found) {
+        isc_throw(isc::config::DataNotFoundError,
+                  id_part + " in " + id_full + " not found");
+    }
+    return (spec_part);
+}
+
+} // anonymous namespace
+
 namespace isc {
 namespace config {
 
@@ -36,11 +93,10 @@ namespace config {
 // validated and conforms to the specification.
 static ConstElementPtr
 find_spec_part(ConstElementPtr spec, const std::string& identifier) {
-    //std::cout << "[XX] find_spec_part for " << identifier << std::endl;
     if (!spec) {
         isc_throw(DataNotFoundError, "Empty specification");
     }
-    //std::cout << "in: " << std::endl << spec << std::endl;
+
     ConstElementPtr spec_part = spec;
     if (identifier == "") {
         isc_throw(DataNotFoundError, "Empty identifier");
@@ -49,59 +105,44 @@ find_spec_part(ConstElementPtr spec, const std::string& identifier) {
     size_t sep = id.find('/');
     while(sep != std::string::npos) {
         std::string part = id.substr(0, sep);
-        //std::cout << "[XX] id part: " << part << std::endl;
+
         if (spec_part->getType() == Element::list) {
-            bool found = false;
-            BOOST_FOREACH(ConstElementPtr list_el, spec_part->listValue()) {
-                if (list_el->getType() == Element::map &&
-                    list_el->contains("item_name") &&
-                    list_el->get("item_name")->stringValue() == part) {
-                    spec_part = list_el;
-                    found = true;
-                }
-            }
-            if (!found) {
-                isc_throw(DataNotFoundError, identifier);
-            }
+            spec_part = findItemInSpecList(spec_part, part, identifier);
+        } else {
+            isc_throw(DataNotFoundError,
+                      "Not a list of spec items: " + spec_part->str());
         }
         id = id.substr(sep + 1);
         sep = id.find("/");
+
+        // As long as we are not in the 'final' element as specified
+        // by the identifier, we want to automatically traverse list
+        // and map specifications
+        if (id != "" && id != "/") {
+            spec_part = findListOrMapSubSpec(spec_part);
+        }
     }
     if (id != "" && id != "/") {
         if (spec_part->getType() == Element::list) {
-            bool found = false;
-            BOOST_FOREACH(ConstElementPtr list_el, spec_part->listValue()) {
-                if (list_el->getType() == Element::map &&
-                    list_el->contains("item_name") &&
-                    list_el->get("item_name")->stringValue() == id) {
-                    spec_part = list_el;
-                    found = true;
-                }
-            }
-            if (!found) {
-                isc_throw(DataNotFoundError, identifier);
-            }
+            spec_part = findItemInSpecList(spec_part, id, identifier);
         } else if (spec_part->getType() == Element::map) {
             if (spec_part->contains("map_item_spec")) {
-                bool found = false;
-                BOOST_FOREACH(ConstElementPtr list_el,
-                              spec_part->get("map_item_spec")->listValue()) {
-                    if (list_el->getType() == Element::map &&
-                        list_el->contains("item_name") &&
-                        list_el->get("item_name")->stringValue() == id) {
-                        spec_part = list_el;
-                        found = true;
-                    }
-                }
-                if (!found) {
-                    isc_throw(DataNotFoundError, identifier);
-                }
+                spec_part = findItemInSpecList(
+                                spec_part->get("map_item_spec"),
+                                id, identifier);
             } else {
-                isc_throw(DataNotFoundError, identifier);
+                // Either we already have the element we are looking
+                // for, or we are trying to reach something that does
+                // not exist (i.e. the code does not match the spec)
+                if (!spec_part->contains("item_name") ||
+                    spec_part->get("item_name")->stringValue() != id) {
+                    isc_throw(DataNotFoundError, "Element above " + id +
+                                                 " in " + identifier +
+                                                 " is not a map: " + spec_part->str());
+                }
             }
         }
     }
-    //std::cout << "[XX] found spec part: " << std::endl << spec_part << std::endl;
     return (spec_part);
 }
 
@@ -164,6 +205,17 @@ ConfigData::getValue(bool& is_default, const std::string& identifier) const {
     return (value);
 }
 
+ConstElementPtr
+ConfigData::getDefaultValue(const std::string& identifier) const {
+    ConstElementPtr spec_part =
+        find_spec_part(_module_spec.getConfigSpec(), identifier);
+    if (spec_part->contains("item_default")) {
+        return spec_part->get("item_default");
+    } else {
+        isc_throw(DataNotFoundError, "No default for " + identifier);
+    }
+}
+
 /// Returns an ElementPtr pointing to a ListElement containing
 /// StringElements with the names of the options at the given
 /// identifier. If recurse is true, maps will be expanded as well

src/lib/config/config_data.h

@@ -57,6 +57,16 @@ public:
     ///        value that is to be returned
     isc::data::ConstElementPtr getValue(const std::string& identifier) const;
 
+    /// Returns the default value for the given identifier.
+    ///
+    /// \exception DataNotFoundError if the given identifier does not
+    ///            exist, or if the given value has no specified default
+    ///
+    /// \param identifier The identifier pointing to the configuration
+    ///        value for which the default is to be returned
+    /// \return ElementPtr containing the default value
+    isc::data::ConstElementPtr getDefaultValue(const std::string& identifier) const;
+
     /// Returns the value currently set for the given identifier
     /// If no value is set, the default value (as specified by the
     /// .spec file) is returned. If there is no value and no default,

src/lib/config/configdef.mes

@@ -48,3 +48,10 @@ channel. The message does not appear to be a valid command, and is
 missing a required element or contains an unknown data format. This
 most likely means that another BIND10 module is sending a bad message.
 The message itself is ignored by this module.
+
+% CCSESSION_MSG_INTERNAL error handling CC session message: %1
+There was an internal problem handling an incoming message on the
+command and control channel. An unexpected exception was thrown. This
+most likely points to an internal inconsistency in the module code. The
+exception message is appended to the log error, and the module will
+continue to run, but will not send back an answer.

src/lib/config/tests/ccsession_unittests.cc

@@ -160,6 +160,10 @@ TEST_F(CCSessionTest, session1) {
     EXPECT_EQ("ConfigManager", group);
     EXPECT_EQ("*", to);
     EXPECT_EQ(0, session.getMsgQueue()->size());
+
+    // without explicit argument, the session should not automatically
+    // subscribe to logging config
+    EXPECT_FALSE(session.haveSubscription("Logging", "*"));
 }
 
 TEST_F(CCSessionTest, session2) {
@@ -351,7 +355,9 @@ int remote_item1(0);
 ConstElementPtr remote_config;
 ModuleCCSession *remote_mccs(NULL);
 
-void remoteHandler(const std::string& module_name, ConstElementPtr config) {
+void remoteHandler(const std::string& module_name,
+                   ConstElementPtr config,
+                   const ConfigData&) {
     remote_module_name = module_name;
     remote_item1 = remote_mccs->getRemoteConfigValue("Spec2", "item1")->
         intValue();
@@ -595,6 +601,27 @@ TEST_F(CCSessionTest, delayedStart) {
                  FakeSession::DoubleRead);
 }
 
+TEST_F(CCSessionTest, loggingStart) {
+    // provide the logging module spec
+    ConstElementPtr log_spec = moduleSpecFromFile(LOG_SPEC_FILE).getFullSpec();
+    session.getMessages()->add(createAnswer(0, log_spec));
+    // just give an empty config
+    session.getMessages()->add(createAnswer(0, el("{}")));
+    ModuleCCSession mccs(ccspecfile("spec2.spec"), session, NULL, NULL,
+                         true, true);
+    EXPECT_TRUE(session.haveSubscription("Logging", "*"));
+}
+
+TEST_F(CCSessionTest, loggingStartBadSpec) {
+    // provide the logging module spec
+    session.getMessages()->add(createAnswer(0, el("{}")));
+    // just give an empty config
+    session.getMessages()->add(createAnswer(0, el("{}")));
+    EXPECT_THROW(new ModuleCCSession(ccspecfile("spec2.spec"), session,
+                 NULL, NULL, true, true), ModuleSpecError);
+    EXPECT_FALSE(session.haveSubscription("Logging", "*"));
+}
+
 // Similar to the above, but more implicitly by calling addRemoteConfig().
 // We should construct ModuleCCSession with start_immediately being false
 // if we need to call addRemoteConfig().

src/lib/config/tests/config_data_unittests.cc

@@ -64,21 +64,35 @@ TEST(ConfigData, getValue) {
     EXPECT_EQ("{  }", cd.getValue(is_default, "value6/")->str());
     EXPECT_TRUE(is_default);
     EXPECT_EQ("[  ]", cd.getValue("value8")->str());
+    EXPECT_EQ("[  ]", cd.getDefaultValue("value8")->str());
+    EXPECT_EQ("empty", cd.getValue("value8/a")->stringValue());
 
     EXPECT_THROW(cd.getValue("")->str(), DataNotFoundError);
     EXPECT_THROW(cd.getValue("/")->str(), DataNotFoundError);
     EXPECT_THROW(cd.getValue("no_such_item")->str(), DataNotFoundError);
     EXPECT_THROW(cd.getValue("value6/a")->str(), DataNotFoundError);
     EXPECT_THROW(cd.getValue("value6/no_such_item")->str(), DataNotFoundError);
-    EXPECT_THROW(cd.getValue("value8/a")->str(), DataNotFoundError);
-    EXPECT_THROW(cd.getValue("value8/a")->str(), DataNotFoundError);
-    EXPECT_THROW(cd.getValue("value8/a")->str(), DataNotFoundError);
+    EXPECT_THROW(cd.getValue("value8/b")->str(), DataNotFoundError);
 
     ModuleSpec spec1 = moduleSpecFromFile(std::string(TEST_DATA_PATH) + "/spec1.spec");
     ConfigData cd1 = ConfigData(spec1);
     EXPECT_THROW(cd1.getValue("anything")->str(), DataNotFoundError);
 }
 
+TEST(ConfigData, getDefaultValue) {
+    ModuleSpec spec31 = moduleSpecFromFile(std::string(TEST_DATA_PATH) + "/spec31.spec");
+    ConfigData cd = ConfigData(spec31);
+    EXPECT_EQ("[  ]", cd.getDefaultValue("first_list_items")->str());
+    EXPECT_EQ("\"foo\"", cd.getDefaultValue("first_list_items/foo")->str());
+    EXPECT_EQ("{  }", cd.getDefaultValue("first_list_items/second_list_items/map_element")->str());
+    EXPECT_EQ("[  ]", cd.getDefaultValue("first_list_items/second_list_items/map_element/list1")->str());
+    EXPECT_EQ("1", cd.getDefaultValue("first_list_items/second_list_items/map_element/list1/number")->str());
+
+    EXPECT_THROW(cd.getDefaultValue("doesnotexist")->str(), DataNotFoundError);
+    EXPECT_THROW(cd.getDefaultValue("first_list_items/second_list_items/map_element/list1/doesnotexist")->str(), DataNotFoundError);
+}
+
+
 TEST(ConfigData, setLocalConfig) {
     ModuleSpec spec2 = moduleSpecFromFile(std::string(TEST_DATA_PATH) + "/spec2.spec");
     ConfigData cd = ConfigData(spec2);

src/lib/config/tests/data_def_unittests_config.h.in

@@ -13,3 +13,4 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #define TEST_DATA_PATH "@abs_srcdir@/testdata"
+#define LOG_SPEC_FILE "@abs_top_srcdir@/src/bin/cfgmgr/plugins/logging.spec"

src/lib/datasrc/tests/run_unittests.cc

@@ -14,6 +14,7 @@
 
 #include <gtest/gtest.h>
 #include <util/unittests/run_all.h>
+#include <log/logger_support.h>
 
 #include <dns/tests/unittest_util.h>
 
@@ -22,5 +23,7 @@ main(int argc, char* argv[]) {
     ::testing::InitGoogleTest(&argc, argv);
     isc::UnitTestUtil::addDataPath(TEST_DATA_DIR);
 
+    isc::log::initLogger();
+
     return (isc::util::unittests::run_all());
 }

src/lib/log/Makefile.am

@@ -17,6 +17,7 @@ liblog_la_SOURCES += logger_level.cc logger_level.h
 liblog_la_SOURCES += logger_level_impl.cc logger_level_impl.h
 liblog_la_SOURCES += logger_manager.cc logger_manager.h
 liblog_la_SOURCES += logger_manager_impl.cc logger_manager_impl.h
+liblog_la_SOURCES += logger_name.cc logger_name.h
 liblog_la_SOURCES += logger_specification.h
 liblog_la_SOURCES += logger_support.cc logger_support.h
 liblog_la_SOURCES += macros.h
@@ -27,7 +28,6 @@ liblog_la_SOURCES += message_initializer.cc message_initializer.h
 liblog_la_SOURCES += message_reader.cc message_reader.h
 liblog_la_SOURCES += message_types.h
 liblog_la_SOURCES += output_option.cc output_option.h
-liblog_la_SOURCES += root_logger_name.cc root_logger_name.h
 
 EXTRA_DIST  = README
 EXTRA_DIST += impldef.mes

src/lib/log/logger.cc

@@ -17,9 +17,9 @@
 
 #include <log/logger.h>
 #include <log/logger_impl.h>
+#include <log/logger_name.h>
 #include <log/message_dictionary.h>
 #include <log/message_types.h>
-#include <log/root_logger_name.h>
 
 #include <util/strutil.h>
 
@@ -74,6 +74,14 @@ Logger::getDebugLevel() {
     return (getLoggerPtr()->getDebugLevel());
 }
 
+// Effective debug level (only relevant if messages of severity DEBUG are being
+// logged).
+
+int
+Logger::getEffectiveDebugLevel() {
+    return (getLoggerPtr()->getEffectiveDebugLevel());
+}
+
 // Check on the current severity settings
 
 bool

src/lib/log/logger.h

@@ -25,26 +25,66 @@
 namespace isc {
 namespace log {
 
-/// \brief Logging API
-///
-/// This module forms the interface into the logging subsystem. Features of the
-/// system and its implementation are:
-///
-/// # Multiple logging objects can be created, each given a name; those with the
-///   same name share characteristics (like destination, level being logged
-///   etc.)
-/// # Messages can be logged at severity levels of FATAL, ERROR, WARN, INFO or
-///   DEBUG.  The DEBUG level has further sub-levels numbered 0 (least
-///   informative) to 99 (most informative).
-/// # Each logger has a severity level set associated with it.  When a message
-///   is logged, it is output only if it is logged at a level equal to the
-///   logger severity level or greater, e.g. if the logger's severity is WARN,
-///   only messages logged at WARN, ERROR or FATAL will be output.
-/// # Messages are identified by message identifiers, which are keys into a
-///   message dictionary.
+/// \page LoggingApi Logging API
+/// \section LoggingApiOverview Overview
+/// BIND 10 logging uses the concepts of the widely-used Java logging
+/// package log4j (http://logging.apache.log/log4j), albeit implemented 
+/// in C++ using an open-source port.  Features of the system are:
+/// 
+/// - Within the code objects - known as loggers - can be created and
+/// used to log messages.  These loggers have names; those with the
+/// same name share characteristics (such as output destination).
+/// - Loggers have a hierarchical relationship, with each logger being
+/// the child of another logger, except for the top of the hierarchy, the
+/// root logger.  If a logger does not log a message, it is passed to the
+/// parent logger.
+/// - Messages can be logged at severity levels of FATAL, ERROR, WARN, INFO
+/// or DEBUG.  The DEBUG level has further sub-levels numbered 0 (least
+/// informative) to 99 (most informative).
+/// - Each logger has a severity level set associated with it.  When a
+/// message is logged, it is output only if it is logged at a level equal
+/// to the logger severity level or greater, e.g. if the logger's severity
+/// is WARN, only messages logged at WARN, ERROR or FATAL will be output.
+/// 
+/// \section LoggingApiLoggerNames BIND 10 Logger Names
+/// Within BIND 10, the root logger root logger is given the name of the
+/// program (via the stand-alone function setRootLoggerName()). Other loggers
+/// are children of the root logger and are named "<program>.<sublogger>".
+/// This name appears in logging output, allowing users to identify both
+/// the BIND 10 program and the component within the program that generated
+/// the message.
+/// 
+/// When creating a logger, the abbreviated name "<sublogger>" can be used;
+/// the program name will be prepended to it when the logger is created.
+/// In this way, individual libraries can have their own loggers without
+/// worrying about the program in which they are used, but:
+/// - The origin of the message will be clearly identified.
+/// - The same component can have different options (e.g. logging severity)
+/// in different programs at the same time.
+/// 
+/// \section LoggingApiLoggingMessages Logging Messages
+/// Instead of embedding the text of messages within the code, each message
+/// is referred to using a symbolic name.  The logging code uses this name as
+/// a key in a dictionary from which the message text is obtained.  Such a
+/// system allows for the optional replacement of message text at run time.
+/// More details about the message disction (and the compiler used to create
+/// the symbol definitions) can be found in other modules in the src/lib/log
+/// directory.
 
 class LoggerImpl;   // Forward declaration of the implementation class
 
+/// \brief Logger Class
+///
+/// This class is the main class used for logging.  Use comprises:
+///
+/// 1. Constructing a logger by instantiating it with a specific name. (If the
+/// same logger is in multiple functions within a file, overhead can be
+/// minimised by declaring it as a file-wide static variable.)
+/// 2. Using the error(), info() etc. methods to log an error.  (However, it is
+/// recommended to use the LOG_ERROR, LOG_INFO etc. macros defined in macros.h.
+/// These will avoid the potentially-expensive evaluation of arguments if the
+/// severity is such that the message will be suppressed.)
+
 class Logger {
 public:
 
@@ -99,6 +139,13 @@ public:
     /// whether the severity is set to debug.
     virtual int getDebugLevel();
 
+    /// \brief Get Effective Debug Level for Logger
+    ///
+    /// \return The effective debug level of the logger.  This is the same
+    /// as getDebugLevel() if the logger has a debug level set, but otherwise
+    /// is the debug level of the parent.
+    virtual int getEffectiveDebugLevel();
+
     /// \brief Returns if Debug Message Should Be Output
     ///
     /// \param dbglevel Level for which debugging is checked.  Debugging is

src/lib/log/logger_impl.cc

@@ -23,14 +23,13 @@
 
 #include <log4cplus/configurator.h>
 
-#include <log/root_logger_name.h>
 #include <log/logger.h>
+#include <log/logger_impl.h>
 #include <log/logger_level.h>
 #include <log/logger_level_impl.h>
-#include <log/logger_impl.h>
+#include <log/logger_name.h>
 #include <log/message_dictionary.h>
 #include <log/message_types.h>
-#include <log/root_logger_name.h>
 
 #include <util/strutil.h>
 
@@ -43,22 +42,14 @@ using namespace std;
 namespace isc {
 namespace log {
 
-// Constructor.  Although it may be immediately reset, logger_ is initialized to
-// the log4cplus root logger; at least one compiler requires that all member
-// variables be constructed before the constructor is run, but log4cplus::Logger
-// (the type of logger_) has no default constructor.
-LoggerImpl::LoggerImpl(const string& name) :
-    logger_(log4cplus::Logger::getRoot())
+// Constructor.  The setting of logger_ must be done when the variable is
+// constructed (instead of being left to the body of the function); at least
+// one compiler requires that all member variables be constructed before the
+// constructor is run, but log4cplus::Logger (the type of logger_) has no
+// default constructor.
+LoggerImpl::LoggerImpl(const string& name) : name_(expandLoggerName(name)),
+    logger_(log4cplus::Logger::getInstance(name_))
 {
-    // Are we the root logger?
-    if (name == getRootLoggerName()) {
-        name_ = name;
-        // logger_ already set to log4cplus root logger at this point
-
-    } else {
-        name_ = getRootLoggerName() + "." + name;
-        logger_ = log4cplus::Logger::getInstance(name);
-    }
 }
 
 // Destructor. (Here because of virtual declaration.)

src/lib/log/logger_impl.h

@@ -50,21 +50,14 @@ namespace log {
 /// documentation, but actually unnamed) and all loggers created are subloggers
 /// if it.
 ///
-/// In this implementation, the name of the logger is checked.  If it is the
-/// name of the program (as set in the call to isc::log::setRootLoggerName),
-/// the log4cplus root logger is used.  Otherwise the name passed is used as
-/// the name of a logger when a log4cplus logger is created.
+/// In this implementation, the log4cplus root logger is unused.  Instead, the
+/// BIND 10 root logger is created as a child of the log4cplus root logger,
+/// and all other loggers used in the program are created as sub-loggers of
+/// that.  In this way, the logging system can just include the name of the
+/// logger in each message without the need to specially consider if the
+/// message is the root logger or not.
 ///
-/// To clarify: if the program is "b10auth" (and that is used to set the BIND 10
-/// root logger name via a call to isc::log::setRootLoggerName()), the BIND 10
-/// logger "b10auth" corresponds to the log4cplus root logger instance (returned
-/// by a call to log4cplus::Logger::getRoot()).  The BIND 10 sub-logger "cache"
-/// corresponds to the log4cplus logger "cache", created by a call to
-/// log4cplus::Logger::getInstance("cache").  The distinction is, however,
-/// invisible to users as the logger reported in messages is always
-/// "programm.sublogger".
-///
-/// b) The idea of debug levels is implemented.  Seee logger_level.h and
+/// b) The idea of debug levels is implemented.  See logger_level.h and
 /// logger_level_impl.h for more details on this.
 
 class LoggerImpl {

src/lib/log/logger_impl_log4cxx.cc

@@ -1,242 +0,0 @@
-// Copyright (C) 2011  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 <iostream>
-
-#include <stdarg.h>
-#include <stdio.h>
-
-#include <log4cxx/appender.h>
-#include <log4cxx/basicconfigurator.h>
-#include <log4cxx/patternlayout.h>
-#include <log4cxx/consoleappender.h>
-
-#include <log/root_logger_name.h>
-#include <log/logger.h>
-#include <log/logger_impl.h>
-#include <log/message_dictionary.h>
-#include <log/message_types.h>
-#include <log/xdebuglevel.h>
-
-#include <util/strutil.h>
-
-using namespace std;
-
-namespace isc {
-namespace log {
-
-// Static initializations
-
-bool LoggerImpl::init_ = false;
-
-// Destructor.  Delete log4cxx stuff if "don't delete" is clear.
-
-LoggerImpl::~LoggerImpl() {
-    if (exit_delete_) {
-        delete loggerptr_;
-    }
-}
-
-// Initialize logger - create a logger as a child of the root logger.  With
-// log4cxx this is assured by naming the logger <parent>.<child>.
-
-void
-LoggerImpl::initLogger() {
-
-    // Initialize basic logging if not already done.  This is a one-off for
-    // all loggers.
-    if (!init_) {
-
-        // TEMPORARY
-        // Add a suitable console logger to the log4cxx root logger.  (This
-        // is the logger at the root of the log4cxx tree, not the BIND-10 root
-        // logger, which is one level down.)  The chosen format is:
-        //
-        // YYYY-MM-DD hh:mm:ss.sss [logger] SEVERITY: text
-        //
-        // As noted, this is a temporary hack: it is done here to ensure that
-        // a suitable output and output pattern is set.  Future versions of the
-        // software will set this based on configuration data.
-
-        log4cxx::LayoutPtr layout(
-            new log4cxx::PatternLayout(
-                "%d{yyyy-MM-DD HH:mm:ss.SSS} %-5p [%c] %m\n"));
-        log4cxx::AppenderPtr console(
-            new log4cxx::ConsoleAppender(layout));
-        log4cxx::LoggerPtr sys_root_logger = log4cxx::Logger::getRootLogger();
-        sys_root_logger->addAppender(console);
-
-        // Set the default logging to INFO
-        sys_root_logger->setLevel(log4cxx::Level::getInfo());
-
-        // All static stuff initialized
-        init_ = true;
-    }
-
-    // Initialize this logger.  Name this as to whether the BIND-10 root logger
-    // name has been set.  (If not, this mucks up the hierarchy :-( ).
-    string root_name = RootLoggerName::getName();
-    if (root_name.empty() || (name_ == root_name)) {
-        loggerptr_ = new log4cxx::LoggerPtr(log4cxx::Logger::getLogger(name_));
-    }
-    else {
-        loggerptr_ = new log4cxx::LoggerPtr(
-            log4cxx::Logger::getLogger(root_name + "." + name_)
-        );
-    }
-}
-
-
-// Set the severity for logging.  There is a 1:1 mapping between the logging
-// severity and the log4cxx logging levels, apart from DEBUG.
-//
-// In log4cxx, each of the logging levels (DEBUG, INFO, WARN etc.) has a numeric
-// value.  The level is set to one of these and any numeric level equal to or
-// above it that is reported.  For example INFO has a value of 20000 and ERROR
-// a value of 40000. So if a message of WARN severity (= 30000) is logged, it is
-// not logged when the logger's severity level is ERROR (as 30000 !>= 40000).
-// It is reported if the logger's severity level is set to WARN (as 30000 >=
-/// 30000) or INFO (30000 >= 20000).
-//
-// This gives a simple system for handling different debug levels.  The debug
-// level is a number between 0 and 99, with 0 being least verbose and 99 the
-// most.  To implement this seamlessly, when DEBUG is set, the numeric value
-// of the logging level is actually set to (DEBUG - debug-level).  Similarly
-// messages of level "n" are logged at a logging level of (DEBUG - n).  Thus if
-// the logging level is set to DEBUG and the debug level set to 25, the actual
-// level set is 10000 - 25 = 99975.
-//
-// Attempting to log a debug message of level 26 is an attempt to log a message
-// of level 10000 - 26 = 9974.  As 9974 !>= 9975, it is not logged.  A
-// message of level 25 is, because 9975 >= 9975.
-//
-// The extended set of logging levels is implemented by the XDebugLevel class.
-
-void
-LoggerImpl::setSeverity(isc::log::Severity severity, int dbglevel) {
-    switch (severity) {
-        case NONE:
-            getLogger()->setLevel(log4cxx::Level::getOff());
-            break;
-
-        case FATAL:
-            getLogger()->setLevel(log4cxx::Level::getFatal());
-            break;
-
-        case ERROR:
-            getLogger()->setLevel(log4cxx::Level::getError());
-            break;
-
-        case WARN:
-            getLogger()->setLevel(log4cxx::Level::getWarn());
-            break;
-
-        case INFO:
-            getLogger()->setLevel(log4cxx::Level::getInfo());
-            break;
-
-        case DEBUG:
-            getLogger()->setLevel(
-                log4cxx::XDebugLevel::getExtendedDebug(dbglevel));
-            break;
-
-        // Will get here for DEFAULT or any other value.  This disables the
-        // logger's own severity and it defaults to the severity of the parent
-        // logger.
-        default:
-            getLogger()->setLevel(0);
-    }
-}
-
-// Convert between numeric log4cxx logging level and BIND-10 logging severity.
-
-isc::log::Severity
-LoggerImpl::convertLevel(int value) {
-
-    // The order is optimised.  This is only likely to be called when testing
-    // for writing debug messages, so the check for DEBUG_INT is first.
-    if (value <= log4cxx::Level::DEBUG_INT) {
-        return (DEBUG);
-    } else if (value <= log4cxx::Level::INFO_INT) {
-        return (INFO);
-    } else if (value <= log4cxx::Level::WARN_INT) {
-        return (WARN);
-    } else if (value <= log4cxx::Level::ERROR_INT) {
-        return (ERROR);
-    } else if (value <= log4cxx::Level::FATAL_INT) {
-        return (FATAL);
-    } else {
-        return (NONE);
-    }
-}
-
-
-// Return the logging severity associated with this logger.
-
-isc::log::Severity
-LoggerImpl::getSeverityCommon(const log4cxx::LoggerPtr& ptrlogger,
-    bool check_parent) {
-
-    log4cxx::LevelPtr level = ptrlogger->getLevel();
-    if (level == log4cxx::LevelPtr()) {
-
-        // Null level returned, logging should be that of the parent.
-
-        if (check_parent) {
-            log4cxx::LoggerPtr parent = ptrlogger->getParent();
-            if (parent == log4cxx::LoggerPtr()) {
-
-                // No parent, so reached the end of the chain.  Return INFO
-                // severity.
-                return (INFO);
-            }
-            else {
-                return (getSeverityCommon(parent, check_parent));
-            }
-        }
-        else {
-            return (DEFAULT);
-        }
-    } else {
-        return (convertLevel(level->toInt()));
-    }
-}
-
-
-// Get the debug level.  This returns 0 unless the severity is DEBUG.
-
-int
-LoggerImpl::getDebugLevel() {
-
-    log4cxx::LevelPtr level = getLogger()->getLevel();
-    if (level == log4cxx::LevelPtr()) {
-
-        // Null pointer returned, logging should be that of the parent.
-        return (0);
-
-    } else {
-        int severity = level->toInt();
-        if (severity <= log4cxx::Level::DEBUG_INT) {
-            return (log4cxx::Level::DEBUG_INT - severity);
-        }
-        else {
-            return (0);
-        }
-    }
-}
-
-
-
-} // namespace log
-} // namespace isc

src/lib/log/logger_impl_log4cxx.h

@@ -1,315 +0,0 @@
-// Copyright (C) 2011  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 __LOGGER_IMPL_LOG4CXX_H
-#define __LOGGER_IMPL_LOG4CXX_H
-
-#include <cstdlib>
-#include <string>
-#include <boost/lexical_cast.hpp>
-#include <log4cxx/logger.h>
-#include <log4cxx/logger.h>
-
-#include <log/debug_levels.h>
-#include <log/logger.h>
-#include <log/message_types.h>
-
-namespace isc {
-namespace log {
-
-/// \brief Log4cxx Logger Implementation
-///
-/// The logger uses a "pimpl" idiom for implementation, where the base logger
-/// class contains little more than a pointer to the implementation class, and
-/// all actions are carried out by the latter.  This class is an implementation
-/// class interfacing to the log4cxx logging system.
-
-class LoggerImpl {
-public:
-
-    /// \brief Constructor
-    ///
-    /// Creates/attaches to a logger of a specific name.
-    ///
-    /// \param name Name of the logger.  If the name is that of the root name,
-    /// this creates an instance of the root logger; otherwise it creates a
-    /// child of the root logger.
-    ///
-    /// \param exit_delete This argument is present to get round a bug in
-    /// log4cxx.  If a log4cxx logger is declared outside an execution unit, it
-    /// is not deleted until the program runs down.  At that point all such
-    /// objects - including internal log4cxx objects - are deleted.  However,
-    /// there seems to be a bug in log4cxx where the way that such objects are
-    /// destroyed causes a MutexException to be thrown (this is described in
-    /// https://issues.apache.org/jira/browse/LOGCXX-322).  As this only occurs
-    /// during program rundown, the issue is not serious - it just looks bad to
-    /// have the program crash instead of shut down cleanly.\n
-    /// \n
-    /// The original implementation of the isc::log::Logger had as a member a
-    /// log4cxx logger (actually a LoggerPtr).  If the isc:: Logger was declared
-    /// statically, when it was destroyed at the end of the program the internal
-    /// LoggerPtr was destroyed, which triggered the problem.  The problem did
-    /// not occur if the isc::log::Logger was created on the stack.  To get
-    /// round this, the internal LoggerPtr is now created dynamically.  The
-    /// exit_delete argument controls its destruction: if true, it is destroyed
-    /// in the ISC Logger destructor.  If false, it is not.\n
-    /// \n
-    /// When creating an isc::log::Logger on the stack, the argument should be
-    /// false (the default); when the Logger is destroyed, all the internal
-    /// log4cxx objects are destroyed.  As only the logger (and not the internal
-    /// log4cxx data structures are being destroyed), all is well.  However,
-    /// when creating the logger statically, the argument should be false.  This
-    /// means that the log4cxx objects are not destroyed at program rundown;
-    /// instead memory is reclaimed and files are closed when the process is
-    /// destroyed, something that does not trigger the bug.
-    LoggerImpl(const std::string& name, bool exit_delete = false) :
-        loggerptr_(NULL), name_(name), exit_delete_(exit_delete)
-    {}
-
-
-    /// \brief Destructor
-    virtual ~LoggerImpl();
-
-
-    /// \brief Get the full name of the logger (including the root name)
-    virtual std::string getName() {
-        return (getLogger()->getName());
-    }
-
-
-    /// \brief Set Severity Level for Logger
-    ///
-    /// Sets the level at which this logger will log messages.  If none is set,
-    /// the level is inherited from the parent.
-    ///
-    /// \param severity Severity level to log
-    /// \param dbglevel If the severity is DEBUG, this is the debug level.
-    /// This can be in the range 1 to 100 and controls the verbosity.  A value
-    /// outside these limits is silently coerced to the nearest boundary.
-    virtual void setSeverity(isc::log::Severity severity, int dbglevel = 1);
-
-
-    /// \brief Get Severity Level for Logger
-    ///
-    /// \return The current logging level of this logger.  In most cases though,
-    /// the effective logging level is what is required.
-    virtual isc::log::Severity getSeverity() {
-        return (getSeverityCommon(getLogger(), false));
-    }
-
-
-    /// \brief Get Effective Severity Level for Logger
-    ///
-    /// \return The effective severity level of the logger.  This is the same
-    /// as getSeverity() if the logger has a severity level set, but otherwise
-    /// is the severity of the parent.
-    virtual isc::log::Severity getEffectiveSeverity() {
-        return (getSeverityCommon(getLogger(), true));
-    }
-
-
-    /// \brief Return DEBUG Level
-    ///
-    /// \return Current setting of debug level.  This is returned regardless of
-    /// whether the
-    virtual int getDebugLevel();
-
-
-    /// \brief Returns if Debug Message Should Be Output
-    ///
-    /// \param dbglevel Level for which debugging is checked.  Debugging is
-    /// enabled only if the logger has DEBUG enabled and if the dbglevel
-    /// checked is less than or equal to the debug level set for the logger.
-    virtual bool
-    isDebugEnabled(int dbglevel = MIN_DEBUG_LEVEL) {
-        return (getLogger()->getEffectiveLevel()->toInt() <=
-            (log4cxx::Level::DEBUG_INT - dbglevel));
-    }
-
-
-    /// \brief Is INFO Enabled?
-    virtual bool isInfoEnabled() {
-        return (getLogger()->isInfoEnabled());
-    }
-
-
-    /// \brief Is WARNING Enabled?
-    virtual bool isWarnEnabled() {
-        return (getLogger()->isWarnEnabled());
-    }
-
-
-    /// \brief Is ERROR Enabled?
-    virtual bool isErrorEnabled() {
-        return (getLogger()->isErrorEnabled());
-    }
-
-
-    /// \brief Is FATAL Enabled?
-    virtual bool isFatalEnabled() {
-        return (getLogger()->isFatalEnabled());
-    }
-
-
-    /// \brief Output Debug Message
-    ///
-    /// \param ident Message identification.
-    /// \param text Text to log
-    void debug(const MessageID& ident, const char* text) {
-        LOG4CXX_DEBUG(getLogger(), ident << ", " << text);
-    }
-
-
-    /// \brief Output Informational Message
-    ///
-    /// \param ident Message identification.
-    /// \param text Text to log
-    void info(const MessageID& ident, const char* text) {
-        LOG4CXX_INFO(getLogger(), ident << ", " << text);
-    }
-
-
-    /// \brief Output Warning Message
-    ///
-    /// \param ident Message identification.
-    /// \param text Text to log
-    void warn(const MessageID& ident, const char* text) {
-        LOG4CXX_WARN(getLogger(), ident << ", " << text);
-    }
-
-
-    /// \brief Output Error Message
-    ///
-    /// \param ident Message identification.
-    /// \param text Text to log
-    void error(const MessageID& ident, const char* text) {
-        LOG4CXX_ERROR(getLogger(), ident << ", " << text);
-    }
-
-
-    /// \brief Output Fatal Message
-    ///
-    /// \param ident Message identification.
-    /// \param text Text to log
-    void fatal(const MessageID& ident, const char* text) {
-        LOG4CXX_FATAL(getLogger(), ident << ", " << text);
-    }
-
-    //@{
-    /// \brief Testing Methods
-    ///
-    /// The next set of methods are used in testing.  As they are accessed from
-    /// the main logger class, they must be public.
-
-    /// \brief Equality
-    ///
-    /// Check if two instances of this logger refer to the same stream.
-    /// (This method is principally for testing.)
-    ///
-    /// \return true if the logger objects are instances of the same logger.
-    bool operator==(LoggerImpl& other) {
-        return (*loggerptr_ == *other.loggerptr_);
-    }
-
-
-    /// \brief Logger Initialized
-    ///
-    /// Check that the logger has been properly initialized.  (This method
-    /// is principally for testing.)
-    ///
-    /// \return true if this logger object has been initialized.
-    bool isInitialized() {
-        return (loggerptr_ != NULL);
-    }
-
-    /// \brief Reset Global Data
-    ///
-    /// Only used for testing, this clears all the logger information and
-    /// resets it back to default values.  This is a no-op for log4cxx.
-    static void reset() {
-    }
-
-    //@}
-
-protected:
-
-    /// \brief Convert Between BIND-10 and log4cxx Logging Levels
-    ///
-    /// This method is marked protected to allow for unit testing.
-    ///
-    /// \param value log4cxx numeric logging level
-    ///
-    /// \return BIND-10 logging severity
-    isc::log::Severity convertLevel(int value);
-
-private:
-
-    /// \brief Get Severity Level for Logger
-    ///
-    /// This is common code for getSeverity() and getEffectiveSeverity() -
-    /// it returns the severity of the logger; if not set (and the check_parent)
-    /// flag is set, it searches up the parent-child tree until a severity
-    /// level is found and uses that.
-    ///
-    /// \param ptrlogger Pointer to the log4cxx logger to check.
-    /// \param check_parent true to search up the tree, false to return the
-    /// current level.
-    ///
-    /// \return The effective severity level of the logger.  This is the same
-    /// as getSeverity() if the logger has a severity level set, but otherwise
-    /// is the severity of the parent.
-    isc::log::Severity getSeverityCommon(const log4cxx::LoggerPtr& ptrlogger,
-        bool check_parent);
-
-
-
-    /// \brief Initialize log4cxx Logger
-    ///
-    /// Creates the log4cxx logger used internally.  A function is provided for
-    /// this so that the creation does not take place when this Logger object
-    /// is created but when it is used.  As the latter occurs in executable
-    /// code but the former can occur during initialization, this order
-    /// guarantees that anything that is statically initialized has completed
-    /// its initialization by the time the logger is used.
-    void initLogger();
-
-
-    /// \brief Return underlying log4cxx logger, initializing it if necessary
-    ///
-    /// \return Loggerptr object
-    log4cxx::LoggerPtr& getLogger() {
-        if (loggerptr_ == NULL) {
-            initLogger();
-        }
-        return (*loggerptr_);
-    }
-
-    // Members.  Note that loggerptr_ is a pointer to a LoggerPtr, which is
-    // itself a pointer to the underlying log4cxx logger.  This is due to the
-    // problems with memory deletion on program exit, explained in the comments
-    // for the "exit_delete" parameter in this class's constructor.
-
-    log4cxx::LoggerPtr*  loggerptr_;    ///< Pointer to the underlying logger
-    std::string          name_;         ///< Name of this logger]
-    bool                 exit_delete_;  ///< Delete loggerptr_ on exit?
-
-    // NOTE - THIS IS A PLACE HOLDER
-    static bool         init_;      ///< Set true when initialized
-};
-
-} // namespace log
-} // namespace isc
-
-
-#endif // __LOGGER_IMPL_LOG4CXX_H

src/lib/log/logger_level.cc

@@ -34,6 +34,8 @@ getSeverity(const std::string& sev_str) {
         return isc::log::ERROR;
     } else if (boost::iequals(sev_str, "FATAL")) {
         return isc::log::FATAL;
+    } else if (boost::iequals(sev_str, "NONE")) {
+        return isc::log::NONE;
     } else {
         Logger logger("log");
         LOG_ERROR(logger, MSG_BADSEVERITY).arg(sev_str);

src/lib/log/logger_level.h

@@ -58,16 +58,16 @@ struct Level {
     // Default assignment and copy constructor is appropriate
 };
 
-/// \brief Returns the isc::log::Severity value represented by the
-///        given string
+/// \brief Returns the isc::log::Severity value represented by the given string
 ///
-/// If the string is not recognized, returns isc::log::DEBUG.
-/// This must be one of the strings "DEBUG", "INFO", "WARN", "ERROR",
-/// "FATAL". (Must be upper case and must not contain leading or
+/// This must be one of the strings "DEBUG", "INFO", "WARN", "ERROR", "FATAL" or
+/// "NONE". (Case is not important, but the string most not contain leading or
 /// trailing spaces.)
 ///
 /// \param sev_str The string representing severity value
-/// \return The severity
+///
+/// \return The severity. If the string is not recognized, an error will be
+///         logged and the string will return  isc::log::INFO.
 isc::log::Severity getSeverity(const std::string& sev_str);
 
 }   // namespace log

src/lib/log/logger_manager.cc

@@ -18,13 +18,13 @@
 #include <log/logger.h>
 #include <log/logger_manager_impl.h>
 #include <log/logger_manager.h>
+#include <log/logger_name.h>
 #include <log/messagedef.h>
 #include <log/message_dictionary.h>
 #include <log/message_exception.h>
 #include <log/message_initializer.h>
 #include <log/message_reader.h>
 #include <log/message_types.h>
-#include <log/root_logger_name.h>
 #include <log/macros.h>
 #include <log/messagedef.h>
 #include <log/message_initializer.h>
@@ -35,8 +35,28 @@ namespace {
 
 // Logger used for logging messages within the logging code itself.
 isc::log::Logger logger("log");
+
+// Static stores for the initialization severity and debug level.
+// These are put in methods to avoid a "static initialization fiasco".
+
+isc::log::Severity& initSeverity() {
+    static isc::log::Severity severity = isc::log::INFO;
+    return (severity);
 }
 
+int& initDebugLevel() {
+    static int dbglevel = 0;
+    return (dbglevel);
+}
+
+std::string& initRootName() {
+    static std::string root("bind10");
+    return (root);
+}
+
+} // Anonymous namespace
+
+
 namespace isc {
 namespace log {
 
@@ -72,9 +92,16 @@ LoggerManager::processEnd() {
 /// Logging system initialization
 
 void
-LoggerManager::init(const std::string& root, const char* file,
-                    isc::log::Severity severity, int dbglevel)
+LoggerManager::init(const std::string& root, isc::log::Severity severity,
+                    int dbglevel, const char* file)
 {
+    // Save name, severity and debug level for later.  No need to save the
+    // file name as once the local message file is read the messages will
+    // not be lost.
+    initRootName() = root;
+    initSeverity() = severity;
+    initDebugLevel() = dbglevel;
+
     // Create the BIND 10 root logger and set the default severity and
     // debug level.  This is the logger that has the name of the application.
     // All other loggers created in this application will be its children.
@@ -145,10 +172,11 @@ LoggerManager::readLocalMessageFile(const char* file) {
     }
 }
 
-// Reset logging
+// Reset logging to settings passed to init()
 void
 LoggerManager::reset() {
-    LoggerManagerImpl::reset();
+    setRootLoggerName(initRootName());
+    LoggerManagerImpl::reset(initSeverity(), initDebugLevel());
 }
 
 } // namespace log

src/lib/log/logger_manager.h

@@ -87,18 +87,17 @@ public:
     ///
     /// \param root Name of the root logger.  This should be set to the name of
     ///        the program.
-    /// \param file Name of the local message file.  This must be NULL if there
-    ///        is no local message file.
     /// \param severity Severity at which to log
     /// \param dbglevel Debug severity (ignored if "severity" is not "DEBUG")
-    static void init(const std::string& root, const char* file = NULL,
+    /// \param file Name of the local message file.  This must be NULL if there
+    ///        is no local message file.
+    static void init(const std::string& root,
                     isc::log::Severity severity = isc::log::INFO,
-                    int dbglevel = 0);
+                    int dbglevel = 0, const char* file = NULL);
 
     /// \brief Reset logging
     ///
-    /// Resets logging to default (just the root logger output INFO or above
-    /// messages to the console.
+    /// Resets logging to whatever was set in the call to init().
     static void reset();
 
     /// \brief Read local message file

src/lib/log/logger_manager_impl.cc

@@ -19,14 +19,14 @@
 #include <log4cplus/configurator.h>
 #include <log4cplus/consoleappender.h>
 #include <log4cplus/fileappender.h>
+#include <log4cplus/syslogappender.h>
 
+#include "log/logger.h"
 #include "log/logger_level_impl.h"
 #include "log/logger_manager.h"
 #include "log/logger_manager_impl.h"
+#include "log/logger_name.h"
 #include "log/logger_specification.h"
-#include "log/root_logger_name.h"
-
-#include "log/logger.h"
 #include "log/messagedef.h"
 
 using namespace std;
@@ -52,9 +52,8 @@ LoggerManagerImpl::processInit() {
 void
 LoggerManagerImpl::processSpecification(const LoggerSpecification& spec) {
 
-    log4cplus::Logger logger = (spec.getName() == getRootLoggerName()) ?
-                               log4cplus::Logger::getRoot() :
-                               log4cplus::Logger::getInstance(spec.getName());
+    log4cplus::Logger logger = log4cplus::Logger::getInstance(
+                                   expandLoggerName(spec.getName()));
 
     // Set severity level according to specification entry.
     logger.setLogLevel(LoggerLevelImpl::convertFromBindLevel(
@@ -134,6 +133,17 @@ LoggerManagerImpl::createFileAppender(log4cplus::Logger& logger,
     logger.addAppender(fileapp);
 }
 
+// Syslog appender. 
+void
+LoggerManagerImpl::createSyslogAppender(log4cplus::Logger& logger,
+                                         const OutputOption& opt)
+{
+    log4cplus::SharedAppenderPtr syslogapp(
+        new log4cplus::SysLogAppender(opt.facility));
+    setSyslogAppenderLayout(syslogapp);
+    logger.addAppender(syslogapp);
+}
+
 
 // One-time initialization of the log4cplus system
 
@@ -150,17 +160,17 @@ LoggerManagerImpl::init(isc::log::Severity severity, int dbglevel) {
     // Add the additional debug levels
     LoggerLevelImpl::init();
 
-    reset();
+    reset(severity, dbglevel);
 }
 
 // Reset logging to default configuration.  This closes all appenders
 // and resets the root logger to output INFO messages to the console.
 // It is principally used in testing.
 void
-LoggerManagerImpl::reset() {
+LoggerManagerImpl::reset(isc::log::Severity severity, int dbglevel) {
 
     // Initialize the root logger
-    initRootLogger();
+    initRootLogger(severity, dbglevel);
 }
 
 // Initialize the root logger
@@ -169,14 +179,20 @@ void LoggerManagerImpl::initRootLogger(isc::log::Severity severity,
 {
     log4cplus::Logger::getDefaultHierarchy().resetConfiguration();
 
-    // Set the severity for the root logger
-    log4cplus::Logger::getRoot().setLogLevel(
-            LoggerLevelImpl::convertFromBindLevel(Level(severity, dbglevel)));
+    // Set the log4cplus root to not output anything - effectively we are
+    // ignoring it.
+    log4cplus::Logger::getRoot().setLogLevel(log4cplus::OFF_LOG_LEVEL);
+
+    // Set the level for the BIND 10 root logger to the given severity and
+    // debug level.
+    log4cplus::Logger b10root = log4cplus::Logger::getInstance(
+                                                    getRootLoggerName());
+    b10root.setLogLevel(LoggerLevelImpl::convertFromBindLevel(
+                                                    Level(severity, dbglevel)));
 
-    // Set the root to use a console logger.
+    // Set the BIND 10 root to use a console logger.
     OutputOption opt;
-    log4cplus::Logger root = log4cplus::Logger::getRoot();
-    createConsoleAppender(root, opt);
+    createConsoleAppender(b10root, opt);
 }
 
 // Set the the "console" layout for the given appenders.  This layout includes
@@ -186,8 +202,22 @@ void LoggerManagerImpl::setConsoleAppenderLayout(
         log4cplus::SharedAppenderPtr& appender)
 {
     // Create the pattern we want for the output - local time.
-    string pattern = "%D{%Y-%m-%d %H:%M:%S.%q} %-5p [";
-    pattern += getRootLoggerName() + string(".%c] %m\n");
+    string pattern = "%D{%Y-%m-%d %H:%M:%S.%q} %-5p [%c] %m\n";
+
+    // Finally the text of the message
+    auto_ptr<log4cplus::Layout> layout(new log4cplus::PatternLayout(pattern));
+    appender->setLayout(layout);
+}
+
+// Set the the "syslog" layout for the given appenders.  This is the same
+// as the console, but without the timestamp (which is expected to be
+// set by syslogd).
+
+void LoggerManagerImpl::setSyslogAppenderLayout(
+        log4cplus::SharedAppenderPtr& appender)
+{
+    // Create the pattern we want for the output - local time.
+    string pattern = "%-5p [%c] %m\n";
 
     // Finally the text of the message
     auto_ptr<log4cplus::Layout> layout(new log4cplus::PatternLayout(pattern));

src/lib/log/logger_manager_impl.h

@@ -96,7 +96,11 @@ public:
     ///
     /// Resets to default configuration (root logger logging to the console
     /// with INFO severity).
-    static void reset();
+    ///
+    /// \param severity Severity to be associated with this logger
+    /// \param dbglevel Debug level associated with the root logger
+    static void reset(isc::log::Severity severity = isc::log::INFO,
+                      int dbglevel = 0);
 
 private:
     /// \brief Create console appender
@@ -128,7 +132,7 @@ private:
     /// \param logger Log4cplus logger to which the appender must be attached.
     /// \param opt Output options for this appender.
     static void createSyslogAppender(log4cplus::Logger& logger,
-                                     const OutputOption& opt) {}
+                                     const OutputOption& opt);
 
     /// \brief Set default layout and severity for root logger
     ///
@@ -145,11 +149,20 @@ private:
     /// Sets the layout of the specified appender to one suitable for file
     /// or console output:
     ///
-    /// YYYY-MM-DD HH:MM:SS.ssss <severity> [root.logger] message
+    /// YYYY-MM-DD HH:MM:SS.ssss SEVERITY [root.logger] message
     ///
     /// \param appender Appender for which this pattern is to be set.
-    /// \param root_name Name of the BIND 10 root logger.
     static void setConsoleAppenderLayout(log4cplus::SharedAppenderPtr& appender);
+
+    /// \brief Set layout for syslog appender
+    ///
+    /// Sets the layout of the specified appender to one suitable for the
+    /// syslog file:
+    ///
+    /// SEVERITY [root.logger] message
+    ///
+    /// \param appender Appender for which this pattern is to be set.
+    static void setSyslogAppenderLayout(log4cplus::SharedAppenderPtr& appender);
 };
 
 } // namespace log

src/lib/log/root_logger_name.cc

@@ -13,7 +13,7 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <string>
-#include <root_logger_name.h>
+#include "log/logger_name.h"
 
 namespace isc {
 namespace log {
@@ -40,5 +40,20 @@ const std::string& getRootLoggerName() {
     return (getRootLoggerNameInternal());
 }
 
+std::string expandLoggerName(const std::string& name) {
+
+    // Are we the root logger, or does the logger name start with
+    // the string "<root_logger_name>.".  If so, use a logger
+    // whose name is the one given.
+    if ((name == getRootLoggerName()) ||
+        (name.find(getRootLoggerName() + std::string(".")) == 0)) {
+        return (name);
+
+    } 
+
+    // Anything else is assumed to be a sub-logger of the root logger.
+    return (getRootLoggerName() + "." + name);
+}
+
 } // namespace log
 } // namespace isc

src/lib/log/root_logger_name.h

@@ -12,22 +12,22 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#ifndef __ROOT_LOGGER_NAME_H
-#define __ROOT_LOGGER_NAME_H
+#ifndef __LOGGER_NAME_H
+#define __LOGGER_NAME_H
 
 #include <string>
 
 /// \brief Define Name of Root Logger
 ///
 /// In BIND-10, the name root logger of a program is the name of the program
-/// itself (in contrast to packages such as log4cxx where the root logger name
-//  is something like ".").  These trivial functions allow the setting and
+/// itself (in contrast to packages such as log4cplus where the root logger name
+//  is something like "root").  These trivial functions allow the setting and
 // getting of that name by the logger classes.
 
 namespace isc {
 namespace log {
 
-/// \brief Set Root Logger Name
+/// \brief Set root logger name
 ///
 /// This function should be called by the program's initialization code before
 /// any logging functions are called.
@@ -35,12 +35,23 @@ namespace log {
 /// \param name Name of the root logger.  This should be the program name.
 void setRootLoggerName(const std::string& name);
 
-/// \brief Get Root Logger Name
+/// \brief Get root logger name
 ///
 /// \return Name of the root logger.
 const std::string& getRootLoggerName();
 
+/// \brief Expand logger name
+///
+/// Given a logger name, returns the fully-expanded logger name.  If the name
+/// starts with the root logger name, it is returned as-is.  Otherwise it is
+/// prefixed with the root logger name.
+///
+/// \param name Name to expand.
+///
+/// \return Fully-expanded logger name.
+std::string expandLoggerName(const std::string& name);
+
 }
 }
 
-#endif // __ROOT_LOGGER_NAME_H
+#endif // __LOGGER_NAME_H

src/lib/log/logger_support.cc

@@ -47,15 +47,15 @@ Logger logger("log");
 void
 initLogger(const string& root, isc::log::Severity severity, int dbglevel,
     const char* file) {
-    LoggerManager::init(root, file, severity, dbglevel);
+    LoggerManager::init(root, severity, dbglevel, file);
 }
 
 /// Logger Run-Time Initialization via Environment Variables
-void initLogger() {
+void initLogger(isc::log::Severity severity, int dbglevel) {
 
     // Root logger name is defined by the environment variable B10_LOGGER_ROOT.
-    // If not present, the name is "b10root".
-    const char* DEFAULT_ROOT = "b10root";
+    // If not present, the name is "bind10".
+    const char* DEFAULT_ROOT = "bind10";
     const char* root = getenv("B10_LOGGER_ROOT");
     if (! root) {
         root = DEFAULT_ROOT;
@@ -65,7 +65,6 @@ void initLogger() {
     // B10_LOGGER_SEVERITY, and can be one of "DEBUG", "INFO", "WARN", "ERROR"
     // of "FATAL".  Note that the string must be in upper case with no leading
     // of trailing blanks.
-    isc::log::Severity severity = isc::log::INFO;
     const char* sev_char = getenv("B10_LOGGER_SEVERITY");
     if (sev_char) {
         severity = isc::log::getSeverity(sev_char);
@@ -73,7 +72,6 @@ void initLogger() {
 
     // If the severity is debug, get the debug level (environment variable
     // B10_LOGGER_DBGLEVEL), which should be in the range 0 to 99.
-    int dbglevel = 0;
     if (severity == isc::log::DEBUG) {
         const char* dbg_char = getenv("B10_LOGGER_DBGLEVEL");
         if (dbg_char) {

src/lib/log/logger_support.h

@@ -49,19 +49,20 @@ void initLogger(const std::string& root,
 /// environment variables.  These are:
 ///
 /// B10_LOGGER_ROOT
-/// Name of the root logger.  If not given, the string "b10root" will be used.
+/// Name of the root logger.  If not given, the string "bind10" will be used.
 ///
 /// B10_LOGGER_SEVERITY
 /// Severity of messages that will be logged.  This must be one of the strings
-/// "DEBUG", "INFO", "WARN", "ERROR", "FATAL". (Must be upper case and must
-/// not contain leading or trailing spaces.)  If not specified (or if
-/// specified but incorrect), the default for the logging system will be used
-/// (currently INFO).
+/// "DEBUG", "INFO", "WARN", "ERROR", "FATAL" or "NONE". (Must be upper case
+/// and must not contain leading or trailing spaces.)  If not specified (or if
+/// specified but incorrect), the default passed as argument to this function
+/// (currently INFO) will be used.
 ///
 /// B10_LOGGER_DBGLEVEL
 /// Ignored if the level is not DEBUG, this should be a number between 0 and
 /// 99 indicating the logging severity.  The default is 0.  If outside these
-/// limits or if not a number, a value of 0 is used.
+/// limits or if not a number, The value passed to this function (default
+/// of 0) is used.
 ///
 /// B10_LOGGER_LOCALMSG
 /// If defined, the path specification of a file that contains message
@@ -71,7 +72,8 @@ void initLogger(const std::string& root,
 ///
 /// This function is most likely to be called from unit test programs.
 
-void initLogger();
+void initLogger(isc::log::Severity severity = isc::log::INFO,
+                int dbglevel = 0);
 
 } // namespace log
 } // namespace isc

src/lib/log/output_option.h

@@ -60,8 +60,8 @@ struct OutputOption {
 
     /// \brief Constructor
     OutputOption() : destination(DEST_CONSOLE), stream(STR_STDERR),
-                     flush(false), facility(""), filename(""), maxsize(0),
-                     maxver(0)
+                     flush(false), facility("LOCAL0"), filename(""),
+                     maxsize(0), maxver(0)
     {}
 
     /// Members. 
@@ -72,7 +72,7 @@ struct OutputOption {
     std::string     facility;           ///< syslog facility
     std::string     filename;           ///< Filename if file output
     size_t          maxsize;            ///< 0 if no maximum size
-    int             maxver;             ///< Maximum versions (none if <= 0)
+    unsigned int    maxver;             ///< Maximum versions (none if <= 0)
 };
 
 OutputOption::Destination getDestination(const std::string& dest_str);

src/lib/log/tests/Makefile.am

@@ -18,14 +18,14 @@ run_unittests_SOURCES += log_formatter_unittest.cc
 run_unittests_SOURCES += logger_level_impl_unittest.cc
 run_unittests_SOURCES += logger_level_unittest.cc
 run_unittests_SOURCES += logger_manager_unittest.cc
+run_unittests_SOURCES += logger_name_unittest.cc
 run_unittests_SOURCES += logger_unittest.cc
 run_unittests_SOURCES += logger_specification_unittest.cc
-# run_unittests_SOURCES += message_dictionary_unittest.cc
-# run_unittests_SOURCES += message_initializer_unittest_2.cc
-# run_unittests_SOURCES += message_initializer_unittest.cc
-# run_unittests_SOURCES += message_reader_unittest.cc
-# run_unittests_SOURCES += output_option_unittest.cc
-# run_unittests_SOURCES += root_logger_name_unittest.cc
+run_unittests_SOURCES += message_dictionary_unittest.cc
+run_unittests_SOURCES += message_initializer_unittest_2.cc
+run_unittests_SOURCES += message_initializer_unittest.cc
+run_unittests_SOURCES += message_reader_unittest.cc
+run_unittests_SOURCES += output_option_unittest.cc
 
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES) $(LOG4CPLUS_INCLUDES)
 run_unittests_LDFLAGS  = $(AM_LDFLAGS)  $(GTEST_LDFLAGS)
@@ -38,12 +38,13 @@ run_unittests_CXXFLAGS += -Wno-unused-variable
 endif
 run_unittests_LDADD  = $(GTEST_LDADD)
 run_unittests_LDADD += $(top_builddir)/src/lib/log/liblog.la
+run_unittests_LDADD += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la
 run_unittests_LDADD += $(top_builddir)/src/lib/util/libutil.la
 run_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
 run_unittests_LDADD += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la
 endif
 
-TESTS += logger_example
+check_PROGRAMS = logger_example
 logger_example_SOURCES = logger_example.cc
 logger_example_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 logger_example_LDFLAGS = $(AM_LDFLAGS) $(LOG4CPLUS_LDFLAGS)
@@ -56,5 +57,6 @@ noinst_PROGRAMS = $(TESTS)
 PYTESTS = console_test.sh local_file_test.sh severity_test.sh
 check-local:
 	$(SHELL) $(abs_builddir)/console_test.sh
+	$(SHELL) $(abs_builddir)/destination_test.sh
 	$(SHELL) $(abs_builddir)/local_file_test.sh
 	$(SHELL) $(abs_builddir)/severity_test.sh

src/lib/log/tests/console_test.sh.in

@@ -28,35 +28,33 @@ tempfile=@abs_builddir@/console_test_tempfile_$$
 passfail() {
     count=`wc -l $tempfile | awk '{print $1}'`
     if [ $count -eq $1 ]; then
-        echo " -- pass"
+        echo " pass"
     else
-        echo " ** FAIL"
+        echo " FAIL"
         failcount=`expr $failcount + $1`
     fi
 }
 
-echo "1. Checking that console output to stdout goes to stdout:"
+echo -n "1. Checking that console output to stdout goes to stdout:"
 rm -f $tempfile
-./logger_example -c stdout -s error -c stdout 1> $tempfile
-passfail 2
+./logger_example -c stdout -s error 1> $tempfile 2> /dev/null
+passfail 4
 
-echo "2. Checking that console output to stdout does not go to stderr:"
+echo -n "2. Checking that console output to stdout does not go to stderr:"
 rm -f $tempfile
-./logger_example -c stdout -s error -c stdout 2> $tempfile
+./logger_example -c stdout -s error 1> /dev/null 2> $tempfile
 passfail 0
 
-echo "3. Checking that console output to stderr goes to stderr:"
+echo -n "3. Checking that console output to stderr goes to stderr:"
 rm -f $tempfile
-./logger_example -c stdout -s error -c stderr 2> $tempfile
-passfail 2
+./logger_example -c stderr -s error 1> /dev/null 2> $tempfile
+passfail 4
 
-echo "4. Checking that console output to stderr does not go to stdout:"
+echo -n "4. Checking that console output to stderr does not go to stdout:"
 rm -f $tempfile
-./logger_example -c stdout -s error -c stderr 1> $tempfile
+./logger_example -c stderr -s error 1> $tempfile 2> /dev/null
 passfail 0
 
-rm -f $tempfile
-
 if [ $failcount -eq 0 ]; then
     echo "PASS: $testname"
 elif [ $failcount -eq 1 ]; then
@@ -65,4 +63,7 @@ else
     echo "FAIL: $testname - $failcount tests failed"
 fi
 
+# Tidy up
+rm -f $tempfile
+
 exit $failcount

src/lib/log/tests/destination_test.sh.in

@@ -0,0 +1,94 @@
+#!/bin/sh
+# Copyright (C) 2011  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.
+
+# \brief Severity test
+#
+# Checks that the logger will limit the output of messages less severy than
+# the severity/debug setting.
+
+testname="Destination test"
+echo $testname
+
+failcount=0
+tempfile=@abs_builddir@/destination_test_tempfile_$$
+destfile1=@abs_builddir@/destination_test_destfile_1_$$
+destfile2=@abs_builddir@/destination_test_destfile_2_$$
+
+passfail() {
+    if [ $1 -eq 0 ]; then
+        echo " pass"
+    else
+        echo " FAIL"
+        failcount=`expr $failcount + $1`
+    fi
+}
+
+echo "1. One logger, multiple destinations:"
+cat > $tempfile << .
+FATAL [example] MSG_WRITERR, error writing to test1: 42
+ERROR [example] MSG_RDLOCMES, reading local message file dummy/file
+FATAL [example.beta] MSG_BADSEVERITY, unrecognized log severity: beta_fatal
+ERROR [example.beta] MSG_BADDESTINATION, unrecognized log destination: beta_error
+.
+rm -f $destfile1 $destfile2
+./logger_example -s error -f $destfile1 -f $destfile2
+
+echo -n  "   - destination 1:"
+cut -d' ' -f3- $destfile1 | diff $tempfile -
+passfail $?
+
+echo -n  "   - destination 2:"
+cut -d' ' -f3- $destfile2 | diff $tempfile -
+passfail $?
+
+echo     "2. Two loggers, different destinations and severities"
+rm -f $destfile1 $destfile2
+./logger_example -l example -s info -f $destfile1 -l alpha -s warn -f $destfile2
+
+# All output for example and example.beta should have gone to destfile1.
+# Output for example.alpha should have done to destfile2.
+
+cat > $tempfile << .
+FATAL [example] MSG_WRITERR, error writing to test1: 42
+ERROR [example] MSG_RDLOCMES, reading local message file dummy/file
+WARN  [example] MSG_BADSTREAM, bad log console output stream: example
+FATAL [example.beta] MSG_BADSEVERITY, unrecognized log severity: beta_fatal
+ERROR [example.beta] MSG_BADDESTINATION, unrecognized log destination: beta_error
+WARN  [example.beta] MSG_BADSTREAM, bad log console output stream: beta_warn
+INFO  [example.beta] MSG_READERR, error reading from message file beta: info
+.
+echo -n  "   - destination 1:"
+cut -d' ' -f3- $destfile1 | diff $tempfile -
+passfail $?
+
+echo -n  "   - destination 2:"
+cat > $tempfile << .
+WARN  [example.alpha] MSG_READERR, error reading from message file a.txt: dummy reason
+.
+cut -d' ' -f3- $destfile2 | diff $tempfile -
+passfail $?
+
+if [ $failcount -eq 0 ]; then
+    echo "PASS: $testname"
+elif [ $failcount -eq 1 ]; then
+    echo "FAIL: $testname - 1 test failed"
+else
+    echo "FAIL: $testname - $failcount tests failed"
+fi
+
+# Tidy up.
+rm -f $tempfile $destfile1 $destfile2
+
+exit $failcount

src/lib/log/tests/local_file_test.sh.in

@@ -27,9 +27,9 @@ tempfile=@abs_builddir@/run_time_init_test_tempfile_$$
 
 passfail() {
     if [ $1 -eq 0 ]; then
-        echo " -- pass"
+        echo " pass"
     else
-        echo " ** FAIL"
+        echo " FAIL"
         failcount=`expr $failcount + $1`
     fi
 }
@@ -43,31 +43,35 @@ cat > $localmes << .
 % RDLOCMES    replacement read local message file, parameter is '%1'
 .
 
-echo "1. Local message replacement:"
+echo -n "1. Local message replacement:"
 cat > $tempfile << .
-WARN  [alpha.log] MSG_IDNOTFND, could not replace message text for 'MSG_NOTHERE': no such message
-FATAL [alpha.example] MSG_WRITERR, error writing to test1: 42
-ERROR [alpha.example] MSG_RDLOCMES, replacement read local message file, parameter is 'dummy/file'
-WARN  [alpha.dlm] MSG_READERR, replacement read error, parameters: 'a.txt' and 'dummy reason'
+WARN  [example.log] MSG_IDNOTFND, could not replace message text for 'MSG_NOTHERE': no such message
+FATAL [example] MSG_WRITERR, error writing to test1: 42
+ERROR [example] MSG_RDLOCMES, replacement read local message file, parameter is 'dummy/file'
+WARN  [example] MSG_BADSTREAM, bad log console output stream: example
+WARN  [example.alpha] MSG_READERR, replacement read error, parameters: 'a.txt' and 'dummy reason'
+FATAL [example.beta] MSG_BADSEVERITY, unrecognized log severity: beta_fatal
+ERROR [example.beta] MSG_BADDESTINATION, unrecognized log destination: beta_error
+WARN  [example.beta] MSG_BADSTREAM, bad log console output stream: beta_warn
 .
 ./logger_example -c stdout -s warn $localmes | cut -d' ' -f3- | diff $tempfile -
 passfail $?
 
-echo "2. Report error if unable to read local message file:"
+echo -n "2. Report error if unable to read local message file:"
 cat > $tempfile << .
-ERROR [alpha.log] MSG_OPENIN, unable to open message file $localmes for input: No such file or directory
-FATAL [alpha.example] MSG_WRITERR, error writing to test1: 42
-ERROR [alpha.example] MSG_RDLOCMES, reading local message file dummy/file
-WARN  [alpha.dlm] MSG_READERR, error reading from message file a.txt: dummy reason
+ERROR [example.log] MSG_OPENIN, unable to open message file $localmes for input: No such file or directory
+FATAL [example] MSG_WRITERR, error writing to test1: 42
+ERROR [example] MSG_RDLOCMES, reading local message file dummy/file
+WARN  [example] MSG_BADSTREAM, bad log console output stream: example
+WARN  [example.alpha] MSG_READERR, error reading from message file a.txt: dummy reason
+FATAL [example.beta] MSG_BADSEVERITY, unrecognized log severity: beta_fatal
+ERROR [example.beta] MSG_BADDESTINATION, unrecognized log destination: beta_error
+WARN  [example.beta] MSG_BADSTREAM, bad log console output stream: beta_warn
 .
 rm -f $localmes
 ./logger_example -c stdout -s warn $localmes | cut -d' ' -f3- | diff $tempfile -
 passfail $?
 
-# Tidy up.
-
-rm -f $tempfile
-
 if [ $failcount -eq 0 ]; then
     echo "PASS: $testname"
 elif [ $failcount -eq 1 ]; then
@@ -76,4 +80,7 @@ else
     echo "FAIL: $testname - $failcount tests failed"
 fi
 
+# Tidy up.
+rm -f $tempfile
+
 exit $failcount

src/lib/log/tests/logger_example.cc

@@ -28,15 +28,16 @@
 
 #include <iostream>
 #include <string>
+#include <vector>
 
 #include <util/strutil.h>
 
 #include <log/logger.h>
 #include <log/logger_level.h>
 #include <log/logger_manager.h>
+#include <log/logger_name.h>
 #include <log/logger_specification.h>
 #include <log/macros.h>
-#include <log/root_logger_name.h>
 
 // Include a set of message definitions.
 #include <log/messagedef.h>
@@ -49,23 +50,54 @@ using namespace std;
 
 void usage() {
     cout <<
-"logger_support_test [-h] [-c stream] [-d dbglevel] [-f file]\n"
-"                    [-s severity] [localfile]\n"
+"logger_support_test [-h | [logger_spec] [[logger_spec]...]]\n"
 "\n"
-"   -h              Print this message and exit\n"
+"     -h            Print this message and exit\n"
+"\n"
+"The rest of the command line comprises the set of logger specifications.\n"
+"Each specification is of the form:\n"
+"\n"
+"    -l logger  [-s severity] [-d dbglevel] output_spec] [[output_spec] ...\n"
+"\n"
+"where:\n"
+"\n"
+"    -l logger      Give the name of the logger to which the following\n"
+"                   output specifications will apply.\n"
+"\n"
+"Each logger is followed by the indication of the serverity it is logging\n"
+"and, if applicable, its debug level:\n"
 "\n"
 "   -d dbglevel     Debug level.  Only interpreted if the severity is 'debug'\n"
 "                   this is a number between 0 and 99.\n"
-"   -c stream       Send output to the console.  'stream' is one of 'stdout'\n"
-"                   of 'stderr'.  The '-c' switch is incompatible with '-f'\n"
-"                   and '-l'\n"
-"   -f file         Send output to specified file, appending to existing file\n"
-"                   if one exists.  Incompatible with -c and -l switches.\n"
 "   -s severity     Set the severity of messages output.  'severity' is one\n"
 "                   of 'debug', 'info', 'warn', 'error', 'fatal', the default\n"
 "                   being 'info'.\n"
 "\n"
-"If none of -c, -f or -l is given, by default, output is sent to stdout\n";
+"The output specifications - there may be more than one per logger - detail\n"
+"the output streams attached to the logger.  These are of the form:\n"
+"\n"
+"   -c stream | -f file [-m maxver] [-z maxsize] | -y facility\n"
+"\n"
+"These are:\n"
+"\n"
+"   -c stream       Send output to the console.  'stream' is one of 'stdout'\n"
+"                   of 'stderr'.\n"
+"   -f file         Send output to specified file, appending to existing file\n"
+"                   if one exists.\n"
+"   -y facility     Send output to the syslog file with the given facility\n"
+"                   name (e.g. local1, cron etc.)\n"
+"\n"
+"The following can be specified for the file logger:\n"
+"\n"
+"   -m maxver       If file rolling is selected (by the maximum file size being\n"
+"                   non-zero), the maximum number of versions to keep (defaults\n"
+"                   to 0)\n"
+"   -z maxsize      Maximum size of the file before the file is closed and a\n"
+"                   new one opened.  The default of 0 means no maximum size.\n"
+"\n"
+"If none of -c, -f or -y is given, by default, output is sent to stdout.  If no\n"
+"logger is specified, the default is the program's root logger ('example').\n";
+
 }
 
 
@@ -73,37 +105,57 @@ void usage() {
 // messages.  Looking at the output determines whether the program worked.
 
 int main(int argc, char** argv) {
-    const string ROOT_NAME = "alpha";
+    const string ROOT_NAME = "example";
 
+    bool                sw_found = false;   // Set true if switch found
     bool                c_found = false;    // Set true if "-c" found
     bool                f_found = false;    // Set true if "-f" found
-    bool                l_found = false;    // Set true if "-l" found
-
+    bool                y_found = false;    // Set true if "-y" found
     int                 option;             // For getopt() processing
-
-    LoggerSpecification spec(ROOT_NAME);    // Logger specification
-    OutputOption        outopt;             // Logger output option
-
-    // Initialize loggers (to set the root name and initialize logging);
-    LoggerManager::init(ROOT_NAME);
-
-    // Parse options
-    while ((option = getopt(argc, argv, "hc:d:f:s:")) != -1) {
+    OutputOption        def_opt;            // Default output option - used
+                                            //    for initialization
+    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.
+    LoggerManager manager;
+    manager.init(ROOT_NAME);
+
+    // In the parsing loop that follows, the construction of the logging
+    // specification is always "one behind".  In other words, the parsing of
+    // command-line options updates thge current logging specification/output
+    // options.  When the flag indicating a new logger or output specification
+    // is encountered, the previous one is added to the list.
+    //
+    // One complication is that there is deemed to be a default active when
+    // the parsing starts (console output for the BIND 10 root logger).  This
+    // is included in the logging specifications UNLESS the first switch on
+    // the command line is a "-l" flag starting a new logger.  To track this,
+    // the "sw_found" flag is set when a switch is completey processed. The
+    // processing of "-l" will only add information for a previous logger to
+    // the list if this flag is set.
+    while ((option = getopt(argc, argv, "hc:d:f:l:m:s:y:z:")) != -1) {
         switch (option) {
-        case 'c':
-            if (f_found || l_found) {
-                cerr << "Cannot specify -c with -f or -l\n";
-                return (1);
+        case 'c':   // Console output
+            // New output spec.  If one was currently active, add it to the
+            // list and reset the current output option to the defaults.
+            if (c_found || f_found || y_found) {
+                cur_spec.addOutputOption(cur_opt);
+                cur_opt = def_opt;
+                c_found = f_found = y_found = false;
             }
 
+            // Set the output option for this switch.
             c_found = true;
-            outopt.destination = OutputOption::DEST_CONSOLE;
-
+            cur_opt.destination = OutputOption::DEST_CONSOLE;
             if (strcmp(optarg, "stdout") == 0) {
-                outopt.stream = OutputOption::STR_STDOUT;
+                cur_opt.stream = OutputOption::STR_STDOUT;
 
             } else if (strcmp(optarg, "stderr") == 0) {
-                outopt.stream = OutputOption::STR_STDERR;
+                cur_opt.stream = OutputOption::STR_STDERR;
 
             } else {
                 cerr << "Unrecognised console option: " << optarg << "\n";
@@ -111,66 +163,143 @@ int main(int argc, char** argv) {
             }
             break;
 
-        case 'd':
-            spec.setDbglevel(boost::lexical_cast<int>(optarg));
+        case 'd':   // Debug level
+            cur_spec.setDbglevel(boost::lexical_cast<int>(optarg));
             break;
 
-        case 'f':
-            if (c_found || l_found) {
-                cerr << "Cannot specify -f with -c or -l\n";
-                return (1);
+        case 'f':   // File output specification
+            // New output spec.  If one was currently active, add it to the
+            // list and reset the current output option to the defaults.
+            if (c_found || f_found || y_found) {
+                cur_spec.addOutputOption(cur_opt);
+                cur_opt = def_opt;
+                c_found = f_found = y_found = false;
             }
 
+            // Set the output option for this switch.
             f_found = true;
-
-            outopt.destination = OutputOption::DEST_FILE;
-            outopt.filename = optarg;
+            cur_opt.destination = OutputOption::DEST_FILE;
+            cur_opt.filename = optarg;
             break;
 
-        case 'h':
+        case 'h':   // Help
             usage();
             return (0);
 
-        case 's':
-            {
-                string severity(optarg);
-                isc::util::str::uppercase(severity);
-                spec.setSeverity(getSeverity(severity));
+        case 'l':   // Logger
+            // If a current specification is active, add the last output option
+            // to it, add it to the list and reset.  A specification is active
+            // if at least one switch has been previously found.
+            if (sw_found) {
+                cur_spec.addOutputOption(cur_opt);
+                loggers.push_back(cur_spec);
+                cur_spec.reset();
+            }
+
+            // Set the logger name
+            cur_spec.setName(std::string(optarg));
+
+            // Reset the output option to the default.
+            cur_opt = def_opt;
+
+            // Indicate nothing is found to prevent the console option (the
+            // default output option) being added to the output list if an
+            // output option is found.
+            c_found = f_found = y_found = false;
+            break;
+
+        case 'm':   // Maximum file version
+            if (!f_found) {
+                std::cerr << "Attempt to set maximum version (-m) "
+                             "outside of file output specification\n";
+                return (1);
+            }
+            try {
+                cur_opt.maxsize = boost::lexical_cast<unsigned int>(optarg);
+            } catch (boost::bad_lexical_cast&) {
+                std::cerr << "Maximum version (-m) argument must be a positive "
+                             "integer\n";
+                return (1);
+            }
+            break;
+
+        case 's':   // Severity
+            severity = optarg;
+            isc::util::str::uppercase(severity);
+            cur_spec.setSeverity(getSeverity(severity));
+            break;
+
+        case 'y':   // Syslog output
+            // New output spec.  If one was currently active, add it to the
+            // list and reset the current output option to the defaults.
+            if (c_found || f_found || y_found) {
+                cur_spec.addOutputOption(cur_opt);
+                cur_opt = def_opt;
+                c_found = f_found = y_found = false;
+            }
+            y_found = true;
+            cur_opt.destination = OutputOption::DEST_SYSLOG;
+            cur_opt.facility = optarg;
+            break;
+
+        case 'z':   // Maximum size
+            if (! f_found) {
+                std::cerr << "Attempt to set file size (-z) "
+                             "outside of file output specification\n";
+                return (1);
+            }
+            try {
+                cur_opt.maxsize = boost::lexical_cast<size_t>(optarg);
+            } catch (boost::bad_lexical_cast&) {
+                std::cerr << "File size (-z) argument must be a positive "
+                             "integer\n";
+                return (1);
             }
             break;
 
+
         default:
             std::cerr << "Unrecognised option: " <<
                 static_cast<char>(option) << "\n";
             return (1);
         }
+
+        // Have found at least one command-line switch, so note the fact.
+        sw_found = true;
     }
 
-    // Update the logging parameters.  If no output options
-    // were set, the defaults will be used.
-    spec.addOutputOption(outopt);
+    // Add the current (unfinished specification) to the list.
+    cur_spec.addOutputOption(cur_opt);
+    loggers.push_back(cur_spec);
 
-    // Set the logging options for the root logger.
-    LoggerManager manager;
-    manager.process(spec);
+    // Set the logging options.
+    manager.process(loggers.begin(), loggers.end());
 
     // Set the local file
     if (optind < argc) {
         LoggerManager::readLocalMessageFile(argv[optind]);
     }
 
+    // Log a few messages to different loggers.
+    isc::log::Logger logger_ex(ROOT_NAME);
+    isc::log::Logger logger_alpha("alpha");
+    isc::log::Logger logger_beta("beta");
 
-    // Log a few messages
-    isc::log::Logger logger_dlm("dlm");
-    isc::log::Logger logger_ex("example");
     LOG_FATAL(logger_ex, MSG_WRITERR).arg("test1").arg("42");
     LOG_ERROR(logger_ex, MSG_RDLOCMES).arg("dummy/file");
-    LOG_WARN(logger_dlm, MSG_READERR).arg("a.txt").arg("dummy reason");
-    LOG_INFO(logger_dlm, MSG_OPENIN).arg("example.msg").arg("dummy reason");
-    LOG_DEBUG(logger_ex, 0, MSG_RDLOCMES).arg("dummy/0");
-    LOG_DEBUG(logger_ex, 24, MSG_RDLOCMES).arg("dummy/24");
-    LOG_DEBUG(logger_ex, 25, MSG_RDLOCMES).arg("dummy/25");
-    LOG_DEBUG(logger_ex, 26, MSG_RDLOCMES).arg("dummy/26");
+    LOG_WARN(logger_ex, MSG_BADSTREAM).arg("example");
+    LOG_WARN(logger_alpha, MSG_READERR).arg("a.txt").arg("dummy reason");
+    LOG_INFO(logger_alpha, MSG_OPENIN).arg("example.msg").arg("dummy reason");
+    LOG_DEBUG(logger_ex, 0, MSG_RDLOCMES).arg("example/0");
+    LOG_DEBUG(logger_ex, 24, MSG_RDLOCMES).arg("example/24");
+    LOG_DEBUG(logger_ex, 25, MSG_RDLOCMES).arg("example/25");
+    LOG_DEBUG(logger_ex, 26, MSG_RDLOCMES).arg("example/26");
+    LOG_FATAL(logger_beta, MSG_BADSEVERITY).arg("beta_fatal");
+    LOG_ERROR(logger_beta, MSG_BADDESTINATION).arg("beta_error");
+    LOG_WARN(logger_beta, MSG_BADSTREAM).arg("beta_warn");
+    LOG_INFO(logger_beta, MSG_READERR).arg("beta").arg("info");
+    LOG_DEBUG(logger_beta, 25, MSG_BADSEVERITY).arg("beta/25");
+    LOG_DEBUG(logger_beta, 26, MSG_BADSEVERITY).arg("beta/26");
 
     return (0);
 }

src/lib/log/tests/logger_impl_log4cxx_unittest.cc

@@ -1,91 +0,0 @@
-// Copyright (C) 2011  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 <iostream>
-#include <string>
-
-#include <gtest/gtest.h>
-
-#include <log/root_logger_name.h>
-#include <log/logger.h>
-#include <log/logger_impl.h>
-#include <log/messagedef.h>
-
-using namespace isc;
-using namespace isc::log;
-using namespace std;
-
-/// \brief Log4cxx Implementation Tests
-///
-/// Some tests of methods that are not directly tested by the logger unit tests
-/// (when the logger is configured to use log4cxx)
-
-namespace isc {
-namespace log {
-
-/// \brief Test Logger
-///
-/// This logger is a subclass of the logger implementation class under test, but
-/// makes protected methods public (for testing)
-
-class TestLoggerImpl : public LoggerImpl {
-public:
-    /// \brief constructor
-    TestLoggerImpl(const string& name) : LoggerImpl(name, true)
-    {}
-
-
-    /// \brief Conversion Between log4cxx Number and BIND-10 Severity
-    Severity convertLevel(int value) {
-        return (LoggerImpl::convertLevel(value));
-    }
-};
-
-} // namespace log
-} // namespace isc
-
-
-class LoggerImplTest : public ::testing::Test {
-protected:
-    LoggerImplTest()
-    {
-    }
-};
-
-// Test the number to severity conversion function
-
-TEST_F(LoggerImplTest, ConvertLevel) {
-
-    // Create a logger
-    RootLoggerName::setName("test3");
-    TestLoggerImpl logger("alpha");
-
-    // Basic 1:1
-    EXPECT_EQ(isc::log::DEBUG, logger.convertLevel(log4cxx::Level::DEBUG_INT));
-    EXPECT_EQ(isc::log::INFO, logger.convertLevel(log4cxx::Level::INFO_INT));
-    EXPECT_EQ(isc::log::WARN, logger.convertLevel(log4cxx::Level::WARN_INT));
-    EXPECT_EQ(isc::log::WARN, logger.convertLevel(log4cxx::Level::WARN_INT));
-    EXPECT_EQ(isc::log::ERROR, logger.convertLevel(log4cxx::Level::ERROR_INT));
-    EXPECT_EQ(isc::log::FATAL, logger.convertLevel(log4cxx::Level::FATAL_INT));
-    EXPECT_EQ(isc::log::FATAL, logger.convertLevel(log4cxx::Level::FATAL_INT));
-    EXPECT_EQ(isc::log::NONE, logger.convertLevel(log4cxx::Level::OFF_INT));
-
-    // Now some debug levels
-    EXPECT_EQ(isc::log::DEBUG,
-        logger.convertLevel(log4cxx::Level::DEBUG_INT - 1));
-    EXPECT_EQ(isc::log::DEBUG,
-        logger.convertLevel(log4cxx::Level::DEBUG_INT - MAX_DEBUG_LEVEL));
-    EXPECT_EQ(isc::log::DEBUG,
-        logger.convertLevel(log4cxx::Level::DEBUG_INT - 2 * MAX_DEBUG_LEVEL));
-}

src/lib/log/tests/logger_level_unittest.cc

@@ -17,23 +17,19 @@
 
 #include <gtest/gtest.h>
 
-#include <log/root_logger_name.h>
 #include <log/logger.h>
 #include <log/logger_manager.h>
+#include <log/logger_name.h>
 #include <log/messagedef.h>
 
 using namespace isc;
 using namespace isc::log;
 using namespace std;
 
-namespace {
-string ROOT_NAME("logleveltest");
-}
-
 class LoggerLevelTest : public ::testing::Test {
 protected:
     LoggerLevelTest() {
-        LoggerManager::init(ROOT_NAME);
+        // Logger initialization is done in main()
     }
     ~LoggerLevelTest() {
         LoggerManager::reset();
@@ -62,11 +58,6 @@ TEST_F(LoggerLevelTest, Creation) {
 }
 
 TEST(LoggerLevel, getSeverity) {
-    // Should initialize logger as getSeverity() may output
-    // a message.  This gives a properly-qualified logger
-    // name.
-    LoggerManager::init(ROOT_NAME);
-
     EXPECT_EQ(DEBUG, getSeverity("DEBUG"));
     EXPECT_EQ(DEBUG, getSeverity("debug"));
     EXPECT_EQ(DEBUG, getSeverity("DeBuG"));

src/lib/log/tests/logger_manager_unittest.cc

@@ -40,15 +40,11 @@ using namespace isc;
 using namespace isc::log;
 using namespace std;
 
-namespace {
-string ROOT_NAME("logmgrtest");
-}
-
 /// \brief LoggerManager Test
 class LoggerManagerTest : public ::testing::Test {
 public:
     LoggerManagerTest() {
-        LoggerManager::init(ROOT_NAME);
+        // Initialization of logging is done in main()
     }
 
     ~LoggerManagerTest() {

src/lib/log/tests/root_logger_name_unittest.cc

@@ -16,21 +16,35 @@
 
 #include <gtest/gtest.h>
 
-#include <log/root_logger_name.h>
+#include <log/logger_name.h>
 
 using namespace isc;
 using namespace isc::log;
 
-class RootLoggerNameTest : public ::testing::Test {
-protected:
-    RootLoggerNameTest()
-    {
+// Test class.  To avoid disturbing the root logger configuration in other
+// tests in the suite, the root logger name is saved in the constructor and
+// restored in the destructor.  However, this is a bit chicken and egg, as the
+// functions used to do the save and restore are those being tested...
+//
+// Note that the root name is originally set by the initialization of the
+// logging configuration done in main().
+
+class LoggerNameTest : public ::testing::Test {
+public:
+    LoggerNameTest() {
+        name_ = getRootLoggerName();
     }
+    ~LoggerNameTest() {
+        setRootLoggerName(name_);
+    }
+
+private:
+    std::string     name_;  ///< Saved name
 };
 
-// Check of the (only) functionality of the class.
+// Check setting and getting of root name
 
-TEST_F(RootLoggerNameTest, SetGet) {
+TEST_F(LoggerNameTest, RootNameSetGet) {
     const std::string name1 = "test1";
     const std::string name2 = "test2";
 
@@ -48,3 +62,16 @@ TEST_F(RootLoggerNameTest, SetGet) {
     setRootLoggerName(name2);
     EXPECT_EQ(name2, getRootLoggerName());
 }
+
+// Check expansion of name
+
+TEST_F(LoggerNameTest, ExpandLoggerName) {
+    const std::string ROOT = "example";
+    const std::string NAME = "something";
+    const std::string FULL_NAME = ROOT + "." + NAME;
+
+    setRootLoggerName(ROOT);
+    EXPECT_EQ(ROOT, expandLoggerName(ROOT));
+    EXPECT_EQ(FULL_NAME, expandLoggerName(NAME));
+    EXPECT_EQ(FULL_NAME, expandLoggerName(FULL_NAME));
+}

src/lib/log/tests/logger_unittest.cc

@@ -17,19 +17,15 @@
 
 #include <gtest/gtest.h>
 
-#include <log/root_logger_name.h>
 #include <log/logger.h>
 #include <log/logger_manager.h>
+#include <log/logger_name.h>
 #include <log/messagedef.h>
 
 using namespace isc;
 using namespace isc::log;
 using namespace std;
 
-namespace {
-string ROOT_NAME = "loggertest";
-}
-
 /// \brief Logger Test
 ///
 /// As the logger is only a shell around the implementation, this tests also
@@ -38,7 +34,7 @@ string ROOT_NAME = "loggertest";
 class LoggerTest : public ::testing::Test {
 public:
     LoggerTest() {
-        LoggerManager::init(ROOT_NAME);
+        // Initialization of logging is done in main()
     }
     ~LoggerTest() {
         LoggerManager::reset();
@@ -54,7 +50,7 @@ TEST_F(LoggerTest, Name) {
     Logger logger("alpha");
 
     // ... and check the name
-    EXPECT_EQ(ROOT_NAME + string(".alpha"), logger.getName());
+    EXPECT_EQ(getRootLoggerName() + string(".alpha"), logger.getName());
 }
 
 // This test attempts to get two instances of a logger with the same name
@@ -154,8 +150,44 @@ TEST_F(LoggerTest, DebugLevels) {
 TEST_F(LoggerTest, SeverityInheritance) {
 
     // Create two loggers.  We cheat here as we know that the underlying
-    // implementation (in this case log4cxx) will set a parent-child
-    // relationship if the loggers are named <parent> and <parent>.<child>.
+    // implementation will set a parent-child relationship if the loggers
+    // are named <parent> and <parent>.<child>.
+    Logger parent("alpha");
+    Logger child("alpha.beta");
+
+    // By default, newly created loggers should have a level of DEFAULT
+    // (i.e. default to parent)
+    EXPECT_EQ(isc::log::DEFAULT, parent.getSeverity());
+    EXPECT_EQ(isc::log::DEFAULT, child.getSeverity());
+
+    // Set the severity of the parent to debug and check what is
+    // reported by the child.
+    parent.setSeverity(isc::log::DEBUG, 42);
+    EXPECT_EQ(42, parent.getDebugLevel());
+    EXPECT_EQ(0,  child.getDebugLevel());
+    EXPECT_EQ(42, child.getEffectiveDebugLevel());
+
+    // Setting the child to DEBUG severity should set its own
+    // debug level.
+    child.setSeverity(isc::log::DEBUG, 53);
+    EXPECT_EQ(53,  child.getDebugLevel());
+    EXPECT_EQ(53, child.getEffectiveDebugLevel());
+
+    // If the child severity is set to something other than DEBUG,
+    // the debug level should be reported as 0.
+    child.setSeverity(isc::log::ERROR);
+    EXPECT_EQ(0,  child.getDebugLevel());
+    EXPECT_EQ(0, child.getEffectiveDebugLevel());
+}
+
+// Check that changing the parent and child debug level does not affect
+// the other.
+
+TEST_F(LoggerTest, DebugLevelInheritance) {
+
+    // Create two loggers.  We cheat here as we know that the underlying
+    // implementation will set a parent-child relationship if the loggers
+    // are named <parent> and <parent>.<child>.
     Logger parent("alpha");
     Logger child("alpha.beta");
 
@@ -184,8 +216,8 @@ TEST_F(LoggerTest, SeverityInheritance) {
 TEST_F(LoggerTest, EffectiveSeverityInheritance) {
 
     // Create two loggers.  We cheat here as we know that the underlying
-    // implementation (in this case log4cxx) will set a parent-child
-    // relationship if the loggers are named <parent> and <parent>.<child>.
+    // implementation will set a parent-child relationship if the loggers
+    // are named <parent> and <parent>.<child>.
     Logger parent("test6");
     Logger child("test6.beta");
 

src/lib/log/tests/output_option_unittest.cc

@@ -30,7 +30,7 @@ TEST(OutputOptionTest, Initialization) {
     EXPECT_EQ(OutputOption::DEST_CONSOLE, option.destination);
     EXPECT_EQ(OutputOption::STR_STDERR, option.stream);
     EXPECT_FALSE(option.flush);
-    EXPECT_EQ(string(""), option.facility);
+    EXPECT_EQ(string("LOCAL0"), option.facility);
     EXPECT_EQ(string(""), option.filename);
     EXPECT_EQ(0, option.maxsize);
     EXPECT_EQ(0, option.maxver);

src/lib/log/tests/severity_test.sh.in

@@ -26,46 +26,57 @@ tempfile=@abs_builddir@/severity_test_tempfile_$$
 
 passfail() {
     if [ $1 -eq 0 ]; then
-        echo " -- pass"
+        echo " pass"
     else
-        echo " ** FAIL"
+        echo " FAIL"
         failcount=`expr $failcount + $1`
     fi
 }
 
-echo "1. runInitTest default parameters: "
+echo -n "1. runInitTest default parameters:"
 cat > $tempfile << .
-FATAL [alpha.example] MSG_WRITERR, error writing to test1: 42
-ERROR [alpha.example] MSG_RDLOCMES, reading local message file dummy/file
-WARN  [alpha.dlm] MSG_READERR, error reading from message file a.txt: dummy reason
-INFO  [alpha.dlm] MSG_OPENIN, unable to open message file example.msg for input: dummy reason
+FATAL [example] MSG_WRITERR, error writing to test1: 42
+ERROR [example] MSG_RDLOCMES, reading local message file dummy/file
+WARN  [example] MSG_BADSTREAM, bad log console output stream: example
+WARN  [example.alpha] MSG_READERR, error reading from message file a.txt: dummy reason
+INFO  [example.alpha] MSG_OPENIN, unable to open message file example.msg for input: dummy reason
+FATAL [example.beta] MSG_BADSEVERITY, unrecognized log severity: beta_fatal
+ERROR [example.beta] MSG_BADDESTINATION, unrecognized log destination: beta_error
+WARN  [example.beta] MSG_BADSTREAM, bad log console output stream: beta_warn
+INFO  [example.beta] MSG_READERR, error reading from message file beta: info
 .
 ./logger_example -c stdout | cut -d' ' -f3- | diff $tempfile -
 passfail $?
 
-echo "2. Severity filter: "
+echo -n "2. Severity filter:"
 cat > $tempfile << .
-FATAL [alpha.example] MSG_WRITERR, error writing to test1: 42
-ERROR [alpha.example] MSG_RDLOCMES, reading local message file dummy/file
+FATAL [example] MSG_WRITERR, error writing to test1: 42
+ERROR [example] MSG_RDLOCMES, reading local message file dummy/file
+FATAL [example.beta] MSG_BADSEVERITY, unrecognized log severity: beta_fatal
+ERROR [example.beta] MSG_BADDESTINATION, unrecognized log destination: beta_error
 .
 ./logger_example -c stdout -s error | cut -d' ' -f3- | diff $tempfile -
 passfail $?
 
-echo "3. Debug level: "
+echo -n "3. Debug level:"
 cat > $tempfile << .
-FATAL [alpha.example] MSG_WRITERR, error writing to test1: 42
-ERROR [alpha.example] MSG_RDLOCMES, reading local message file dummy/file
-WARN  [alpha.dlm] MSG_READERR, error reading from message file a.txt: dummy reason
-INFO  [alpha.dlm] MSG_OPENIN, unable to open message file example.msg for input: dummy reason
-DEBUG [alpha.example] MSG_RDLOCMES, reading local message file dummy/0
-DEBUG [alpha.example] MSG_RDLOCMES, reading local message file dummy/24
-DEBUG [alpha.example] MSG_RDLOCMES, reading local message file dummy/25
+FATAL [example] MSG_WRITERR, error writing to test1: 42
+ERROR [example] MSG_RDLOCMES, reading local message file dummy/file
+WARN  [example] MSG_BADSTREAM, bad log console output stream: example
+WARN  [example.alpha] MSG_READERR, error reading from message file a.txt: dummy reason
+INFO  [example.alpha] MSG_OPENIN, unable to open message file example.msg for input: dummy reason
+DEBUG [example] MSG_RDLOCMES, reading local message file example/0
+DEBUG [example] MSG_RDLOCMES, reading local message file example/24
+DEBUG [example] MSG_RDLOCMES, reading local message file example/25
+FATAL [example.beta] MSG_BADSEVERITY, unrecognized log severity: beta_fatal
+ERROR [example.beta] MSG_BADDESTINATION, unrecognized log destination: beta_error
+WARN  [example.beta] MSG_BADSTREAM, bad log console output stream: beta_warn
+INFO  [example.beta] MSG_READERR, error reading from message file beta: info
+DEBUG [example.beta] MSG_BADSEVERITY, unrecognized log severity: beta/25
 .
 ./logger_example -c stdout -s debug -d 25 | cut -d' ' -f3- | diff $tempfile -
 passfail $?
 
-rm -f $tempfile
-
 if [ $failcount -eq 0 ]; then
     echo "PASS: $testname"
 elif [ $failcount -eq 1 ]; then
@@ -74,4 +85,7 @@ else
     echo "FAIL: $testname - $failcount tests failed"
 fi
 
+# Tidy up
+rm -f $tempfile
+
 exit $failcount

src/lib/log/tests/xdebuglevel_unittest.cc

@@ -1,203 +0,0 @@
-// Copyright (C) 2011  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 <iostream>
-#include <string>
-
-#include <gtest/gtest.h>
-
-#include <log4cxx/level.h>
-#include <log/xdebuglevel.h>
-#include <log/debug_levels.h>
-
-/// \brief XDebugLevel (Debug Extension to Level Class)
-///
-/// The class is an extension of the log4cxx Level class; this set of tests
-/// only test the extensions, they do not test the underlying Level class
-/// itself.
-
-using namespace log4cxx;
-
-class XDebugLevelTest : public ::testing::Test {
-protected:
-    XDebugLevelTest()
-    {
-    }
-};
-
-// Check a basic assertion about the numeric values of the debug levels
-
-TEST_F(XDebugLevelTest, NumericValues) {
-    EXPECT_EQ(XDebugLevel::XDEBUG_MIN_LEVEL_INT, Level::DEBUG_INT);
-    EXPECT_EQ(XDebugLevel::XDEBUG_MAX_LEVEL_INT,
-        Level::DEBUG_INT - MAX_DEBUG_LEVEL);
-
-    // ... and check that assumptions used below - that the debug levels
-    // range from 0 to 99 - are valid.
-    EXPECT_EQ(0, MIN_DEBUG_LEVEL);
-    EXPECT_EQ(99, MAX_DEBUG_LEVEL);
-}
-
-
-// Checks that the main function for generating logging level objects from
-// debug levels is working.
-
-TEST_F(XDebugLevelTest, GetExtendedDebug) {
-
-    // Get a debug level of 0.  This should be the same as the main DEBUG
-    // level.
-    LevelPtr debug0 = XDebugLevel::getExtendedDebug(0);
-    EXPECT_EQ(std::string("DEBUG"), debug0->toString());
-    EXPECT_EQ(Level::DEBUG_INT, debug0->toInt());
-    EXPECT_TRUE(*Level::getDebug() == *debug0);
-
-    // Get an arbitrary debug level in the allowed range.
-    LevelPtr debug32 = XDebugLevel::getExtendedDebug(32);
-    EXPECT_EQ(std::string("DEBUG32"), debug32->toString());
-    EXPECT_TRUE((XDebugLevel::XDEBUG_MIN_LEVEL_INT - 32) == debug32->toInt());
-
-    // Check that a value outside the range gives the nearest level.
-    LevelPtr debug_more = XDebugLevel::getExtendedDebug(MAX_DEBUG_LEVEL + 1);
-    EXPECT_TRUE(*XDebugLevel::getExtendedDebug(MAX_DEBUG_LEVEL) == *debug_more);
-
-    LevelPtr debug_less = XDebugLevel::getExtendedDebug(MIN_DEBUG_LEVEL - 1);
-    EXPECT_TRUE(*XDebugLevel::getExtendedDebug(MIN_DEBUG_LEVEL) == *debug_less);
-}
-
-
-// Creation of a level from an int - should return the default debug level
-// if outside the range.
-
-TEST_F(XDebugLevelTest, FromIntOneArg) {
-
-    // Check that a valid debug level is as expected
-    LevelPtr debug42 = XDebugLevel::toLevel(
-        XDebugLevel::XDEBUG_MIN_LEVEL_INT - 42);
-    EXPECT_TRUE(*XDebugLevel::getExtendedDebug(42) == *debug42);
-
-    // ... and that an invalid one returns an object of type debug.
-    LevelPtr debug_invalid = XDebugLevel::toLevel(Level::getInfo()->toInt());
-    EXPECT_TRUE(*Level::getDebug() == *debug_invalid);
-}
-
-
-// Creation of a level from an int - should return the default level
-// if outside the range.
-
-TEST_F(XDebugLevelTest, FromIntTwoArg) {
-
-    // Check that a valid debug level is as expected
-    LevelPtr debug42 = XDebugLevel::toLevel(
-        (XDebugLevel::XDEBUG_MIN_LEVEL_INT - 42), Level::getFatal());
-    EXPECT_TRUE(*XDebugLevel::getExtendedDebug(42) == *debug42);
-
-    // ... and that an invalid one returns an object of type debug.
-    LevelPtr debug_invalid = XDebugLevel::toLevel(
-        Level::getInfo()->toInt(), Level::getFatal());
-    EXPECT_TRUE(*Level::getFatal() == *debug_invalid);
-}
-
-
-// Creation of a level from a string - should return the default debug level
-// if outside the range.
-
-TEST_F(XDebugLevelTest, FromStringOneArg) {
-
-    // Check that a valid debug levels are as expected
-    LevelPtr debug85 = XDebugLevel::toLevelLS(LOG4CXX_STR("DEBUG85"));
-    EXPECT_TRUE(*XDebugLevel::getExtendedDebug(85) == *debug85);
-
-    LevelPtr debug92 = XDebugLevel::toLevelLS(LOG4CXX_STR("debug92"));
-    EXPECT_TRUE(*XDebugLevel::getExtendedDebug(92) == *debug92);
-
-    LevelPtr debug27 = XDebugLevel::toLevelLS(LOG4CXX_STR("Debug27"));
-    EXPECT_TRUE(*XDebugLevel::getExtendedDebug(27) == *debug27);
-
-    LevelPtr debug0 = XDebugLevel::toLevelLS(LOG4CXX_STR("DEBUG"));
-    EXPECT_TRUE(*XDebugLevel::getExtendedDebug(0) == *debug0);
-
-    // ... and that an invalid one returns an object of type debug (which is
-    // the equivalent of a debug level 0 object).
-    LevelPtr debug_invalid1 = XDebugLevel::toLevelLS(LOG4CXX_STR("DEBU"));
-    EXPECT_TRUE(*XDebugLevel::getExtendedDebug(0) == *debug_invalid1);
-
-    LevelPtr debug_invalid2 = XDebugLevel::toLevelLS(LOG4CXX_STR("EBU"));
-    EXPECT_TRUE(*XDebugLevel::getExtendedDebug(0) == *debug_invalid2);
-
-    LevelPtr debug_invalid3 = XDebugLevel::toLevelLS(LOG4CXX_STR(""));
-    EXPECT_TRUE(*XDebugLevel::getExtendedDebug(0) == *debug_invalid3);
-
-    LevelPtr debug_invalid4 = XDebugLevel::toLevelLS(LOG4CXX_STR("DEBUGTEN"));
-    EXPECT_TRUE(*XDebugLevel::getExtendedDebug(0) == *debug_invalid4);
-
-    LevelPtr debug_invalid5 = XDebugLevel::toLevelLS(LOG4CXX_STR("DEBUG105"));
-    EXPECT_TRUE(*XDebugLevel::getExtendedDebug(MAX_DEBUG_LEVEL) ==
-        *debug_invalid5);
-
-    LevelPtr debug_invalid6 = XDebugLevel::toLevelLS(LOG4CXX_STR("DEBUG-7"));
-    EXPECT_TRUE(*XDebugLevel::getExtendedDebug(MIN_DEBUG_LEVEL) ==
-        *debug_invalid6);
-}
-
-
-// Creation of a level from a string - should return the default level
-// if outside the range.
-
-TEST_F(XDebugLevelTest, FromStringTwoArg) {
-
-    // Check that a valid debug levels are as expected
-    LevelPtr debug85 = XDebugLevel::toLevelLS(LOG4CXX_STR("DEBUG85"),
-            Level::getFatal());
-    EXPECT_TRUE(*XDebugLevel::getExtendedDebug(85) == *debug85);
-
-    LevelPtr debug92 = XDebugLevel::toLevelLS(LOG4CXX_STR("debug92"),
-            Level::getFatal());
-    EXPECT_TRUE(*XDebugLevel::getExtendedDebug(92) == *debug92);
-
-    LevelPtr debug27 = XDebugLevel::toLevelLS(LOG4CXX_STR("Debug27"),
-            Level::getFatal());
-    EXPECT_TRUE(*XDebugLevel::getExtendedDebug(27) == *debug27);
-
-    LevelPtr debug0 = XDebugLevel::toLevelLS(LOG4CXX_STR("DEBUG"),
-            Level::getFatal());
-    EXPECT_TRUE(*XDebugLevel::getExtendedDebug(0) == *debug0);
-
-    // ... and that an invalid one returns an object of type debug (which is
-    // the equivalent of a debug level 0 object).
-    LevelPtr debug_invalid1 = XDebugLevel::toLevelLS(LOG4CXX_STR("DEBU"),
-            Level::getFatal());
-    EXPECT_TRUE(*Level::getFatal() == *debug_invalid1);
-
-    LevelPtr debug_invalid2 = XDebugLevel::toLevelLS(LOG4CXX_STR("EBU"),
-            Level::getFatal());
-    EXPECT_TRUE(*Level::getFatal() == *debug_invalid2);
-
-    LevelPtr debug_invalid3 = XDebugLevel::toLevelLS(LOG4CXX_STR(""),
-            Level::getFatal());
-    EXPECT_TRUE(*Level::getFatal() == *debug_invalid3);
-
-    LevelPtr debug_invalid4 = XDebugLevel::toLevelLS(LOG4CXX_STR("DEBUGTEN"),
-            Level::getFatal());
-    EXPECT_TRUE(*Level::getFatal() == *debug_invalid4);
-
-    LevelPtr debug_invalid5 = XDebugLevel::toLevelLS(LOG4CXX_STR("DEBUG105"),
-            Level::getFatal());
-    EXPECT_TRUE(*XDebugLevel::getExtendedDebug(MAX_DEBUG_LEVEL) ==
-        *debug_invalid5);
-
-    LevelPtr debug_invalid6 = XDebugLevel::toLevelLS(LOG4CXX_STR("DEBUG-7"),
-            Level::getFatal());
-    EXPECT_TRUE(*XDebugLevel::getExtendedDebug(MIN_DEBUG_LEVEL) ==
-        *debug_invalid6);
-}

src/lib/python/isc/config/cfgmgr.py

@@ -214,7 +214,7 @@ class ConfigManager:
            is returned"""
         if module_name:
             if module_name in self.module_specs:
-                return self.module_specs[module_name]
+                return self.module_specs[module_name].get_full_spec()
             else:
                 # TODO: log error?
                 return {}

src/lib/python/isc/config/tests/cfgmgr_test.py

@@ -176,7 +176,7 @@ class TestConfigManager(unittest.TestCase):
         self.cm.set_module_spec(module_spec)
         self.assert_(module_spec.get_module_name() in self.cm.module_specs)
         module_spec2 = self.cm.get_module_spec(module_spec.get_module_name())
-        self.assertEqual(module_spec, module_spec2)
+        self.assertEqual(module_spec.get_full_spec(), module_spec2)
 
         self.assertEqual({}, self.cm.get_module_spec("nosuchmodule"))
 

src/lib/python/isc/notify/notify_out.py

@@ -21,6 +21,7 @@ import threading
 import time
 import errno
 from isc.datasrc import sqlite3_ds
+from isc.net import addr
 import isc
 try: 
     from pydnspp import * 
@@ -66,7 +67,7 @@ class ZoneNotifyInfo:
         self.zone_name = zone_name_
         self.zone_class = class_
         self.notify_msg_id = 0
-        self.notify_timeout = 0
+        self.notify_timeout = None
         self.notify_try_num = 0  #Notify times sending to one target.
        
     def set_next_notify_target(self):
@@ -77,8 +78,7 @@ class ZoneNotifyInfo:
             self._notify_current = None
 
     def prepare_notify_out(self):
-        '''Create the socket and set notify timeout time to now'''
-        self._sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) #TODO support IPv6?
+        '''Set notify timeout time to now'''
         self.notify_timeout = time.time()
         self.notify_try_num = 0
         self._slave_index = 0
@@ -89,6 +89,12 @@ class ZoneNotifyInfo:
         if self._sock:
             self._sock.close()
             self._sock = None
+        self.notify_timeout = None
+
+    def create_socket(self, dest_addr):
+        self._sock = socket.socket(addr.IPAddr(dest_addr).family,
+                                   socket.SOCK_DGRAM)
+        return self._sock
 
     def get_socket(self):
         return self._sock
@@ -270,8 +276,15 @@ class NotifyOut:
             sock = self._notify_infos[info].get_socket()
             if sock:
                 valid_socks.append(sock)
+
+            # If a non null timeout is specified notify has been scheduled
+            # (in which case socket is still None) or sent (with a valid
+            # socket).  In either case we need add the zone to notifying_zones
+            # so that we can invoke the appropriate event for the zone after
+            # select.
+            tmp_timeout = self._notify_infos[info].notify_timeout
+            if tmp_timeout is not None:
                 notifying_zones[info] = self._notify_infos[info]
-                tmp_timeout = self._notify_infos[info].notify_timeout
                 if min_timeout is not None:
                     if tmp_timeout < min_timeout:
                         min_timeout = tmp_timeout
@@ -380,12 +393,13 @@ class NotifyOut:
         render.set_length_limit(512) 
         msg.to_wire(render)
         zone_notify_info.notify_msg_id = qid
-        sock = zone_notify_info.get_socket()
         try:
+            sock = zone_notify_info.create_socket(addrinfo[0])
             sock.sendto(render.get_data(), 0, addrinfo)
             self._log_msg('info', 'sending notify to %s' % addr_to_str(addrinfo))
-        except socket.error as err:
-            self._log_msg('error', 'send notify to %s failed: %s' % (addr_to_str(addrinfo), str(err)))
+        except (socket.error, addr.InvalidAddress) as err:
+            self._log_msg('error', 'send notify to %s failed: %s' %
+                          (addr_to_str(addrinfo), str(err)))
             return False
 
         return True

src/lib/python/isc/notify/tests/notify_out_test.py

@@ -24,9 +24,7 @@ from isc.notify import notify_out, SOCK_DATA
 
 # our fake socket, where we can read and insert messages
 class MockSocket():
-    def __init__(self, family, type):
-        self.family = family
-        self.type = type
+    def __init__(self):
         self._local_sock, self._remote_sock = socket.socketpair()
 
     def connect(self, to):
@@ -51,12 +49,16 @@ class MockSocket():
         return self._remote_sock
 
 # We subclass the ZoneNotifyInfo class we're testing here, only
-# to override the prepare_notify_out() method.
+# to override the create_socket() method.
 class MockZoneNotifyInfo(notify_out.ZoneNotifyInfo):
-    def prepare_notify_out(self):
-        super().prepare_notify_out();
+    def create_socket(self, addrinfo):
+        super().create_socket(addrinfo)
+        # before replacing the underlying socket, remember the address family
+        # of the original socket so that tests can check that.
+        self.sock_family = self._sock.family
         self._sock.close()
-        self._sock = MockSocket(socket.AF_INET, socket.SOCK_DGRAM)
+        self._sock = MockSocket()
+        return self._sock
 
 class TestZoneNotifyInfo(unittest.TestCase):
     def setUp(self):
@@ -64,11 +66,12 @@ class TestZoneNotifyInfo(unittest.TestCase):
 
     def test_prepare_finish_notify_out(self):
         self.info.prepare_notify_out()
-        self.assertNotEqual(self.info._sock, None)
+        self.assertNotEqual(self.info.notify_timeout, None)
         self.assertIsNone(self.info._notify_current)
 
         self.info.finish_notify_out()
         self.assertEqual(self.info._sock, None)
+        self.assertEqual(self.info.notify_timeout, None)
 
     def test_set_next_notify_target(self):
         self.info.notify_slaves.append(('127.0.0.1', 53))
@@ -155,6 +158,11 @@ class TestNotifyOut(unittest.TestCase):
         self.assertEqual(len(replied_zones), 0)
         self.assertEqual(len(timeout_zones), 2)
 
+        # Trigger timeout events to "send" notifies via a mock socket
+        for zone in timeout_zones:
+            self._notify._zone_notify_handler(timeout_zones[zone],
+                                              notify_out._EVENT_TIMEOUT)
+
         # Now make one socket be readable
         self._notify._notify_infos[('example.net.', 'IN')].notify_timeout = time.time() + 10
         self._notify._notify_infos[('example.com.', 'IN')].notify_timeout = time.time() + 10
@@ -234,11 +242,31 @@ class TestNotifyOut(unittest.TestCase):
         data = b'\x2f\x18\x10\x10\x00\x01\x00\x00\x00\x00\x00\x00\x07example\03com\x00\x00\x06\x00\x01'
         self.assertEqual(notify_out._BAD_QR, self._notify._handle_notify_reply(example_com_info, data))
 
-    def test_send_notify_message_udp(self):
+    def test_send_notify_message_udp_ipv4(self):
         example_com_info = self._notify._notify_infos[('example.net.', 'IN')]
         example_com_info.prepare_notify_out()
-        ret = self._notify._send_notify_message_udp(example_com_info, ('1.1.1.1', 53))
+        ret = self._notify._send_notify_message_udp(example_com_info,
+                                                    ('192.0.2.1', 53))
+        self.assertTrue(ret)
+        self.assertEqual(socket.AF_INET, example_com_info.sock_family)
+
+    def test_send_notify_message_udp_ipv6(self):
+        example_com_info = self._notify._notify_infos[('example.net.', 'IN')]
+        ret = self._notify._send_notify_message_udp(example_com_info,
+                                                    ('2001:db8::53', 53))
         self.assertTrue(ret)
+        self.assertEqual(socket.AF_INET6, example_com_info.sock_family)
+
+    def test_send_notify_message_with_bogus_address(self):
+        example_com_info = self._notify._notify_infos[('example.net.', 'IN')]
+
+        # As long as the underlying data source validates RDATA this shouldn't
+        # happen, but right now it's not actually the case.  Even if the
+        # data source does its job, it's prudent to confirm the behavior for
+        # an unexpected case.
+        ret = self._notify._send_notify_message_udp(example_com_info,
+                                                    ('invalid', 53))
+        self.assertFalse(ret)
 
     def test_zone_notify_handler(self):
         old_send_msg = self._notify._send_notify_message_udp

src/lib/server_common/keyring.cc

@@ -27,7 +27,8 @@ KeyringPtr keyring;
 namespace {
 
 void
-updateKeyring(const std::string&, ConstElementPtr data) {
+updateKeyring(const std::string&, ConstElementPtr data,
+              const isc::config::ConfigData&) {
     ConstElementPtr list(data->get("keys"));
     KeyringPtr load(new TSIGKeyRing);
 

src/lib/util/encode/base_n.cc

@@ -160,19 +160,42 @@ public:
         base_zero_code_(base_zero_code),
         base_(base), base_beginpad_(base_beginpad), base_end_(base_end),
         in_pad_(false)
-    {}
+    {
+        // Skip beginning spaces, if any.  We need do it here because
+        // otherwise the first call to operator*() would be confused.
+        skipSpaces();
+    }
     DecodeNormalizer& operator++() {
         ++base_;
-        while (base_ != base_end_ && isspace(*base_)) {
-            ++base_;
-        }
+        skipSpaces();
         if (base_ == base_beginpad_) {
             in_pad_ = true;
         }
         return (*this);
     }
+    void skipSpaces() {
+        // If (char is signed and) *base_ < 0, on Windows platform with Visual
+        // Studio compiler it may trigger _ASSERTE((unsigned)(c + 1) <= 256);
+        // so make sure that the parameter of isspace() is larger than 0.
+        // We don't simply cast it to unsigned char to avoid confusing the
+        // isspace() implementation with a possible extension for values
+        // larger than 127.  Also note the check is not ">= 0"; for systems
+        // where char is unsigned that would always be true and would possibly
+        // trigger a compiler warning that could stop the build.
+        while (base_ != base_end_ && *base_ > 0 && isspace(*base_)) {
+            ++base_;
+        }
+    }
     const char& operator*() const {
-        if (in_pad_ && *base_ == BASE_PADDING_CHAR) {
+        if (base_ == base_end_) {
+            // binary_from_baseX calls this operator when it needs more bits
+            // even if the internal iterator (base_) has reached its end
+            // (if that happens it means the input is an incomplete baseX
+            // string and should be rejected).  So this is the only point
+            // we can catch and reject this type of invalid input.
+            isc_throw(BadValue, "Unexpected end of input in BASE decoder");
+        }
+        if (in_pad_) {
             return (base_zero_code_);
         } else {
             return (*base_);
@@ -268,7 +291,7 @@ BaseNTransformer<BitsPerChunk, BaseZeroCode, Encoder, Decoder>::decode(
                 isc_throw(BadValue, "Too many " << algorithm
                           << " padding characters: " << input);
             }
-        } else if (!isspace(ch)) {
+        } else if (ch < 0 || !isspace(ch)) {
             break;
         }
         ++srit;

src/lib/util/tests/base32hex_unittest.cc

@@ -66,7 +66,7 @@ decodeCheck(const string& input_string, vector<uint8_t>& output,
             const string& expected)
 {
     decodeBase32Hex(input_string, output);
-    EXPECT_EQ(expected, string(&output[0], &output[0] + output.size()));
+    EXPECT_EQ(expected, string(output.begin(), output.end()));
 }
 
 TEST_F(Base32HexTest, decode) {
@@ -79,6 +79,11 @@ TEST_F(Base32HexTest, decode) {
     // whitespace should be allowed
     decodeCheck("CP NM\tUOG=", decoded_data, "foob");
     decodeCheck("CPNMU===\n", decoded_data, "foo");
+    decodeCheck("  CP NM\tUOG=", decoded_data, "foob");
+    decodeCheck(" ", decoded_data, "");
+
+    // Incomplete input
+    EXPECT_THROW(decodeBase32Hex("CPNMUOJ", decoded_data), BadValue);
 
     // invalid number of padding characters
     EXPECT_THROW(decodeBase32Hex("CPNMU0==", decoded_data), BadValue);

src/lib/util/tests/base64_unittest.cc

@@ -52,7 +52,7 @@ decodeCheck(const string& input_string, vector<uint8_t>& output,
             const string& expected)
 {
     decodeBase64(input_string, output);
-    EXPECT_EQ(expected, string(&output[0], &output[0] + output.size()));
+    EXPECT_EQ(expected, string(output.begin(), output.end()));
 }
 
 TEST_F(Base64Test, decode) {
@@ -66,6 +66,12 @@ TEST_F(Base64Test, decode) {
     decodeCheck("Zm 9v\tYmF\ny", decoded_data, "foobar");
     decodeCheck("Zm9vYg==", decoded_data, "foob");
     decodeCheck("Zm9vYmE=\n", decoded_data, "fooba");
+    decodeCheck(" Zm9vYmE=\n", decoded_data, "fooba");
+    decodeCheck(" ", decoded_data, "");
+    decodeCheck("\n\t", decoded_data, "");
+
+    // incomplete input
+    EXPECT_THROW(decodeBase64("Zm9vYmF", decoded_data), BadValue);
 
     // only up to 2 padding characters are allowed
     EXPECT_THROW(decodeBase64("A===", decoded_data), BadValue);