Merge branch 'master' into trac955

ChangeLog

@@ -1,8 +1,53 @@
-246.    [func]      stephen
+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
+	Add file and stderr as destinations for logging.
+	(Trac555, git 38b3546867425bd64dbc5920111a843a3330646b)
+
+247.    [func]      	jelte
+	Upstream queries from the resolver now set EDNS0 buffer size.
+	(Trac834, git 48e10c2530fe52c9bde6197db07674a851aa0f5d)
+
+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

@@ -890,7 +890,11 @@ AC_OUTPUT([doc/version.ent
            src/lib/dns/tests/testdata/gen-wiredata.py
            src/lib/cc/session_config.h.pre
            src/lib/cc/tests/session_unittests_config.h
-           src/lib/log/tests/run_time_init_test.sh
+           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
            src/lib/util/python/mkpywrapper.py
            src/lib/server_common/tests/data_path.h
            tests/system/conf.sh
@@ -917,7 +921,10 @@ AC_OUTPUT([doc/version.ent
            chmod +x src/bin/msgq/tests/msgq_test
            chmod +x src/lib/dns/gen-rdatacode.py
            chmod +x src/lib/dns/tests/testdata/gen-wiredata.py
-           chmod +x src/lib/log/tests/run_time_init_test.sh
+           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 tests/system/conf.sh
           ])

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/xfrout/tests/xfrout_test.py.in

@@ -18,11 +18,14 @@
 
 import unittest
 import os
+from isc.testutils.tsigctx_mock import MockTSIGContext
 from isc.cc.session import *
 from pydnspp import *
 from xfrout import *
 import xfrout
 
+TSIG_KEY = TSIGKey("example.com:SFuWd/q99SzF8Yzd1QbB9g==")
+
 # our fake socket, where we can read and insert messages
 class MySocket():
     def __init__(self, family, type):
@@ -85,10 +88,36 @@ class TestXfroutSession(unittest.TestCase):
         msg.from_wire(self.mdata)
         return msg
 
+    def create_mock_tsig_ctx(self, error):
+        # This helper function creates a MockTSIGContext for a given key
+        # and TSIG error to be used as a result of verify (normally faked
+        # one)
+        mock_ctx = MockTSIGContext(TSIG_KEY)
+        mock_ctx.error = error
+        return mock_ctx
+
+    def message_has_tsig(self, msg):
+        return msg.get_tsig_record() is not None
+
+    def create_request_data_with_tsig(self):
+        msg = Message(Message.RENDER)
+        query_id = 0x1035
+        msg.set_qid(query_id)
+        msg.set_opcode(Opcode.QUERY())
+        msg.set_rcode(Rcode.NOERROR())
+        query_question = Question(Name("example.com."), RRClass.IN(), RRType.AXFR())
+        msg.add_question(query_question)
+
+        renderer = MessageRenderer()
+        tsig_ctx = MockTSIGContext(TSIG_KEY)
+        msg.to_wire(renderer, tsig_ctx)
+        reply_data = renderer.get_data()
+        return reply_data
+
     def setUp(self):
         self.sock = MySocket(socket.AF_INET,socket.SOCK_STREAM)
         self.log = isc.log.NSLogger('xfrout', '',  severity = 'critical', log_to_console = False )
-        self.xfrsess = MyXfroutSession(self.sock, None, Dbserver(), self.log)
+        self.xfrsess = MyXfroutSession(self.sock, None, Dbserver(), self.log, TSIGKeyRing())
         self.mdata = bytes(b'\xd6=\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x07example\x03com\x00\x00\xfc\x00\x01')
         self.soa_record = (4, 3, 'example.com.', 'com.example.', 3600, 'SOA', None, 'master.example.com. admin.example.com. 1234 3600 1800 2419200 7200')
 
@@ -96,6 +125,18 @@ class TestXfroutSession(unittest.TestCase):
         [get_rcode, get_msg] = self.xfrsess._parse_query_message(self.mdata)
         self.assertEqual(get_rcode.to_text(), "NOERROR")
 
+        # tsig signed query message
+        request_data = self.create_request_data_with_tsig()
+        # BADKEY
+        [rcode, msg] = self.xfrsess._parse_query_message(request_data)
+        self.assertEqual(rcode.to_text(), "NOTAUTH")
+        self.assertTrue(self.xfrsess._tsig_ctx is not None)
+        # NOERROR
+        self.xfrsess._tsig_key_ring.add(TSIG_KEY)
+        [rcode, msg] = self.xfrsess._parse_query_message(request_data)
+        self.assertEqual(rcode.to_text(), "NOERROR")
+        self.assertTrue(self.xfrsess._tsig_ctx is not None)
+
     def test_get_query_zone_name(self):
         msg = self.getmsg()
         self.assertEqual(self.xfrsess._get_query_zone_name(msg), "example.com.")
@@ -111,6 +152,14 @@ class TestXfroutSession(unittest.TestCase):
         get_msg = self.sock.read_msg()
         self.assertEqual(get_msg.get_rcode().to_text(), "NXDOMAIN")
 
+        # tsig signed message
+        msg = self.getmsg()
+        self.xfrsess._tsig_ctx = self.create_mock_tsig_ctx(TSIGError.NOERROR)
+        self.xfrsess._reply_query_with_error_rcode(msg, self.sock, Rcode(3))
+        get_msg = self.sock.read_msg()
+        self.assertEqual(get_msg.get_rcode().to_text(), "NXDOMAIN")
+        self.assertTrue(self.message_has_tsig(get_msg))
+
     def test_send_message(self):
         msg = self.getmsg()
         msg.make_response()
@@ -152,6 +201,14 @@ class TestXfroutSession(unittest.TestCase):
         get_msg = self.sock.read_msg()
         self.assertEqual(get_msg.get_rcode().to_text(), "FORMERR")
 
+        # tsig signed message
+        msg = self.getmsg()
+        self.xfrsess._tsig_ctx = self.create_mock_tsig_ctx(TSIGError.NOERROR)
+        self.xfrsess._reply_query_with_format_error(msg, self.sock)
+        get_msg = self.sock.read_msg()
+        self.assertEqual(get_msg.get_rcode().to_text(), "FORMERR")
+        self.assertTrue(self.message_has_tsig(get_msg))
+
     def test_create_rrset_from_db_record(self):
         rrset = self.xfrsess._create_rrset_from_db_record(self.soa_record)
         self.assertEqual(rrset.get_name().to_text(), "example.com.")
@@ -162,11 +219,16 @@ class TestXfroutSession(unittest.TestCase):
 
     def test_send_message_with_last_soa(self):
         rrset_soa = self.xfrsess._create_rrset_from_db_record(self.soa_record)
-
         msg = self.getmsg()
         msg.make_response()
-        self.xfrsess._send_message_with_last_soa(msg, self.sock, rrset_soa, 0)
+
+        # packet number less than TSIG_SIGN_EVERY_NTH
+        packet_neet_not_sign = xfrout.TSIG_SIGN_EVERY_NTH - 1
+        self.xfrsess._send_message_with_last_soa(msg, self.sock, rrset_soa,
+                                                 0, packet_neet_not_sign)
         get_msg = self.sock.read_msg()
+        # tsig context is not exist
+        self.assertFalse(self.message_has_tsig(get_msg))
 
         self.assertEqual(get_msg.get_rr_count(Message.SECTION_QUESTION), 1)
         self.assertEqual(get_msg.get_rr_count(Message.SECTION_ANSWER), 1)
@@ -180,6 +242,42 @@ class TestXfroutSession(unittest.TestCase):
         rdata = answer.get_rdata()
         self.assertEqual(rdata[0].to_text(), self.soa_record[7])
 
+        # msg is the TSIG_SIGN_EVERY_NTH one
+        # sending the message with last soa together
+        self.xfrsess._send_message_with_last_soa(msg, self.sock, rrset_soa,
+                                                 0, TSIG_SIGN_EVERY_NTH)
+        get_msg = self.sock.read_msg()
+        # tsig context is not exist
+        self.assertFalse(self.message_has_tsig(get_msg))
+
+    def test_send_message_with_last_soa_with_tsig(self):
+        # create tsig context
+        self.xfrsess._tsig_ctx = self.create_mock_tsig_ctx(TSIGError.NOERROR)
+
+        rrset_soa = self.xfrsess._create_rrset_from_db_record(self.soa_record)
+        msg = self.getmsg()
+        msg.make_response()
+
+        # packet number less than TSIG_SIGN_EVERY_NTH
+        packet_neet_not_sign = xfrout.TSIG_SIGN_EVERY_NTH - 1
+        # msg is not the TSIG_SIGN_EVERY_NTH one
+        # sending the message with last soa together
+        self.xfrsess._send_message_with_last_soa(msg, self.sock, rrset_soa,
+                                                 0, packet_neet_not_sign)
+        get_msg = self.sock.read_msg()
+        self.assertTrue(self.message_has_tsig(get_msg))
+
+        self.assertEqual(get_msg.get_rr_count(Message.SECTION_QUESTION), 1)
+        self.assertEqual(get_msg.get_rr_count(Message.SECTION_ANSWER), 1)
+        self.assertEqual(get_msg.get_rr_count(Message.SECTION_AUTHORITY), 0)
+
+        # msg is the TSIG_SIGN_EVERY_NTH one
+        # sending the message with last soa together
+        self.xfrsess._send_message_with_last_soa(msg, self.sock, rrset_soa,
+                                                 0, TSIG_SIGN_EVERY_NTH)
+        get_msg = self.sock.read_msg()
+        self.assertTrue(self.message_has_tsig(get_msg))
+
     def test_trigger_send_message_with_last_soa(self):
         rrset_a = RRset(Name("example.com"), RRClass.IN(), RRType.A(), RRTTL(3600))
         rrset_a.add_rdata(Rdata(RRType.A(), RRClass.IN(), "192.0.2.1"))
@@ -187,15 +285,21 @@ class TestXfroutSession(unittest.TestCase):
 
         msg = self.getmsg()
         msg.make_response()
-
         msg.add_rrset(Message.SECTION_ANSWER, rrset_a)
-        # give the function a value that is larger than MAX-len(rrset)
-        self.xfrsess._send_message_with_last_soa(msg, self.sock, rrset_soa, 65520)
 
+        # length larger than MAX-len(rrset)
+        length_need_split = xfrout.XFROUT_MAX_MESSAGE_SIZE - get_rrset_len(rrset_soa) + 1
+        # packet number less than TSIG_SIGN_EVERY_NTH
+        packet_neet_not_sign = xfrout.TSIG_SIGN_EVERY_NTH - 1
+
+        # give the function a value that is larger than MAX-len(rrset)
         # this should have triggered the sending of two messages
         # (1 with the rrset we added manually, and 1 that triggered
         # the sending in _with_last_soa)
+        self.xfrsess._send_message_with_last_soa(msg, self.sock, rrset_soa, length_need_split,
+                                                 packet_neet_not_sign)
         get_msg = self.sock.read_msg()
+        self.assertFalse(self.message_has_tsig(get_msg))
         self.assertEqual(get_msg.get_rr_count(Message.SECTION_QUESTION), 1)
         self.assertEqual(get_msg.get_rr_count(Message.SECTION_ANSWER), 1)
         self.assertEqual(get_msg.get_rr_count(Message.SECTION_AUTHORITY), 0)
@@ -208,6 +312,7 @@ class TestXfroutSession(unittest.TestCase):
         self.assertEqual(rdata[0].to_text(), "192.0.2.1")
 
         get_msg = self.sock.read_msg()
+        self.assertFalse(self.message_has_tsig(get_msg))
         self.assertEqual(get_msg.get_rr_count(Message.SECTION_QUESTION), 0)
         self.assertEqual(get_msg.get_rr_count(Message.SECTION_ANSWER), 1)
         self.assertEqual(get_msg.get_rr_count(Message.SECTION_AUTHORITY), 0)
@@ -223,6 +328,45 @@ class TestXfroutSession(unittest.TestCase):
         # and it should not have sent anything else
         self.assertEqual(0, len(self.sock.sendqueue))
 
+    def test_trigger_send_message_with_last_soa_with_tsig(self):
+        self.xfrsess._tsig_ctx = self.create_mock_tsig_ctx(TSIGError.NOERROR)
+        rrset_soa = self.xfrsess._create_rrset_from_db_record(self.soa_record)
+        msg = self.getmsg()
+        msg.make_response()
+        msg.add_rrset(Message.SECTION_ANSWER, rrset_soa)
+
+        # length larger than MAX-len(rrset)
+        length_need_split = xfrout.XFROUT_MAX_MESSAGE_SIZE - get_rrset_len(rrset_soa) + 1
+        # packet number less than TSIG_SIGN_EVERY_NTH
+        packet_neet_not_sign = xfrout.TSIG_SIGN_EVERY_NTH - 1
+
+        # give the function a value that is larger than MAX-len(rrset)
+        # this should have triggered the sending of two messages
+        # (1 with the rrset we added manually, and 1 that triggered
+        # the sending in _with_last_soa)
+        self.xfrsess._send_message_with_last_soa(msg, self.sock, rrset_soa, length_need_split,
+                                                 packet_neet_not_sign)
+        get_msg = self.sock.read_msg()
+        # msg is not the TSIG_SIGN_EVERY_NTH one, it shouldn't be tsig signed
+        self.assertFalse(self.message_has_tsig(get_msg))
+        # the last packet should be tsig signed
+        get_msg = self.sock.read_msg()
+        self.assertTrue(self.message_has_tsig(get_msg))
+        # and it should not have sent anything else
+        self.assertEqual(0, len(self.sock.sendqueue))
+
+
+        # msg is the TSIG_SIGN_EVERY_NTH one, it should be tsig signed
+        self.xfrsess._send_message_with_last_soa(msg, self.sock, rrset_soa, length_need_split,
+                                                 xfrout.TSIG_SIGN_EVERY_NTH)
+        get_msg = self.sock.read_msg()
+        self.assertTrue(self.message_has_tsig(get_msg))
+        # the last packet should be tsig signed
+        get_msg = self.sock.read_msg()
+        self.assertTrue(self.message_has_tsig(get_msg))
+        # and it should not have sent anything else
+        self.assertEqual(0, len(self.sock.sendqueue))
+
     def test_get_rrset_len(self):
         rrset_soa = self.xfrsess._create_rrset_from_db_record(self.soa_record)
         self.assertEqual(82, get_rrset_len(rrset_soa))
@@ -313,6 +457,51 @@ class TestXfroutSession(unittest.TestCase):
         reply_msg = self.sock.read_msg()
         self.assertEqual(reply_msg.get_rr_count(Message.SECTION_ANSWER), 2)
 
+    def test_reply_xfrout_query_noerror_with_tsig(self):
+        rrset_data = (4, 3, 'a.example.com.', 'com.example.', 3600, 'A', None, '192.168.1.1')
+        global sqlite3_ds
+        global xfrout
+        def get_zone_soa(zonename, file):
+            return self.soa_record
+
+        def get_zone_datas(zone, file):
+            zone_rrsets = []
+            for i in range(0, 100):
+                zone_rrsets.insert(i, rrset_data)
+            return zone_rrsets
+
+        def get_rrset_len(rrset):
+            return 65520
+
+        sqlite3_ds.get_zone_soa = get_zone_soa
+        sqlite3_ds.get_zone_datas = get_zone_datas
+        xfrout.get_rrset_len = get_rrset_len
+
+        self.xfrsess._tsig_ctx = self.create_mock_tsig_ctx(TSIGError.NOERROR)
+        self.xfrsess._reply_xfrout_query(self.getmsg(), self.sock, "example.com.")
+
+        # tsig signed first package
+        reply_msg = self.sock.read_msg()
+        self.assertEqual(reply_msg.get_rr_count(Message.SECTION_ANSWER), 1)
+        self.assertTrue(self.message_has_tsig(reply_msg))
+        # (TSIG_SIGN_EVERY_NTH - 1) packets have no tsig
+        for i in range(0, xfrout.TSIG_SIGN_EVERY_NTH - 1):
+            reply_msg = self.sock.read_msg()
+            self.assertFalse(self.message_has_tsig(reply_msg))
+        # TSIG_SIGN_EVERY_NTH packet has tsig
+        reply_msg = self.sock.read_msg()
+        self.assertTrue(self.message_has_tsig(reply_msg))
+
+        for i in range(0, 100 - TSIG_SIGN_EVERY_NTH):
+            reply_msg = self.sock.read_msg()
+            self.assertFalse(self.message_has_tsig(reply_msg))
+        # tsig signed last package
+        reply_msg = self.sock.read_msg()
+        self.assertTrue(self.message_has_tsig(reply_msg))
+
+        # and it should not have sent anything else
+        self.assertEqual(0, len(self.sock.sendqueue))
+
 class MyCCSession():
     def __init__(self):
         pass
@@ -347,8 +536,23 @@ class TestUnixSockServer(unittest.TestCase):
         self.assertEqual(recv_msg, send_msg)
 
     def test_updata_config_data(self):
+        tsig_key_str = 'example.com:SFuWd/q99SzF8Yzd1QbB9g=='
+        tsig_key_list = [tsig_key_str]
+        bad_key_list = ['bad..example.com:SFuWd/q99SzF8Yzd1QbB9g==']
         self.unix.update_config_data({'transfers_out':10 })
         self.assertEqual(self.unix._max_transfers_out, 10)
+        self.assertTrue(self.unix.tsig_key_ring is not None)
+
+        self.unix.update_config_data({'transfers_out':9, 'tsig_key_ring':tsig_key_list})
+        self.assertEqual(self.unix._max_transfers_out, 9)
+        self.assertEqual(self.unix.tsig_key_ring.size(), 1)
+        self.unix.tsig_key_ring.remove(Name("example.com."))
+        self.assertEqual(self.unix.tsig_key_ring.size(), 0)
+
+        # bad tsig key
+        config_data = {'transfers_out':9, 'tsig_key_ring': bad_key_list}
+        self.assertRaises(None, self.unix.update_config_data(config_data))
+        self.assertEqual(self.unix.tsig_key_ring.size(), 0)
 
     def test_get_db_file(self):
         self.assertEqual(self.unix.get_db_file(), "initdb.file")

src/bin/xfrout/xfrout.py.in

@@ -74,10 +74,12 @@ SPECFILE_LOCATION = SPECFILE_PATH + "/xfrout.spec"
 AUTH_SPECFILE_LOCATION = AUTH_SPECFILE_PATH + os.sep + "auth.spec"
 MAX_TRANSFERS_OUT = 10
 VERBOSE_MODE = False
-
+# tsig sign every N axfr packets.
+TSIG_SIGN_EVERY_NTH = 96
 
 XFROUT_MAX_MESSAGE_SIZE = 65535
 
+
 def get_rrset_len(rrset):
     """Returns the wire length of the given RRset"""
     bytes = bytearray()
@@ -86,15 +88,22 @@ def get_rrset_len(rrset):
 
 
 class XfroutSession():
-    def __init__(self, sock_fd, request_data, server, log):
+    def __init__(self, sock_fd, request_data, server, log, tsig_key_ring):
         # The initializer for the superclass may call functions
         # that need _log to be set, so we set it first
         self._sock_fd = sock_fd
         self._request_data = request_data
         self._server = server
         self._log = log
+        self._tsig_key_ring = tsig_key_ring
+        self._tsig_ctx = None
+        self._tsig_len = 0
         self.handle()
 
+    def create_tsig_ctx(self, tsig_record, tsig_key_ring):
+        return TSIGContext(tsig_record.get_name(), tsig_record.get_rdata().get_algorithm(),
+                           tsig_key_ring)
+
     def handle(self):
         ''' Handle a xfrout query, send xfrout response '''
         try:
@@ -105,17 +114,33 @@ class XfroutSession():
 
         os.close(self._sock_fd)
 
+    def _check_request_tsig(self, msg, request_data):
+        ''' If request has a tsig record, perform tsig related checks '''
+        tsig_record = msg.get_tsig_record()
+        if tsig_record is not None:
+            self._tsig_len = tsig_record.get_length()
+            self._tsig_ctx = self.create_tsig_ctx(tsig_record, self._tsig_key_ring)
+            tsig_error = self._tsig_ctx.verify(tsig_record, request_data)
+            if tsig_error != TSIGError.NOERROR:
+                return Rcode.NOTAUTH()
+
+        return Rcode.NOERROR()
+
     def _parse_query_message(self, mdata):
         ''' parse query message to [socket,message]'''
         #TODO, need to add parseHeader() in case the message header is invalid
         try:
             msg = Message(Message.PARSE)
             Message.from_wire(msg, mdata)
+
+            # TSIG related checks
+            rcode = self._check_request_tsig(msg, mdata)
+
         except Exception as err:
             self._log.log_message("error", str(err))
             return Rcode.FORMERR(), None
 
-        return Rcode.NOERROR(), msg
+        return rcode, msg
 
     def _get_query_zone_name(self, msg):
         question = msg.get_question()[0]
@@ -130,13 +155,20 @@ class XfroutSession():
             total_count += count
 
 
-    def _send_message(self, sock_fd, msg):
+    def _send_message(self, sock_fd, msg, tsig_ctx=None):
         render = MessageRenderer()
         # As defined in RFC5936 section3.4, perform case-preserving name
         # compression for AXFR message.
         render.set_compress_mode(MessageRenderer.CASE_SENSITIVE)
         render.set_length_limit(XFROUT_MAX_MESSAGE_SIZE)
-        msg.to_wire(render)
+
+        # XXX Currently, python wrapper doesn't accept 'None' parameter in this case,
+        # we should remove the if statement and use a universal interface later.
+        if tsig_ctx is not None:
+            msg.to_wire(render, tsig_ctx)
+        else:
+            msg.to_wire(render)
+
         header_len = struct.pack('H', socket.htons(render.get_length()))
         self._send_data(sock_fd, header_len)
         self._send_data(sock_fd, render.get_data())
@@ -145,7 +177,7 @@ class XfroutSession():
     def _reply_query_with_error_rcode(self, msg, sock_fd, rcode_):
         msg.make_response()
         msg.set_rcode(rcode_)
-        self._send_message(sock_fd, msg)
+        self._send_message(sock_fd, msg, self._tsig_ctx)
 
 
     def _reply_query_with_format_error(self, msg, sock_fd):
@@ -155,7 +187,7 @@ class XfroutSession():
 
         msg.make_response()
         msg.set_rcode(Rcode.FORMERR())
-        self._send_message(sock_fd, msg)
+        self._send_message(sock_fd, msg, self._tsig_ctx)
 
     def _zone_has_soa(self, zone):
         '''Judge if the zone has an SOA record.'''
@@ -204,7 +236,9 @@ class XfroutSession():
     def dns_xfrout_start(self, sock_fd, msg_query):
         rcode_, msg = self._parse_query_message(msg_query)
         #TODO. create query message and parse header
-        if rcode_ != Rcode.NOERROR():
+        if rcode_ == Rcode.NOTAUTH():
+            return self._reply_query_with_error_rcode(msg, sock_fd, rcode_)
+        elif rcode_ != Rcode.NOERROR():
             return self._reply_query_with_format_error(msg, sock_fd)
 
         zone_name = self._get_query_zone_name(msg)
@@ -248,37 +282,43 @@ class XfroutSession():
         rrset_.add_rdata(rdata_)
         return rrset_
 
-    def _send_message_with_last_soa(self, msg, sock_fd, rrset_soa, message_upper_len):
+    def _send_message_with_last_soa(self, msg, sock_fd, rrset_soa, message_upper_len,
+                                    count_since_last_tsig_sign):
         '''Add the SOA record to the end of message. If it can't be
         added, a new message should be created to send out the last soa .
         '''
         rrset_len = get_rrset_len(rrset_soa)
 
-        if message_upper_len + rrset_len < XFROUT_MAX_MESSAGE_SIZE:
-            msg.add_rrset(Message.SECTION_ANSWER, rrset_soa)
-        else:
+        if (count_since_last_tsig_sign == TSIG_SIGN_EVERY_NTH and
+            message_upper_len + rrset_len >= XFROUT_MAX_MESSAGE_SIZE):
+            # If tsig context exist, sign the packet with serial number TSIG_SIGN_EVERY_NTH
+            self._send_message(sock_fd, msg, self._tsig_ctx)
+            msg = self._clear_message(msg)
+        elif (count_since_last_tsig_sign != TSIG_SIGN_EVERY_NTH and
+              message_upper_len + rrset_len + self._tsig_len >= XFROUT_MAX_MESSAGE_SIZE):
             self._send_message(sock_fd, msg)
             msg = self._clear_message(msg)
-            msg.add_rrset(Message.SECTION_ANSWER, rrset_soa)
 
-        self._send_message(sock_fd, msg)
+        # If tsig context exist, sign the last packet
+        msg.add_rrset(Message.SECTION_ANSWER, rrset_soa)
+        self._send_message(sock_fd, msg, self._tsig_ctx)
 
 
     def _reply_xfrout_query(self, msg, sock_fd, zone_name):
         #TODO, there should be a better way to insert rrset.
+        count_since_last_tsig_sign = TSIG_SIGN_EVERY_NTH
         msg.make_response()
         msg.set_header_flag(Message.HEADERFLAG_AA)
         soa_record = sqlite3_ds.get_zone_soa(zone_name, self._server.get_db_file())
         rrset_soa = self._create_rrset_from_db_record(soa_record)
         msg.add_rrset(Message.SECTION_ANSWER, rrset_soa)
 
-        message_upper_len = get_rrset_len(rrset_soa)
+        message_upper_len = get_rrset_len(rrset_soa) + self._tsig_len
 
         for rr_data in sqlite3_ds.get_zone_datas(zone_name, self._server.get_db_file()):
             if  self._server._shutdown_event.is_set(): # Check if xfrout is shutdown
                 self._log.log_message("info", "xfrout process is being shutdown")
                 return
-
             # TODO: RRType.SOA() ?
             if RRType(rr_data[5]) == RRType("SOA"): #ignore soa record
                 continue
@@ -294,12 +334,25 @@ class XfroutSession():
                 message_upper_len += rrset_len
                 continue
 
-            self._send_message(sock_fd, msg)
+            # If tsig context exist, sign every N packets
+            if count_since_last_tsig_sign == TSIG_SIGN_EVERY_NTH:
+                count_since_last_tsig_sign = 0
+                self._send_message(sock_fd, msg, self._tsig_ctx)
+            else:
+                self._send_message(sock_fd, msg)
+
+            count_since_last_tsig_sign += 1
             msg = self._clear_message(msg)
             msg.add_rrset(Message.SECTION_ANSWER, rrset_) # Add the rrset to the new message
-            message_upper_len = rrset_len
 
-        self._send_message_with_last_soa(msg, sock_fd, rrset_soa, message_upper_len)
+            # Reserve tsig space for signed packet
+            if count_since_last_tsig_sign == TSIG_SIGN_EVERY_NTH:
+                message_upper_len = rrset_len + self._tsig_len
+            else:
+                message_upper_len = rrset_len
+
+        self._send_message_with_last_soa(msg, sock_fd, rrset_soa, message_upper_len,
+                                         count_since_last_tsig_sign)
 
 class UnixSockServer(socketserver_mixin.NoPollMixIn, ThreadingUnixStreamServer):
     '''The unix domain socket server which accept xfr query sent from auth server.'''
@@ -403,7 +456,7 @@ class UnixSockServer(socketserver_mixin.NoPollMixIn, ThreadingUnixStreamServer):
 
     def finish_request(self, sock_fd, request_data):
         '''Finish one request by instantiating RequestHandlerClass.'''
-        self.RequestHandlerClass(sock_fd, request_data, self, self._log)
+        self.RequestHandlerClass(sock_fd, request_data, self, self._log, self.tsig_key_ring)
 
     def _remove_unused_sock_file(self, sock_file):
         '''Try to remove the socket file. If the file is being used
@@ -449,10 +502,27 @@ class UnixSockServer(socketserver_mixin.NoPollMixIn, ThreadingUnixStreamServer):
         self._log.log_message('info', 'update config data start.')
         self._lock.acquire()
         self._max_transfers_out = new_config.get('transfers_out')
+        self.set_tsig_key_ring(new_config.get('tsig_key_ring'))
         self._log.log_message('info', 'max transfer out : %d', self._max_transfers_out)
         self._lock.release()
         self._log.log_message('info', 'update config data complete.')
 
+    def set_tsig_key_ring(self, key_list):
+        """Set the tsig_key_ring , given a TSIG key string list representation. """
+
+        # XXX add values to configure zones/tsig options
+        self.tsig_key_ring = TSIGKeyRing()
+        # If key string list is empty, create a empty tsig_key_ring
+        if not key_list:
+            return
+
+        for key_item in key_list:
+            try:
+                self.tsig_key_ring.add(TSIGKey(key_item))
+            except InvalidParameter as ipe:
+                errmsg = "bad TSIG key string: " + str(key_item)
+                self._log.log_message('error', '%s' % errmsg)
+
     def get_db_file(self):
         file, is_default = self._cc.get_remote_config_value("Auth", "database_file")
         # this too should be unnecessary, but currently the

src/bin/xfrout/xfrout.spec.pre.in

@@ -37,6 +37,18 @@
     	 "item_type": "integer",
          "item_optional": false,
     	 "item_default": 1048576
+       },
+       {
+         "item_name": "tsig_key_ring",
+         "item_type": "list",
+         "item_optional": true,
+         "item_default": [],
+         "list_item_spec" :
+         {
+             "item_name": "tsig_key",
+             "item_type": "string",
+             "item_optional": true
+         }
        }
       ],
       "commands": [

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/asiodns/io_fetch.cc

@@ -209,16 +209,6 @@ IOFetch::IOFetch(Protocol protocol, IOService& service,
     msg->setHeaderFlag(Message::HEADERFLAG_CD,
                        query_message->getHeaderFlag(Message::HEADERFLAG_CD));
 
-    ConstEDNSPtr edns(query_message->getEDNS());
-    const bool dnssec_ok = edns && edns->getDNSSECAwareness();
-    if (edns) {
-        EDNSPtr edns_response(new EDNS());
-        edns_response->setDNSSECAwareness(dnssec_ok);
-        // TODO: We should make our own edns bufsize length configurable
-        edns_response->setUDPSize(Message::DEFAULT_MAX_EDNS0_UDPSIZE);
-        msg->setEDNS(edns_response);
-    }
-
     initIOFetch(msg, protocol, service,
                 **(query_message->beginQuestion()),
                 address, port, buff, cb, wait);
@@ -238,6 +228,9 @@ IOFetch::initIOFetch(MessagePtr& query_msg, Protocol protocol, IOService& servic
     query_msg->setRcode(Rcode::NOERROR());
     query_msg->setHeaderFlag(Message::HEADERFLAG_RD);
     query_msg->addQuestion(question);
+    EDNSPtr edns_query(new EDNS());
+    edns_query->setUDPSize(Message::DEFAULT_MAX_EDNS0_UDPSIZE);
+    query_msg->setEDNS(edns_query);
     MessageRenderer renderer(*data_->msgbuf);
     query_msg->toWire(renderer);
 }

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/io_fetch_unittest.cc

@@ -130,6 +130,9 @@ public:
         msg.setRcode(Rcode::NOERROR());
         msg.setHeaderFlag(Message::HEADERFLAG_RD);
         msg.addQuestion(question_);
+        EDNSPtr msg_edns(new EDNS());
+        msg_edns->setUDPSize(Message::DEFAULT_MAX_EDNS0_UDPSIZE);
+        msg.setEDNS(msg_edns);
         MessageRenderer renderer(*msgbuf_);
         msg.toWire(renderer);
         MessageRenderer renderer2(*expected_buffer_);

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

@@ -8,12 +8,17 @@ CLEANFILES = *.gcno *.gcda
 lib_LTLIBRARIES = liblog.la
 liblog_la_SOURCES  =
 liblog_la_SOURCES += dummylog.h dummylog.cc
+liblog_la_SOURCES += impldef.cc impldef.h
 liblog_la_SOURCES += log_formatter.h log_formatter.cc
 liblog_la_SOURCES += logger.cc logger.h
 liblog_la_SOURCES += logger_impl.cc logger_impl.h
 liblog_la_SOURCES += logger_level.h
-liblog_la_SOURCES += logger_level.h
+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
 liblog_la_SOURCES += messagedef.cc messagedef.h
@@ -22,9 +27,10 @@ liblog_la_SOURCES += message_exception.h
 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 += root_logger_name.cc root_logger_name.h
+liblog_la_SOURCES += output_option.cc output_option.h
 
 EXTRA_DIST  = README
+EXTRA_DIST += impldef.mes
 EXTRA_DIST += messagedef.mes
 
 # Note: the ordering matters: -Wno-... must follow -Wextra (defined in

src/lib/log/compiler/message.cc

@@ -35,6 +35,8 @@
 
 #include <log/logger.h>
 
+#include <boost/foreach.hpp>
+
 using namespace std;
 using namespace isc::log;
 using namespace isc::util;
@@ -78,10 +80,11 @@ version() {
 void
 usage() {
     cout <<
-        "Usage: message [-h] [-v] <message-file>\n" <<
+        "Usage: message [-h] [-v] [-p] <message-file>\n" <<
         "\n" <<
         "-h       Print this message and exit\n" <<
         "-v       Print the program version and exit\n" <<
+        "-p       Output python source instead of C++ ones\n" <<
         "\n" <<
         "<message-file> is the name of the input message file.\n";
 }
@@ -237,6 +240,44 @@ writeClosingNamespace(ostream& output, const vector<string>& ns) {
     }
 }
 
+/// \breif Write python file
+///
+/// Writes the python file containing the symbol definitions as module level
+/// constants. These are objects which register themself at creation time,
+/// so they can be replaced by dictionary later.
+///
+/// \param file Name of the message file. The source code is written to a file
+///     file of the same name but with a .py suffix.
+/// \param dictionary The dictionary holding the message definitions.
+///
+/// \note We don't use the namespace as in C++. We don't need it, because
+///     python file/module works as implicit namespace as well.
+
+void
+writePythonFile(const string& file, MessageDictionary& dictionary) {
+    Filename message_file(file);
+    Filename python_file(Filename(message_file.name()).useAsDefault(".py"));
+
+    // Open the file for writing
+    ofstream pyfile(python_file.fullName().c_str());
+
+    // Write the comment and imports
+    pyfile <<
+        "# File created from " << message_file.fullName() << " on " <<
+            currentTime() << "\n" <<
+        "\n" <<
+        "import isc.log.message\n" <<
+        "\n";
+
+    vector<string> idents(sortedIdentifiers(dictionary));
+    BOOST_FOREACH(const string& ident, idents) {
+        pyfile << ident << " = isc.log.message.create(\"" <<
+            ident << "\", \"" << quoteString(dictionary.getText(ident)) <<
+            "\")\n";
+    }
+
+    pyfile.close();
+}
 
 /// \brief Write Header File
 ///
@@ -264,52 +305,46 @@ writeHeaderFile(const string& file, const vector<string>& ns_components,
     // Open the output file for writing
     ofstream hfile(header_file.fullName().c_str());
 
-    try {
-        if (hfile.fail()) {
-            throw MessageException(MSG_OPENOUT, header_file.fullName(),
-                strerror(errno));
-        }
-
-        // Write the header preamble.  If there is an error, we'll pick it up
-        // after the last write.
-
-        hfile <<
-            "// File created from " << message_file.fullName() << " on " <<
-                currentTime() << "\n" <<
-             "\n" <<
-             "#ifndef " << sentinel_text << "\n" <<
-             "#define "  << sentinel_text << "\n" <<
-             "\n" <<
-             "#include <log/message_types.h>\n" <<
-             "\n";
-
-        // Write the message identifiers, bounded by a namespace declaration
-        writeOpeningNamespace(hfile, ns_components);
-
-        vector<string> idents = sortedIdentifiers(dictionary);
-        for (vector<string>::const_iterator j = idents.begin();
-            j != idents.end(); ++j) {
-            hfile << "extern const isc::log::MessageID " << *j << ";\n";
-        }
-        hfile << "\n";
+    if (hfile.fail()) {
+        throw MessageException(MSG_OPENOUT, header_file.fullName(),
+            strerror(errno));
+    }
 
-        writeClosingNamespace(hfile, ns_components);
+    // Write the header preamble.  If there is an error, we'll pick it up
+    // after the last write.
+
+    hfile <<
+        "// File created from " << message_file.fullName() << " on " <<
+            currentTime() << "\n" <<
+         "\n" <<
+         "#ifndef " << sentinel_text << "\n" <<
+         "#define "  << sentinel_text << "\n" <<
+         "\n" <<
+         "#include <log/message_types.h>\n" <<
+         "\n";
+
+    // Write the message identifiers, bounded by a namespace declaration
+    writeOpeningNamespace(hfile, ns_components);
+
+    vector<string> idents = sortedIdentifiers(dictionary);
+    for (vector<string>::const_iterator j = idents.begin();
+        j != idents.end(); ++j) {
+        hfile << "extern const isc::log::MessageID " << *j << ";\n";
+    }
+    hfile << "\n";
 
-        // ... and finally the postamble
-        hfile << "#endif // " << sentinel_text << "\n";
+    writeClosingNamespace(hfile, ns_components);
 
-        // Report errors (if any) and exit
-        if (hfile.fail()) {
-            throw MessageException(MSG_WRITERR, header_file.fullName(),
-                strerror(errno));
-        }
+    // ... and finally the postamble
+    hfile << "#endif // " << sentinel_text << "\n";
 
-        hfile.close();
-    }
-    catch (MessageException&) {
-        hfile.close();
-        throw;
+    // Report errors (if any) and exit
+    if (hfile.fail()) {
+        throw MessageException(MSG_WRITERR, header_file.fullName(),
+            strerror(errno));
     }
+
+    hfile.close();
 }
 
 
@@ -357,76 +392,71 @@ writeProgramFile(const string& file, const vector<string>& ns_components,
 
     // Open the output file for writing
     ofstream ccfile(program_file.fullName().c_str());
-    try {
-        if (ccfile.fail()) {
-            throw MessageException(MSG_OPENOUT, program_file.fullName(),
-                strerror(errno));
-        }
 
-        // Write the preamble.  If there is an error, we'll pick it up after
-        // the last write.
+    if (ccfile.fail()) {
+        throw MessageException(MSG_OPENOUT, program_file.fullName(),
+            strerror(errno));
+    }
 
-        ccfile <<
-            "// File created from " << message_file.fullName() << " on " <<
-                currentTime() << "\n" <<
-             "\n" <<
-             "#include <cstddef>\n" <<
-             "#include <log/message_types.h>\n" <<
-             "#include <log/message_initializer.h>\n" <<
-             "\n";
+    // Write the preamble.  If there is an error, we'll pick it up after
+    // the last write.
 
-        // Declare the message symbols themselves.
+    ccfile <<
+        "// File created from " << message_file.fullName() << " on " <<
+            currentTime() << "\n" <<
+         "\n" <<
+         "#include <cstddef>\n" <<
+         "#include <log/message_types.h>\n" <<
+         "#include <log/message_initializer.h>\n" <<
+         "\n";
 
-        writeOpeningNamespace(ccfile, ns_components);
+    // Declare the message symbols themselves.
 
-        vector<string> idents = sortedIdentifiers(dictionary);
-        for (vector<string>::const_iterator j = idents.begin();
-            j != idents.end(); ++j) {
-            ccfile << "extern const isc::log::MessageID " << *j <<
-                " = \"" << *j << "\";\n";
-        }
-        ccfile << "\n";
+    writeOpeningNamespace(ccfile, ns_components);
 
-        writeClosingNamespace(ccfile, ns_components);
+    vector<string> idents = sortedIdentifiers(dictionary);
+    for (vector<string>::const_iterator j = idents.begin();
+        j != idents.end(); ++j) {
+        ccfile << "extern const isc::log::MessageID " << *j <<
+            " = \"" << *j << "\";\n";
+    }
+    ccfile << "\n";
 
-        // Now the code for the message initialization.
+    writeClosingNamespace(ccfile, ns_components);
 
-        ccfile <<
-             "namespace {\n" <<
-             "\n" <<
-             "const char* values[] = {\n";
+    // Now the code for the message initialization.
 
-        // Output the identifiers and the associated text.
-        idents = sortedIdentifiers(dictionary);
-        for (vector<string>::const_iterator i = idents.begin();
-            i != idents.end(); ++i) {
-                ccfile << "    \"" << *i << "\", \"" <<
-                    quoteString(dictionary.getText(*i)) << "\",\n";
-        }
+    ccfile <<
+         "namespace {\n" <<
+         "\n" <<
+         "const char* values[] = {\n";
 
+    // Output the identifiers and the associated text.
+    idents = sortedIdentifiers(dictionary);
+    for (vector<string>::const_iterator i = idents.begin();
+        i != idents.end(); ++i) {
+            ccfile << "    \"" << *i << "\", \"" <<
+                quoteString(dictionary.getText(*i)) << "\",\n";
+    }
 
-        // ... and the postamble
-        ccfile <<
-            "    NULL\n" <<
-            "};\n" <<
-            "\n" <<
-            "const isc::log::MessageInitializer initializer(values);\n" <<
-            "\n" <<
-            "} // Anonymous namespace\n" <<
-            "\n";
-
-        // Report errors (if any) and exit
-        if (ccfile.fail()) {
-            throw MessageException(MSG_WRITERR, program_file.fullName(),
-                strerror(errno));
-        }
 
-        ccfile.close();
-    }
-    catch (MessageException&) {
-        ccfile.close();
-        throw;
+    // ... and the postamble
+    ccfile <<
+        "    NULL\n" <<
+        "};\n" <<
+        "\n" <<
+        "const isc::log::MessageInitializer initializer(values);\n" <<
+        "\n" <<
+        "} // Anonymous namespace\n" <<
+        "\n";
+
+    // Report errors (if any) and exit
+    if (ccfile.fail()) {
+        throw MessageException(MSG_WRITERR, program_file.fullName(),
+            strerror(errno));
     }
+
+    ccfile.close();
 }
 
 
@@ -466,13 +496,19 @@ warnDuplicates(MessageReader& reader) {
 int
 main(int argc, char* argv[]) {
 
-    const char* soptions = "hv";               // Short options
+    const char* soptions = "hvp";               // Short options
 
     optind = 1;             // Ensure we start a new scan
     int  opt;               // Value of the option
 
+    bool doPython = false;
+
     while ((opt = getopt(argc, argv, soptions)) != -1) {
         switch (opt) {
+            case 'p':
+                doPython = true;
+                break;
+
             case 'h':
                 usage();
                 return 0;
@@ -508,15 +544,27 @@ main(int argc, char* argv[]) {
         MessageReader reader(&dictionary);
         reader.readFile(message_file);
 
-        // Get the namespace into which the message definitions will be put and
-        // split it into components.
-        vector<string> ns_components = splitNamespace(reader.getNamespace());
-
-        // Write the header file.
-        writeHeaderFile(message_file, ns_components, dictionary);
-
-        // Write the file that defines the message symbols and text
-        writeProgramFile(message_file, ns_components, dictionary);
+        if (doPython) {
+            // Warn in case of ignored directives
+            if (!reader.getNamespace().empty()) {
+                cerr << "Python mode, ignoring the $NAMESPACE directive" <<
+                    endl;
+            }
+
+            // Write the whole python file
+            writePythonFile(message_file, dictionary);
+        } else {
+            // Get the namespace into which the message definitions will be put and
+            // split it into components.
+            vector<string> ns_components =
+                splitNamespace(reader.getNamespace());
+
+            // Write the header file.
+            writeHeaderFile(message_file, ns_components, dictionary);
+
+            // Write the file that defines the message symbols and text
+            writeProgramFile(message_file, ns_components, dictionary);
+        }
 
         // Finally, warn of any duplicates encountered.
         warnDuplicates(reader);

src/lib/log/impldef.cc

@@ -0,0 +1,29 @@
+// File created from impldef.mes on Wed Jun  1 10:32:57 2011
+
+#include <cstddef>
+#include <log/message_types.h>
+#include <log/message_initializer.h>
+
+namespace isc {
+namespace log {
+
+extern const isc::log::MessageID LOGIMPL_ABOVEDBGMAX = "LOGIMPL_ABOVEDBGMAX";
+extern const isc::log::MessageID LOGIMPL_BADDEBUG = "LOGIMPL_BADDEBUG";
+extern const isc::log::MessageID LOGIMPL_BELOWDBGMIN = "LOGIMPL_BELOWDBGMIN";
+
+} // namespace log
+} // namespace isc
+
+namespace {
+
+const char* values[] = {
+    "LOGIMPL_ABOVEDBGMAX", "debug level of %1 is too high and will be set to the maximum of %2",
+    "LOGIMPL_BADDEBUG", "debug string is '%1': must be of the form DEBUGn",
+    "LOGIMPL_BELOWDBGMIN", "debug level of %1 is too low and will be set to the minimum of %2",
+    NULL
+};
+
+const isc::log::MessageInitializer initializer(values);
+
+} // Anonymous namespace
+

src/lib/log/impldef.h

@@ -0,0 +1,18 @@
+// File created from impldef.mes on Wed Jun  1 10:32:57 2011
+
+#ifndef __IMPLDEF_H
+#define __IMPLDEF_H
+
+#include <log/message_types.h>
+
+namespace isc {
+namespace log {
+
+extern const isc::log::MessageID LOGIMPL_ABOVEDBGMAX;
+extern const isc::log::MessageID LOGIMPL_BADDEBUG;
+extern const isc::log::MessageID LOGIMPL_BELOWDBGMIN;
+
+} // namespace log
+} // namespace isc
+
+#endif // __IMPLDEF_H

src/lib/log/impldef.mes

@@ -0,0 +1,38 @@
+# 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 Logger Implementation Messages
+#
+# This holds messages generated by the underlying logger implementation.  They
+# are likely to be specific to that implementation, and may well change if the
+# underlying implementation is changed.  For that reason, they have been put
+# in a separate file.
+
+$PREFIX LOGIMPL_
+$NAMESPACE isc::log
+
+% ABOVEDBGMAX   debug level of %1 is too high and will be set to the maximum of %2
+A message from the underlying logger implementation code, the debug level
+(as set by the string DEBGUGn) is above the maximum allowed value and has
+been reduced to that value.
+
+% BADDEBUG      debug string is '%1': must be of the form DEBUGn
+The string indicating the extended logging level (used by the underlying
+logger implementation code) is not of the stated form.  In particular,
+it starts DEBUG but does not end with an integer.
+
+% BELOWDBGMIN   debug level of %1 is too low and will be set to the minimum of %2
+A message from the underlying logger implementation code, the debug level
+(as set by the string DEBGUGn) is below the minimum allowed value and has
+been increased to that value.

src/lib/log/log_formatter.h

@@ -15,7 +15,9 @@
 #ifndef __LOG_FORMATTER_H
 #define __LOG_FORMMATER_H
 
+#include <cstddef>
 #include <string>
+#include <iostream>
 #include <boost/lexical_cast.hpp>
 #include <log/logger_level.h>
 
@@ -84,7 +86,6 @@ private:
     /// \brief Which will be the next placeholder to replace
     unsigned nextPlaceholder_;
 
-    Formatter& operator =(const Formatter& other);
 
 public:
     /// \brief Constructor of "active" formatter
@@ -108,12 +109,18 @@ public:
     {
     }
 
+    /// \brief Copy constructor
+    ///
+    /// "Control" is passed to the created object in that it is the created object
+    /// that will have responsibility for outputting the formatted message - the
+    /// object being copied relinquishes that responsibility.
     Formatter(const Formatter& other) :
         logger_(other.logger_), severity_(other.severity_),
         message_(other.message_), nextPlaceholder_(other.nextPlaceholder_)
     {
-        other.logger_ = false;
+        other.logger_ = NULL;
     }
+
     /// \brief Destructor.
     //
     /// This is the place where output happens if the formatter is active.
@@ -123,6 +130,23 @@ public:
             delete message_;
         }
     }
+
+    /// \brief Assignment operator
+    ///
+    /// Essentially the same function as the assignment operator - the object being
+    /// assigned to takes responsibility for outputting the message.
+    Formatter& operator =(const Formatter& other) {
+        if (&other != this) {
+            logger_ = other.logger_;
+            severity_ = other.severity_;
+            message_ = other.message_;
+            nextPlaceholder_ = other.nextPlaceholder_;
+            other.logger_ = NULL;
+        }
+
+        return *this;
+    }
+
     /// \brief Replaces another placeholder
     ///
     /// Replaces another placeholder and returns a new formatter with it.
@@ -137,6 +161,7 @@ public:
             return (*this);
         }
     }
+
     /// \brief String version of arg.
     Formatter& arg(const std::string& arg) {
         if (logger_) {
@@ -154,6 +179,7 @@ public:
         }
         return (*this);
     }
+
 };
 
 }

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>
 
@@ -28,8 +28,7 @@ using namespace std;
 namespace isc {
 namespace log {
 
-// Initialize Logger implementation.  Does not check whether the implementation
-// has already been initialized - that was done by the caller (getLoggerPtr()).
+// Initialize Logger.
 void Logger::initLoggerImpl() {
     loggerptr_ = new LoggerImpl(name_);
 }
@@ -75,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
@@ -173,12 +180,5 @@ Logger::operator==(Logger& other) {
     return (*getLoggerPtr() == *other.getLoggerPtr());
 }
 
-// Reset (used in testing only).  This is a static method.
-
-void
-Logger::reset() {
-    LoggerImpl::reset();
-}
-
 } // namespace log
 } // namespace isc

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
@@ -152,12 +199,6 @@ public:
     /// \return true if the logger objects are instances of the same logger.
     bool operator==(Logger& other);
 
-protected:
-    /// \brief Clear logging hierachy
-    ///
-    /// This is for test use only, hence is protected.
-    static void reset();
-
 private:
     friend class isc::log::Formatter<Logger>;
 

src/lib/log/logger_impl.cc

@@ -12,6 +12,7 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE
 
+#include <iostream>
 #include <iomanip>
 #include <algorithm>
 
@@ -22,18 +23,17 @@
 
 #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>
 
-// Note: as log4cplus and th3e BIND 10 logger have many concepts in common, and
+// Note: as log4cplus and the BIND 10 logger have many concepts in common, and
 // thus many similar names, to disambiguate types we don't "use" the log4cplus
 // namespace: instead, all log4cplus types are explicitly qualified.
 
@@ -42,25 +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_))
 {
-    // Initialize log4cplus if not already done
-    initLog4cplus();
-
-    // 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.)
@@ -137,81 +126,5 @@ LoggerImpl::outputRaw(const Severity& severity, const string& message) {
     }
 }
 
-// Initialization.  This is one initialization for all loggers, so requires
-// a singleton to hold the initialization flag.  The flag is held within a
-// static method to ensure that it is created (and initialized) when needed.
-// This avoids a static initialization fiasco.
-
-bool&
-LoggerImpl::initialized() {
-    static bool initialized = false;
-    return (initialized);
-}
-
-void
-LoggerImpl::initLog4cplus() {
-
-    if (! initialized()) {
-
-        // Set up basic configurator.  This attaches a ConsoleAppender to the
-        // root logger with suitable output.  This is used until we we have
-        // actually read the logging configuration, in which case the output
-        // may well be changed.
-        log4cplus::BasicConfigurator config;
-        config.configure();
-        setRootAppenderLayout();
-
-        // Add additional debug levels
-        LoggerLevelImpl::init();
-
-        // All done.
-        initialized() = true;
-    }
-}
-
-void LoggerImpl::setRootAppenderLayout() {
-
-    // 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");
-
-    // Retrieve the appenders on the root instance and set the layout to
-    // use that pattern.
-    log4cplus::SharedAppenderPtrList list =
-        log4cplus::Logger::getRoot().getAllAppenders();
-
-    for (log4cplus::SharedAppenderPtrList::iterator i = list.begin();
-         i != list.end(); ++i) {
-        auto_ptr<log4cplus::Layout> layout(
-            new log4cplus::PatternLayout(pattern));
-        (*i)->setLayout(layout);
-    }
-}
-
-// Reset.  Just reset logger hierarchy to default settings (don't remove the
-// loggers - this appears awkward); this is effectively the same as removing
-// them.
-void
-LoggerImpl::reset() {
-    log4cplus::Logger::getDefaultHierarchy().resetConfiguration();
-    initialized() = false;
-
-    // N.B.  The documentation is not clear, but it does not appear that the
-    // methods used to format the new logging levels are removed from the
-    // log4cxx LogLevelManager class - indeed, there appears to be no way
-    // to do this.  This would seem to suggest that a re-initialization may
-    // well add another instance of the toString/fromString to the manager's
-    // list of methods.
-    //
-    // We could get round this by making setting the LogManager a truly
-    // one-shot process.  However, apart from taking up memory there is little
-    // overhead if multiple instances are added.  The manager walks the list and
-    // uses the first method that processes the argument, so multiple methods
-    // doing the same think will not affect functionality.  Besides, this
-    // reset() method is only used in testing and not in the distributed
-    // software.
-}
-
-
 } // namespace log
 } // namespace isc

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 {
@@ -184,31 +177,7 @@ public:
         return (name_ == other.name_);
     }
 
-    /// \brief Reset logging
-    ///
-    /// Resets (clears) the log4cplus logging, requiring that an initialization
-    /// call be performed again.
-    static void reset();
-
-
 private:
-
-    /// \brief Initialize log4cplus
-    ///
-    /// Static method to perform initialization of the log4cplus system.
-    static void initLog4cplus();
-
-    /// \brief Initialization Flag
-    ///
-    /// Static method to access an initialization flag.  Doing it this
-    /// way means that there is no static initialization fiasco.
-    static bool& initialized();
-
-    /// \brief Set layout pattern
-    ///
-    /// Sets the layout for root logger appender(s)
-    static void setRootAppenderLayout();
-
     std::string         name_;              ///< Full name of this logger
     log4cplus::Logger   logger_;            ///< Underlying log4cplus logger
 };

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

@@ -0,0 +1,48 @@
+// 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 <log/logger_level.h>
+#include <log/macros.h>
+#include <log/messagedef.h>
+
+#include <boost/algorithm/string.hpp>
+
+
+namespace isc {
+namespace log {
+
+isc::log::Severity
+getSeverity(const std::string& sev_str) {
+    if (boost::iequals(sev_str, "DEBUG")) {
+        return isc::log::DEBUG;
+    } else if (boost::iequals(sev_str, "INFO")) {
+        return isc::log::INFO;
+    } else if (boost::iequals(sev_str, "WARN")) {
+        return isc::log::WARN;
+    } else if (boost::iequals(sev_str, "ERROR")) {
+        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);
+        return isc::log::INFO;
+    }
+}
+
+
+} // namespace log
+} // namespace isc

src/lib/log/logger_level.h

@@ -15,6 +15,8 @@
 #ifndef __LOGGER_LEVEL_H
 #define __LOGGER_LEVEL_H
 
+#include <string>
+
 namespace isc {
 namespace log {
 
@@ -56,6 +58,18 @@ struct Level {
     // Default assignment and copy constructor is appropriate
 };
 
+/// \brief Returns the isc::log::Severity value represented by the given string
+///
+/// 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. 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
 }   // namespace isc
 

src/lib/log/logger_level_impl.cc

@@ -18,12 +18,19 @@
 #include <boost/lexical_cast.hpp>
 
 #include <log4cplus/logger.h>
+
+#include <log/impldef.h>
 #include <log/logger_level.h>
 #include <log/logger_level_impl.h>
+#include <log/macros.h>
 
 using namespace log4cplus;
 using namespace std;
 
+namespace {
+isc::log::Logger logger("log");
+}
+
 namespace isc {
 namespace log {
 
@@ -87,7 +94,8 @@ LoggerLevelImpl::convertToBindLevel(const log4cplus::LogLevel loglevel) {
     } else if (loglevel <= log4cplus::DEBUG_LOG_LEVEL) {
 
         // Debug severity, so extract the debug level from the numeric value.
-        // If outside the limits, change the severity to the level above or below.
+        // If outside the limits, change the severity to the level above or
+        // below.
         int dbglevel = MIN_DEBUG_LEVEL +
                        static_cast<int>(log4cplus::DEBUG_LOG_LEVEL) -
                        static_cast<int>(loglevel);
@@ -148,16 +156,28 @@ LoggerLevelImpl::logLevelFromString(const log4cplus::tstring& level) {
                 // if DEBUG99 has been specified.
                 try {
                     int dbglevel = boost::lexical_cast<int>(name.substr(5));
+                    if (dbglevel < MIN_DEBUG_LEVEL) {
+                        LOG_WARN(logger, LOGIMPL_BELOWDBGMIN).arg(dbglevel)
+                            .arg(MIN_DEBUG_LEVEL);
+                        dbglevel = MIN_DEBUG_LEVEL;
+
+                    } else if (dbglevel > MAX_DEBUG_LEVEL) {
+                        LOG_WARN(logger, LOGIMPL_ABOVEDBGMAX).arg(dbglevel)
+                            .arg(MAX_DEBUG_LEVEL);
+                        dbglevel = MAX_DEBUG_LEVEL;
+
+                    }
                     return convertFromBindLevel(Level(DEBUG, dbglevel));
                 }
                 catch (boost::bad_lexical_cast&) {
+                    LOG_ERROR(logger, LOGIMPL_BADDEBUG).arg(name);
                     return (NOT_SET_LOG_LEVEL);
                 }
             }
-        }
-        else {
+        } else {
 
-            // Unknown string - return default. 
+            // Unknown string - return default.  Log4cplus will call any other
+            // registered conversion functions to interpret it.
             return (NOT_SET_LOG_LEVEL);
         }
     }
@@ -175,6 +195,9 @@ LoggerLevelImpl::logLevelToString(log4cplus::LogLevel level) {
         ((dbglevel >= MIN_DEBUG_LEVEL) && (dbglevel <= MAX_DEBUG_LEVEL))) {
         return (tstring("DEBUG"));
     }
+
+    // Unknown, so return empty string for log4cplus to try other conversion
+    // functions.
     return (tstring());
 }
 

src/lib/log/logger_manager.cc

@@ -0,0 +1,183 @@
+// 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 <algorithm>
+#include <vector>
+
+#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/macros.h>
+#include <log/messagedef.h>
+#include <log/message_initializer.h>
+
+using namespace std;
+
+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 {
+
+// Constructor - create the implementation  class.
+LoggerManager::LoggerManager() {
+    impl_ = new LoggerManagerImpl();
+}
+
+// Destructor - get rid of the implementation class
+LoggerManager::~LoggerManager() {
+    delete impl_;
+}
+
+// Initialize processing
+void
+LoggerManager::processInit() {
+    impl_->processInit();
+}
+
+// Process logging specification
+void
+LoggerManager::processSpecification(const LoggerSpecification& spec) {
+    impl_->processSpecification(spec);
+}
+
+// End Processing
+void
+LoggerManager::processEnd() {
+    impl_->processEnd();
+}
+
+
+/// Logging system initialization
+
+void
+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.
+    setRootLoggerName(root);
+
+    // Initialize the implementation logging.  After this point, some basic
+    // logging has been set up and messages can be logged.
+    LoggerManagerImpl::init(severity, dbglevel);
+
+    // Check if there were any duplicate message IDs in the default dictionary
+    // and if so, log them.  Log using the logging facility logger.
+    vector<string>& duplicates = MessageInitializer::getDuplicates();
+    if (!duplicates.empty()) {
+
+        // There are duplicates present.  This will be listed in alphabetic
+        // order of message ID, so they need to be sorted.  This list itself may
+        // contain duplicates; if so, the message ID is listed as many times as
+        // there are duplicates.
+        sort(duplicates.begin(), duplicates.end());
+        for (vector<string>::iterator i = duplicates.begin();
+             i != duplicates.end(); ++i) {
+            LOG_WARN(logger, MSG_DUPMSGID).arg(*i);
+        }
+
+    }
+
+    // Replace any messages with local ones (if given)
+    if (file) {
+        readLocalMessageFile(file);
+    }
+}
+
+
+// Read local message file
+// TODO This should be done after the configuration has been read so that
+// the file can be placed in the local configuration
+void
+LoggerManager::readLocalMessageFile(const char* file) {
+
+    MessageDictionary& dictionary = MessageDictionary::globalDictionary();
+    MessageReader reader(&dictionary);
+    try {
+
+        logger.info(MSG_RDLOCMES).arg(file);
+        reader.readFile(file, MessageReader::REPLACE);
+
+        // File successfully read.  As each message in the file is supposed to
+        // replace one in the dictionary, any ID read that can't be located in
+        // the dictionary will not be used.  To aid problem diagnosis, the
+        // unknown message IDs are listed.
+        MessageReader::MessageIDCollection unknown = reader.getNotAdded();
+        for (MessageReader::MessageIDCollection::const_iterator
+            i = unknown.begin(); i != unknown.end(); ++i) {
+            string message_id = boost::lexical_cast<string>(*i);
+                logger.warn(MSG_IDNOTFND).arg(message_id);
+        }
+    }
+    catch (MessageException& e) {
+        MessageID ident = e.id();
+        vector<string> args = e.arguments();
+
+        // Log the variable number of arguments.  The actual message will be
+        // logged when the error_message variable is destroyed.
+        Formatter<isc::log::Logger> error_message = logger.error(ident);
+        for (int i = 0; i < args.size(); ++i) {
+            error_message = error_message.arg(args[i]);
+        }
+    }
+}
+
+// Reset logging to settings passed to init()
+void
+LoggerManager::reset() {
+    setRootLoggerName(initRootName());
+    LoggerManagerImpl::reset(initSeverity(), initDebugLevel());
+}
+
+} // namespace log
+} // namespace isc

src/lib/log/logger_manager.h

@@ -0,0 +1,141 @@
+// 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_MANAGER_H
+#define __LOGGER_MANAGER_H
+
+#include "exceptions/exceptions.h"
+#include <log/logger_specification.h>
+
+// Generated if, when updating the logging specification, an unknown
+// destination is encountered.
+class UnknownLoggingDestination : public isc::Exception {
+public:
+    UnknownLoggingDestination(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what)
+    {}
+};
+
+namespace isc {
+namespace log {
+
+class LoggerManagerImpl;
+
+/// \brief Logger Manager
+///
+/// The logger manager class exists to process the set of logger specifications
+/// and use them to set up the logging in the program appropriately.
+///
+/// To isolate the underlying implementation from basic processing, the
+/// LoggerManager is implemented using the "pimpl" idiom.
+
+class LoggerManager {
+public:
+    /// \brief Constructor
+    LoggerManager();
+
+    /// \brief Destructor
+    ~LoggerManager();
+
+    /// \brief Process Specifications
+    ///
+    /// Replaces the current logging configuration by the one given.
+    ///
+    /// \param start Iterator pointing to the start of the collection of
+    ///        logging specifications.
+    /// \param finish Iterator pointing to the end of the collection of
+    ///        logging specification.
+    template <typename T>
+    void process(T start, T finish) {
+        processInit();
+        for (T i = start; i != finish; ++i) {
+            processSpecification(*i);
+        }
+        processEnd();
+    }
+
+    /// \brief Process a single specification
+    ///
+    /// A convenience function for a single specification.
+    ///
+    /// \param spec Specification to process
+    void process(const LoggerSpecification& spec) {
+        processInit();
+        processSpecification(spec);
+        processEnd();
+    }
+
+    /// \brief Run-Time Initialization
+    ///
+    /// Performs run-time initialization of the logger system, in particular
+    /// supplying the root logger name and name of a replacement message file.
+    ///
+    /// This must be the first logging function called in the program.  If
+    /// an attempt is made to log a message before this is function is called,
+    /// the results will be dependent on the underlying logging package.
+    ///
+    /// \param root Name of the root logger.  This should be set to the name of
+    ///        the program.
+    /// \param severity Severity at which to log
+    /// \param dbglevel Debug severity (ignored if "severity" is not "DEBUG")
+    /// \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, const char* file = NULL);
+
+    /// \brief Reset logging
+    ///
+    /// Resets logging to whatever was set in the call to init().
+    static void reset();
+
+    /// \brief Read local message file
+    ///
+    /// Reads the local message file into the global dictionary, overwriting
+    /// existing messages.  If the file contained any message IDs not in the
+    /// dictionary, they are listed in a warning message.
+    ///
+    /// \param file Name of the local message file
+    static void readLocalMessageFile(const char* file);
+
+private:
+    /// \brief Initialize Processing
+    ///
+    /// Initializes the processing of a list of specifications by resetting all
+    /// loggers to their defaults, which is to pass the message to their
+    /// parent logger.  (Except for the root logger, where the default action is
+    /// to output the message.)
+    void processInit();
+
+    /// \brief Process Logging Specification
+    ///
+    /// Processes the given specification.  It is assumed at this point that
+    /// either the logger does not exist or has been made inactive.
+    void processSpecification(const LoggerSpecification& spec);
+
+    /// \brief End Processing
+    ///
+    /// Place holder for finish processing.
+    /// TODO: Check that the root logger has something enabled
+    void processEnd();
+
+    // Members
+    LoggerManagerImpl*  impl_;      ///< Pointer to implementation
+};
+
+} // namespace log
+} // namespace isc
+
+
+#endif // __LOGGER_MANAGER_H

src/lib/log/logger_manager_impl.cc

@@ -0,0 +1,228 @@
+// 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 <algorithm>
+#include <iostream>
+
+#include <log4cplus/logger.h>
+#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/messagedef.h"
+
+using namespace std;
+
+namespace isc {
+namespace log {
+
+// Reset hierarchy of loggers back to default settings.  This removes all
+// appenders from loggers, sets their severity to NOT_SET (so that events are
+// passed back to the parent) and resets the root logger to logging
+// informational messages.  (This last is not a log4cplus default, so we have to
+// explicitly reset the logging severity.)
+
+void
+LoggerManagerImpl::processInit() {
+    log4cplus::Logger::getDefaultHierarchy().resetConfiguration();
+    initRootLogger();
+}
+
+// Process logging specification.  Set up the common states then dispatch to
+// add output specifications.
+
+void
+LoggerManagerImpl::processSpecification(const LoggerSpecification& spec) {
+
+    log4cplus::Logger logger = log4cplus::Logger::getInstance(
+                                   expandLoggerName(spec.getName()));
+
+    // Set severity level according to specification entry.
+    logger.setLogLevel(LoggerLevelImpl::convertFromBindLevel(
+                       Level(spec.getSeverity(), spec.getDbglevel())));
+
+    // Set the additive flag.
+    logger.setAdditivity(spec.getAdditive());
+
+    // Output options given?
+    if (spec.optionCount() > 0) {
+
+        // Yes, so replace all appenders for this logger.
+        logger.removeAllAppenders();
+
+        // Now process output specifications.
+        for (LoggerSpecification::const_iterator i = spec.begin();
+             i != spec.end(); ++i) {
+            switch (i->destination) {
+            case OutputOption::DEST_CONSOLE:
+                createConsoleAppender(logger, *i);
+                break;
+
+            case OutputOption::DEST_FILE:
+                createFileAppender(logger, *i);
+                break;
+
+            case OutputOption::DEST_SYSLOG:
+                createSyslogAppender(logger, *i);
+                break;
+
+            default:
+                // Not a valid destination.  As we are in the middle of updating
+                // logging destinations, we could be in the situation where
+                // there are no valid appenders.  For this reason, throw an
+                // exception.
+                isc_throw(UnknownLoggingDestination,
+                          "Unknown logging destination, code = " <<
+                          i->destination);
+            }
+        }
+    }
+}
+
+// Console appender - log to either stdout or stderr.
+void
+LoggerManagerImpl::createConsoleAppender(log4cplus::Logger& logger,
+                                         const OutputOption& opt)
+{
+    log4cplus::SharedAppenderPtr console(
+        new log4cplus::ConsoleAppender(
+            (opt.stream == OutputOption::STR_STDERR), opt.flush));
+    setConsoleAppenderLayout(console);
+    logger.addAppender(console);
+}
+
+// File appender.  Depending on whether a maximum size is given, either
+// a standard file appender or a rolling file appender will be created.
+void
+LoggerManagerImpl::createFileAppender(log4cplus::Logger& logger,
+                                         const OutputOption& opt)
+{
+    LOG4CPLUS_OPEN_MODE_TYPE mode = 
+        LOG4CPLUS_FSTREAM_NAMESPACE::ios::app;  // Append to existing file
+
+    log4cplus::SharedAppenderPtr fileapp;
+    if (opt.maxsize == 0) {
+        fileapp = log4cplus::SharedAppenderPtr(new log4cplus::FileAppender(
+            opt.filename, mode, opt.flush));
+    } else {
+        fileapp = log4cplus::SharedAppenderPtr(
+            new log4cplus::RollingFileAppender(opt.filename, opt.maxsize,
+                                               opt.maxver, opt.flush));
+    }
+
+    // use the same console layout for the files.
+    setConsoleAppenderLayout(fileapp);
+    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
+
+void
+LoggerManagerImpl::init(isc::log::Severity severity, int dbglevel) {
+
+    // Set up basic configurator.  This attaches a ConsoleAppender to the
+    // root logger with suitable output.  This is used until we we have
+    // actually read the logging configuration, in which case the output
+    // may well be changed.
+    log4cplus::BasicConfigurator config;
+    config.configure();
+
+    // Add the additional debug levels
+    LoggerLevelImpl::init();
+
+    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(isc::log::Severity severity, int dbglevel) {
+
+    // Initialize the root logger
+    initRootLogger(severity, dbglevel);
+}
+
+// Initialize the root logger
+void LoggerManagerImpl::initRootLogger(isc::log::Severity severity,
+                                       int dbglevel)
+{
+    log4cplus::Logger::getDefaultHierarchy().resetConfiguration();
+
+    // 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 BIND 10 root to use a console logger.
+    OutputOption opt;
+    createConsoleAppender(b10root, opt);
+}
+
+// Set the the "console" layout for the given appenders.  This layout includes
+// a date/time and the name of the logger.
+
+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 [%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));
+    appender->setLayout(layout);
+}
+
+} // namespace log
+} // namespace isc

src/lib/log/logger_manager_impl.h

@@ -0,0 +1,171 @@
+// 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_MANAGER_IMPL_H
+#define __LOGGER_MANAGER_IMPL_H
+
+#include <string>
+
+#include <log4cplus/appender.h>
+#include <log/logger_level.h>
+
+// Forward declaration to avoid need to include log4cplus header file here.
+namespace log4cplus {
+class Logger;
+class Appender;
+}
+
+namespace isc {
+namespace log {
+
+// Forward declarations
+class LoggerSpecification;
+class OutputOption;
+
+/// \brief Logger Manager Implementation
+///
+/// This is the implementation of the logger manager for the log4cplus
+/// underlying logger.
+///
+/// As noted in logger_manager.h, the logger manager class exists to set up the
+/// logging given a set of specifications.  This class handles the processing
+/// of those specifications.
+///
+/// Note: the logging has been implemented using a "pimpl" idiom to conceal
+/// the underlying implementation (log4cplus) from the BIND 10 interface.
+/// This requires that there be an implementation class, even though in this
+/// case, all the implementation class methods can be declared static.
+
+class LoggerManagerImpl {
+public:
+
+    /// \brief Constructor
+    LoggerManagerImpl()
+    {}
+
+    /// \brief Initialize Processing
+    ///
+    /// This resets the hierachy of loggers back to their defaults.  This means
+    /// that all non-root loggers (if they exist) are set to NOT_SET, and the
+    /// root logger reset to logging informational messages.
+    ///
+    /// \param root_name BIND 10 name of the root logger
+    static void processInit();
+
+    /// \brief Process Specification
+    ///
+    /// Processes the specification for a single logger.
+    ///
+    /// \param spec Logging specification for this logger
+    static void processSpecification(const LoggerSpecification& spec);
+
+    /// \brief End Processing
+    ///
+    /// Terminates the processing of the logging specifications.
+    static void processEnd()
+    {}
+
+    /// \brief Implementation-specific initialization
+    ///
+    /// Sets the basic configuration for logging (the root logger has INFO and
+    /// more severe messages routed to stdout).  Unless this function (or
+    /// process() with a valid specification for all loggers that will log
+    /// messages) is called before a message is logged, log4cplus will output
+    /// a message to stderr noting that logging has not been initialized.
+    ///
+    /// It is assumed here that the name of the BIND 10 root logger can be
+    /// obtained from the global function getRootLoggerName().
+    ///
+    /// \param severity Severity to be associated with this logger
+    /// \param dbglevel Debug level associated with the root logger
+    static void init(isc::log::Severity severity = isc::log::INFO,
+                     int dbglevel = 0);
+
+    /// \brief Reset logging
+    ///
+    /// Resets to default configuration (root logger logging to the console
+    /// with INFO severity).
+    ///
+    /// \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
+    ///
+    /// Creates an object that, when attached to a logger, will log to one
+    /// of the output streams (stdout or stderr).
+    ///
+    /// \param logger Log4cplus logger to which the appender must be attached.
+    /// \param opt Output options for this appender.
+    static void createConsoleAppender(log4cplus::Logger& logger,
+                                      const OutputOption& opt);
+
+    /// \brief Create file appender
+    ///
+    /// Creates an object that, when attached to a logger, will log to a
+    /// specified file.  This also includes the ability to "roll" files when
+    /// they reach a specified size.
+    ///
+    /// \param logger Log4cplus logger to which the appender must be attached.
+    /// \param opt Output options for this appender.
+    static void createFileAppender(log4cplus::Logger& logger,
+                                   const OutputOption& opt);
+
+    /// \brief Create syslog appender
+    ///
+    /// Creates an object that, when attached to a logger, will log to the
+    /// syslog file.
+    ///
+    /// \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);
+
+    /// \brief Set default layout and severity for root logger
+    ///
+    /// Initializes the root logger to BIND 10 defaults - console output and
+    /// the passed severity/debug level.
+    ///
+    /// \param severity Severity of messages that the logger should output.
+    /// \param dbglevel Debug level if severity = DEBUG
+    static void initRootLogger(isc::log::Severity severity = isc::log::INFO,
+                               int dbglevel = 0);
+
+    /// \brief Set layout for console appender
+    ///
+    /// 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
+    ///
+    /// \param appender Appender for which this pattern is to be set.
+    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
+} // namespace isc
+
+#endif // __LOGGER_MANAGER_IMPL_H

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_specification.h

@@ -0,0 +1,156 @@
+// 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_SPECIFICATION_H
+#define __LOGGER_SPECIFICATION_H
+
+#include <stdint.h>
+#include <stdlib.h>
+
+#include <log/logger_level.h>
+#include <log/output_option.h>
+
+/// \brief Logger Specification
+///
+/// The logging configuration options are a list of logger specifications, each
+/// of which represents a logger and the options for its appenders.
+///
+/// Unlike OutputOption (which is a struct), this contains a bit more
+/// structure and is concealed in a class.
+
+#include <vector>
+
+namespace isc {
+namespace log {
+
+class LoggerSpecification {
+public:
+    typedef std::vector<OutputOption>::iterator         iterator;
+    typedef std::vector<OutputOption>::const_iterator   const_iterator;
+
+    /// \brief Constructor
+    ///
+    /// \param name Name of the logger.
+    /// \param severity Severity at which this logger logs
+    /// \param dbglevel Debug level
+    /// \param additive true to cause message logged with this logger to be
+    ///        passed to the parent for logging.
+    LoggerSpecification(const std::string& name = "",
+                        isc::log::Severity severity = isc::log::INFO,
+                        int dbglevel = 0, bool additive = false) :
+        name_(name), severity_(severity), dbglevel_(dbglevel),
+        additive_(additive)
+    {}
+
+    /// \brief Set the name of the logger.
+    ///
+    /// \param name Name of the logger.
+    void setName(const std::string& name) {
+        name_ = name;
+    }
+
+    /// \return Return logger name.
+    std::string getName() const {
+        return name_;
+    }
+
+    /// \brief Set the severity.
+    ///
+    /// \param severity New severity of the logger.
+    void setSeverity(isc::log::Severity severity) {
+        severity_ = severity;
+    }
+
+    /// \return Return logger severity.
+    isc::log::Severity getSeverity() const {
+        return severity_;
+    }
+
+    /// \brief Set the debug level.
+    ///
+    /// \param dbglevel New debug level of the logger.
+    void setDbglevel(int dbglevel) {
+        dbglevel_ = dbglevel;
+    }
+
+    /// \return Return logger debug level
+    int getDbglevel() const {
+        return dbglevel_;
+    }
+
+    /// \brief Set the additive flag.
+    ///
+    /// \param additive New value of the additive flag.
+    void setAdditive(bool additive) {
+        additive_ = additive;
+    }
+
+    /// \return Return additive flag.
+    int getAdditive() const {
+        return additive_;
+    }
+
+    /// \brief Add output option.
+    ///
+    /// \param Option to add to the list.
+    void addOutputOption(const OutputOption& option) {
+        options_.push_back(option);
+    }
+
+    /// \return Iterator to start of output options.
+    iterator begin() {
+        return options_.begin();
+    }
+
+    /// \return Iterator to start of output options.
+    const_iterator begin() const {
+        return options_.begin();
+    }
+
+    /// \return Iterator to end of output options.
+    iterator end() {
+        return options_.end();
+    }
+
+    /// \return Iterator to end of output options.
+    const_iterator end() const {
+        return options_.end();
+    }
+
+    /// \return Number of output specification options.
+    size_t optionCount() const {
+        return options_.size();
+    }
+
+    /// \brief Reset back to defaults.
+    void reset() {
+        name_ = "";
+        severity_ = isc::log::INFO;
+        dbglevel_ = 0;
+        additive_ = false;
+        options_.clear();
+    }
+
+private:
+    std::string                 name_;          ///< Logger name
+    isc::log::Severity          severity_;      ///< Severity for this logger
+    int                         dbglevel_;      ///< Debug level
+    bool                        additive_;      ///< Chaining output 
+    std::vector<OutputOption>   options_;       ///< Logger options
+};
+
+} // namespace log
+} // namespace isc
+
+#endif // __LOGGER_SPEC_IFICATIONH

src/lib/log/logger_support.cc

@@ -28,18 +28,10 @@
 #include <algorithm>
 #include <iostream>
 #include <string>
-#include <vector>
-#include <boost/lexical_cast.hpp>
 
 #include <log/logger.h>
+#include <log/logger_manager.h>
 #include <log/logger_support.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>
 
 namespace isc {
 namespace log {
@@ -50,96 +42,20 @@ using namespace std;
 // root logger and is used in all functions in this file.
 Logger logger("log");
 
-
-/// \brief Reads Local Message File
-///
-/// Reads the local message file into the global dictionary, overwriting
-/// existing messages.  If the file contained any message IDs not in the
-/// dictionary, they are listed in a warning message.
-///
-/// \param file Name of the local message file
-static void
-readLocalMessageFile(const char* file) {
-
-    MessageDictionary& dictionary = MessageDictionary::globalDictionary();
-    MessageReader reader(&dictionary);
-    try {
-        logger.info(MSG_RDLOCMES).arg(file);
-        reader.readFile(file, MessageReader::REPLACE);
-
-        // File successfully read, list the duplicates
-        MessageReader::MessageIDCollection unknown = reader.getNotAdded();
-        for (MessageReader::MessageIDCollection::const_iterator
-            i = unknown.begin(); i != unknown.end(); ++i) {
-            string message_id = boost::lexical_cast<string>(*i);
-                logger.warn(MSG_IDNOTFND).arg(message_id);
-        }
-    }
-    catch (MessageException& e) {
-        MessageID ident = e.id();
-        vector<string> args = e.arguments();
-        switch (args.size()) {
-        case 0:
-            logger.error(ident);
-            break;
-
-        case 1:
-            logger.error(ident).arg(args[0]);
-            break;
-
-        case 2:
-            logger.error(ident).arg(args[0]).arg(args[1]);
-            break;
-
-        default:    // 3 or more (3 should be the maximum)
-            logger.error(ident).arg(args[0]).arg(args[1]).arg(args[2]);
-        }
-    }
-}
-
 /// Logger Run-Time Initialization
 
 void
 initLogger(const string& root, isc::log::Severity severity, int dbglevel,
     const char* file) {
-
-    // Create the application 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.
-    setRootLoggerName(root);
-    Logger root_logger(isc::log::getRootLoggerName());
-
-    // Set the severity associated with it.  If no other logger has a severity,
-    // this will be the default.
-    root_logger.setSeverity(severity, dbglevel);
-
-    // Check if there were any duplicate message IDs in the default dictionary
-    // and if so, log them.  Log using the logging facility root logger.
-    vector<string>& duplicates = MessageInitializer::getDuplicates();
-    if (!duplicates.empty()) {
-
-        // There are - sort and remove any duplicates.
-        sort(duplicates.begin(), duplicates.end());
-        vector<string>::iterator new_end =
-            unique(duplicates.begin(), duplicates.end());
-        for (vector<string>::iterator i = duplicates.begin(); i != new_end; ++i) {
-            logger.warn(MSG_DUPMSGID).arg(*i);
-        }
-
-    }
-
-    // Replace any messages with local ones (if given)
-    if (file) {
-        readLocalMessageFile(file);
-    }
+    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;
@@ -149,29 +65,13 @@ 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::DEBUG;
     const char* sev_char = getenv("B10_LOGGER_SEVERITY");
     if (sev_char) {
-        string sev_string(sev_char);
-        if (sev_string == "DEBUG") {
-            severity = isc::log::DEBUG;
-        } else if (sev_string == "INFO") {
-            severity = isc::log::INFO;
-        } else if (sev_string == "WARN") {
-            severity = isc::log::WARN;
-        } else if (sev_string == "ERROR") {
-            severity = isc::log::ERROR;
-        } else if (sev_string == "FATAL") {
-            severity = isc::log::FATAL;
-        } else {
-            std::cerr << "**ERROR** unrecognised logger severity of '"
-                      << sev_string << "' - default severity will be used\n";
-        }
+        severity = isc::log::getSeverity(sev_char);
     }
 
     // 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

@@ -15,6 +15,8 @@
 #ifndef __LOGGER_SUPPORT_H
 #define __LOGGER_SUPPORT_H
 
+#include <unistd.h>
+
 #include <string>
 #include <log/logger.h>
 
@@ -36,8 +38,9 @@ namespace log {
 /// \param severity Severity at which to log
 /// \param dbglevel Debug severity (ignored if "severity" is not "DEBUG")
 /// \param file Name of the local message file.
-void initLogger(const std::string& root, isc::log::Severity severity,
-    int dbglevel, const char* file);
+void initLogger(const std::string& root,
+                isc::log::Severity severity = isc::log::INFO,
+                int dbglevel = 0, const char* file = NULL);
 
 
 /// \brief Run-Time Initialization from Environment
@@ -46,19 +49,20 @@ void initLogger(const std::string& root, isc::log::Severity severity,
 /// 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
@@ -68,7 +72,8 @@ void initLogger(const std::string& root, isc::log::Severity severity,
 ///
 /// 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/messagedef.cc

@@ -1,4 +1,4 @@
-// File created from messagedef.mes on Mon May  9 13:52:54 2011
+// File created from messagedef.mes on Fri May 27 14:49:45 2011
 
 #include <cstddef>
 #include <log/message_types.h>
@@ -7,6 +7,9 @@
 namespace isc {
 namespace log {
 
+extern const isc::log::MessageID MSG_BADDESTINATION = "MSG_BADDESTINATION";
+extern const isc::log::MessageID MSG_BADSEVERITY = "MSG_BADSEVERITY";
+extern const isc::log::MessageID MSG_BADSTREAM = "MSG_BADSTREAM";
 extern const isc::log::MessageID MSG_DUPLNS = "MSG_DUPLNS";
 extern const isc::log::MessageID MSG_DUPMSGID = "MSG_DUPMSGID";
 extern const isc::log::MessageID MSG_IDNOTFND = "MSG_IDNOTFND";
@@ -31,6 +34,9 @@ extern const isc::log::MessageID MSG_WRITERR = "MSG_WRITERR";
 namespace {
 
 const char* values[] = {
+    "MSG_BADDESTINATION", "unrecognized log destination: %1",
+    "MSG_BADSEVERITY", "unrecognized log severity: %1",
+    "MSG_BADSTREAM", "bad log console output stream: %1",
     "MSG_DUPLNS", "line %1: duplicate $NAMESPACE directive found",
     "MSG_DUPMSGID", "duplicate message ID (%1) in compiled code",
     "MSG_IDNOTFND", "could not replace message text for '%1': no such message",

src/lib/log/messagedef.h

@@ -1,4 +1,4 @@
-// File created from messagedef.mes on Mon May  9 13:52:54 2011
+// File created from messagedef.mes on Fri May 27 14:49:45 2011
 
 #ifndef __MESSAGEDEF_H
 #define __MESSAGEDEF_H
@@ -8,6 +8,9 @@
 namespace isc {
 namespace log {
 
+extern const isc::log::MessageID MSG_BADDESTINATION;
+extern const isc::log::MessageID MSG_BADSEVERITY;
+extern const isc::log::MessageID MSG_BADSTREAM;
 extern const isc::log::MessageID MSG_DUPLNS;
 extern const isc::log::MessageID MSG_DUPMSGID;
 extern const isc::log::MessageID MSG_IDNOTFND;

src/lib/log/messagedef.mes

@@ -117,3 +117,15 @@ the named output file.
 % UNRECDIR      line %1: unrecognised directive '%2'
 A line starting with a dollar symbol was found, but the first word on the line
 (shown in the message) was not a recognised message compiler directive.
+
+% BADSEVERITY   unrecognized log severity: %1
+A logger severity value was given that was not recognized. The severity
+should be one of "DEBUG", "INFO", "WARN", "ERROR", or "FATAL".
+
+% BADDESTINATION unrecognized log destination: %1
+A logger destination value was given that was not recognized. The
+destination should be one of "console", "file", or "syslog".
+
+% BADSTREAM     bad log console output stream: %1
+A log console output stream was given that was not recognized. The
+output stream should be one of "stdout", or "stderr"

src/lib/log/output_option.cc

@@ -0,0 +1,54 @@
+// 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 <string>
+#include <log/output_option.h>
+#include <log/macros.h>
+#include <log/messagedef.h>
+
+#include <boost/algorithm/string.hpp>
+
+namespace isc {
+namespace log {
+
+OutputOption::Destination
+getDestination(const std::string& dest_str) {
+    if (boost::iequals(dest_str, "console")) {
+        return OutputOption::DEST_CONSOLE;
+    } else if (boost::iequals(dest_str, "file")) {
+        return OutputOption::DEST_FILE;
+    } else if (boost::iequals(dest_str, "syslog")) {
+        return OutputOption::DEST_SYSLOG;
+    } else {
+        Logger logger("log");
+        LOG_ERROR(logger, MSG_BADDESTINATION).arg(dest_str);
+        return OutputOption::DEST_CONSOLE;
+    }
+}
+
+OutputOption::Stream
+getStream(const std::string& stream_str) {
+    if (boost::iequals(stream_str, "stderr")) {
+        return OutputOption::STR_STDERR;
+    } else if (boost::iequals(stream_str, "stdout")) {
+        return OutputOption::STR_STDOUT;
+    } else {
+        Logger logger("log");
+        LOG_ERROR(logger, MSG_BADSTREAM).arg(stream_str);
+        return OutputOption::STR_STDOUT;
+    }
+}
+
+} // namespace log
+} // namespace isc

src/lib/log/output_option.h

@@ -0,0 +1,85 @@
+// 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 __OUTPUT_OPTION_H
+#define __OUTPUT_OPTION_H
+
+#include <stdint.h>
+#include <stdlib.h>
+#include <string>
+
+/// \brief Logger Output Option
+///
+/// The logging configuration options are a list of logger specifications, each
+/// with one or more output options.  This class represents an output option;
+/// one or more of these are attached to a LoggerSpecification object which is
+/// then passed to the LoggerManager to configure the logger.
+///
+/// Although there are three distinct output types (console, file, syslog) and
+/// the options for each do not really overlap.  Although it is tempting to
+/// define a base OutputOption class and derive a class for each type
+/// (ConsoleOutputOptions etc.), it would be messy to use in practice.  At
+/// some point the exact class would have to be known to get the class-specific
+/// options and the (pointer to) the base class cast to the appropriate type.
+/// Instead, this "struct" contains the union of all output options; it is up
+/// to the caller to cherry-pick the members it needs.
+///
+/// One final note: this object holds data and does no computation.  For this
+/// reason, it is a "struct" and members are accessed directly instead of
+/// through methods.
+
+namespace isc {
+namespace log {
+
+struct OutputOption {
+
+    /// Destinations.  Prefixed "DEST_" to avoid problems with the C stdio.h
+    /// FILE type.
+    typedef enum {
+        DEST_CONSOLE = 0,
+        DEST_FILE = 1,
+        DEST_SYSLOG = 2
+    } Destination;
+
+    /// If console, stream on which messages are output
+    typedef enum {
+        STR_STDOUT = 1,
+        STR_STDERR = 2
+    } Stream;
+
+    /// \brief Constructor
+    OutputOption() : destination(DEST_CONSOLE), stream(STR_STDERR),
+                     flush(false), facility("LOCAL0"), filename(""),
+                     maxsize(0), maxver(0)
+    {}
+
+    /// Members. 
+
+    Destination     destination;        ///< Where the output should go
+    Stream          stream;             ///< stdout/stderr if console output
+    bool            flush;              ///< true to flush after each message
+    std::string     facility;           ///< syslog facility
+    std::string     filename;           ///< Filename if file output
+    size_t          maxsize;            ///< 0 if no maximum size
+    unsigned int    maxver;             ///< Maximum versions (none if <= 0)
+};
+
+OutputOption::Destination getDestination(const std::string& dest_str);
+OutputOption::Stream getStream(const std::string& stream_str);
+
+
+} // namespace log
+} // namespace isc
+
+#endif // __OUTPUT_OPTION_H

src/lib/log/tests/Makefile.am

@@ -13,16 +13,19 @@ CLEANFILES = *.gcno *.gcda
 TESTS =
 if HAVE_GTEST
 TESTS += run_unittests
-run_unittests_SOURCES  = root_logger_name_unittest.cc
-run_unittests_SOURCES += logger_unittest.cc
-run_unittests_SOURCES += logger_level_unittest.cc
+run_unittests_SOURCES  = run_unittests.cc
+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_reader_unittest.cc
-run_unittests_SOURCES += message_initializer_unittest.cc
 run_unittests_SOURCES += message_initializer_unittest_2.cc
-run_unittests_SOURCES += run_unittests.cc
-run_unittests_SOURCES += log_formatter_unittest.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)
@@ -36,17 +39,24 @@ 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_support_test
-logger_support_test_SOURCES  = logger_support_test.cc
-logger_support_test_CPPFLAGS = $(AM_CPPFLAGS)
-logger_support_test_LDFLAGS  = $(AM_LDFLAGS)
-logger_support_test_LDADD    = $(top_builddir)/src/lib/log/liblog.la
+check_PROGRAMS = logger_example
+logger_example_SOURCES = logger_example.cc
+logger_example_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
+logger_example_LDFLAGS = $(AM_LDFLAGS) $(LOG4CPLUS_LDFLAGS)
+logger_example_LDADD  = $(top_builddir)/src/lib/log/liblog.la
+logger_example_LDADD += $(top_builddir)/src/lib/util/libutil.la
 
 noinst_PROGRAMS = $(TESTS)
 
 # Additional test using the shell
-PYTESTS = run_time_init_test.sh
+PYTESTS = console_test.sh local_file_test.sh severity_test.sh
 check-local:
-	$(SHELL) $(abs_builddir)/run_time_init_test.sh
+	$(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

@@ -0,0 +1,69 @@
+#!/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
+#
+# The logger supports the idea of a "console" logger than logs to either stdout
+# or stderr.  This test checks that both these options work.
+
+testname="Console output test"
+echo $testname
+
+failcount=0
+tempfile=@abs_builddir@/console_test_tempfile_$$
+
+# Look at tempfile and check that the count equals the expected count
+passfail() {
+    count=`wc -l $tempfile | awk '{print $1}'`
+    if [ $count -eq $1 ]; then
+        echo " pass"
+    else
+        echo " FAIL"
+        failcount=`expr $failcount + $1`
+    fi
+}
+
+echo -n "1. Checking that console output to stdout goes to stdout:"
+rm -f $tempfile
+./logger_example -c stdout -s error 1> $tempfile 2> /dev/null
+passfail 4
+
+echo -n "2. Checking that console output to stdout does not go to stderr:"
+rm -f $tempfile
+./logger_example -c stdout -s error 1> /dev/null 2> $tempfile
+passfail 0
+
+echo -n "3. Checking that console output to stderr goes to stderr:"
+rm -f $tempfile
+./logger_example -c stderr -s error 1> /dev/null 2> $tempfile
+passfail 4
+
+echo -n "4. Checking that console output to stderr does not go to stdout:"
+rm -f $tempfile
+./logger_example -c stderr -s error 1> $tempfile 2> /dev/null
+passfail 0
+
+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
+
+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

@@ -0,0 +1,86 @@
+#!/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 Local message file test
+#
+# Checks that a local message file can override the definitions in the message
+# dictionary.
+
+testname="Local message file test"
+echo $testname
+
+failcount=0
+localmes=@abs_builddir@/localdef_mes_$$
+tempfile=@abs_builddir@/run_time_init_test_tempfile_$$
+
+passfail() {
+    if [ $1 -eq 0 ]; then
+        echo " pass"
+    else
+        echo " FAIL"
+        failcount=`expr $failcount + $1`
+    fi
+}
+
+# Create the local message file for testing
+
+cat > $localmes << .
+\$PREFIX MSG_
+% NOTHERE     this message is not in the global dictionary
+% READERR     replacement read error, parameters: '%1' and '%2'
+% RDLOCMES    replacement read local message file, parameter is '%1'
+.
+
+echo -n "1. Local message replacement:"
+cat > $tempfile << .
+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 -n "2. Report error if unable to read local message file:"
+cat > $tempfile << .
+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 $?
+
+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
+
+exit $failcount

src/lib/log/tests/logger_example.cc

@@ -0,0 +1,305 @@
+// 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 Example Program
+///
+/// Simple example program showing how to use the logger.  The various
+/// command-line options let most aspects of the logger be exercised, so
+/// making this a useful tool for testing.
+///
+/// See the usage() method for details of use.
+
+#include <stdlib.h>
+#include <unistd.h>
+#include <string.h>
+
+#include <boost/lexical_cast.hpp>
+
+#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 a set of message definitions.
+#include <log/messagedef.h>
+
+using namespace isc::log;
+using namespace std;
+
+
+// Print usage information
+
+void usage() {
+    cout <<
+"logger_support_test [-h | [logger_spec] [[logger_spec]...]]\n"
+"\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"
+"   -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"
+"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";
+
+}
+
+
+// The program sets the attributes on the root logger and logs a set of
+// messages.  Looking at the output determines whether the program worked.
+
+int main(int argc, char** argv) {
+    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                y_found = false;    // Set true if "-y" found
+    int                 option;             // For getopt() processing
+    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':   // 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;
+            cur_opt.destination = OutputOption::DEST_CONSOLE;
+            if (strcmp(optarg, "stdout") == 0) {
+                cur_opt.stream = OutputOption::STR_STDOUT;
+
+            } else if (strcmp(optarg, "stderr") == 0) {
+                cur_opt.stream = OutputOption::STR_STDERR;
+
+            } else {
+                cerr << "Unrecognised console option: " << optarg << "\n";
+                return (1);
+            }
+            break;
+
+        case 'd':   // Debug level
+            cur_spec.setDbglevel(boost::lexical_cast<int>(optarg));
+            break;
+
+        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;
+            cur_opt.destination = OutputOption::DEST_FILE;
+            cur_opt.filename = optarg;
+            break;
+
+        case 'h':   // Help
+            usage();
+            return (0);
+
+        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;
+    }
+
+    // Add the current (unfinished specification) to the list.
+    cur_spec.addOutputOption(cur_opt);
+    loggers.push_back(cur_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_FATAL(logger_ex, MSG_WRITERR).arg("test1").arg("42");
+    LOG_ERROR(logger_ex, MSG_RDLOCMES).arg("dummy/file");
+    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,8 +17,9 @@
 
 #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;
@@ -27,11 +28,12 @@ using namespace std;
 
 class LoggerLevelTest : public ::testing::Test {
 protected:
-    LoggerLevelTest()
-    {}
-
-    ~LoggerLevelTest()
-    {}
+    LoggerLevelTest() {
+        // Logger initialization is done in main()
+    }
+    ~LoggerLevelTest() {
+        LoggerManager::reset();
+    }
 };
 
 
@@ -54,3 +56,27 @@ TEST_F(LoggerLevelTest, Creation) {
     EXPECT_EQ(isc::log::DEBUG, level3.severity);
     EXPECT_EQ(42, level3.dbglevel);
 }
+
+TEST(LoggerLevel, getSeverity) {
+    EXPECT_EQ(DEBUG, getSeverity("DEBUG"));
+    EXPECT_EQ(DEBUG, getSeverity("debug"));
+    EXPECT_EQ(DEBUG, getSeverity("DeBuG"));
+    EXPECT_EQ(INFO, getSeverity("INFO"));
+    EXPECT_EQ(INFO, getSeverity("info"));
+    EXPECT_EQ(INFO, getSeverity("iNfO"));
+    EXPECT_EQ(WARN, getSeverity("WARN"));
+    EXPECT_EQ(WARN, getSeverity("warn"));
+    EXPECT_EQ(WARN, getSeverity("wARn"));
+    EXPECT_EQ(ERROR, getSeverity("ERROR"));
+    EXPECT_EQ(ERROR, getSeverity("error"));
+    EXPECT_EQ(ERROR, getSeverity("ERRoR"));
+    EXPECT_EQ(FATAL, getSeverity("FATAL"));
+    EXPECT_EQ(FATAL, getSeverity("fatal"));
+    EXPECT_EQ(FATAL, getSeverity("FAtaL"));
+
+    // bad values should default to stdout
+    EXPECT_EQ(INFO, getSeverity("some bad value"));
+    EXPECT_EQ(INFO, getSeverity(""));
+
+    LoggerManager::reset();
+}

src/lib/log/tests/logger_manager_unittest.cc

@@ -0,0 +1,321 @@
+// 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 <stdio.h>
+#include <unistd.h>
+
+#include <fstream>
+#include <iostream>
+#include <string>
+
+#include <gtest/gtest.h>
+
+#include <boost/scoped_array.hpp>
+#include <boost/lexical_cast.hpp>
+
+#include <exceptions/exceptions.h>
+
+#include <log/macros.h>
+#include <log/messagedef.h>
+#include <log/logger.h>
+#include <log/logger_level.h>
+#include <log/logger_manager.h>
+#include <log/logger_specification.h>
+#include <log/output_option.h>
+
+#include "tempdir.h"
+
+using namespace isc;
+using namespace isc::log;
+using namespace std;
+
+/// \brief LoggerManager Test
+class LoggerManagerTest : public ::testing::Test {
+public:
+    LoggerManagerTest() {
+        // Initialization of logging is done in main()
+    }
+
+    ~LoggerManagerTest() {
+        LoggerManager::reset();
+    }
+};
+
+
+
+// Convenience class to create the specification for the logger "filelogger",
+// which, as the name suggests, logs to a file.  It remembers the file name and
+// deletes the file when instance of the class is destroyed.
+class SpecificationForFileLogger {
+public:
+
+    // Constructor - allocate file and create the specification object
+    SpecificationForFileLogger() : spec_(), name_(""), logname_("filelogger") {
+
+        // Set the output to a temporary file.
+        OutputOption option;
+        option.destination = OutputOption::DEST_FILE;
+        option.filename = name_ = createTempFilename();
+
+        // Set target output to the file logger.  The defauls indicate
+        // INFO severity.
+        spec_.setName(logname_);
+        spec_.addOutputOption(option);
+    }
+
+    // Destructor, remove the file.  This is only a test, so ignore failures
+    ~SpecificationForFileLogger() {
+        if (! name_.empty()) {
+            (void) unlink(name_.c_str());
+        }
+    }
+
+    // Return reference to the logging specification for this loggger
+    LoggerSpecification& getSpecification() {
+        return spec_;
+    }
+
+    // Return name of the logger
+    string getLoggerName() const {
+        return logname_;
+    }
+
+    // Return name of the file
+    string getFileName() const {
+        return name_;
+    }
+
+    // Create temporary filename
+    //
+    // The compiler warns against tmpnam() and suggests mkstemp instead.
+    // Unfortunately, this creates the filename and opens it.  So we need to
+    // close and delete the file before returning the name.  Also, the name
+    // is based on the template supplied and the name of the temporary
+    // directory may vary between systems.  So translate TMPDIR and if that
+    // does not exist, use /tmp.
+    //
+    // \return Temporary file name
+    std::string createTempFilename() {
+        string filename = TEMP_DIR + "/bind10_logger_manager_test_XXXXXX";
+
+        // Copy into writeable storage for the call to mkstemp
+        boost::scoped_array<char> tname(new char[filename.size() + 1]);
+        strcpy(tname.get(), filename.c_str());
+
+        // Create file, close and delete it, and store the name for later.
+        // There is still a race condition here, albeit a small one.
+        int filenum = mkstemp(tname.get());
+        if (filenum == -1) {
+            isc_throw(Exception, "Unable to obtain unique filename");
+        }
+        close(filenum);
+
+        return (string(tname.get()));
+    }
+
+
+private:
+    LoggerSpecification     spec_;      // Specification for this file logger
+    string                  name_;      // Name of the output file
+    string                  logname_;   // Name of this logger
+};
+
+
+// Convenience function to read an output log file and check that each line
+// contains the expected message ID
+//
+// \param filename Name of the file to check
+// \param start Iterator pointing to first expected message ID
+// \param finish Iterator pointing to last expected message ID
+template <typename T>
+void checkFileContents(const std::string& filename, T start, T finish) {
+
+    // Access the file for input
+    ifstream infile(filename.c_str());
+    if (! infile.good()) {
+        FAIL() << "Unable to open the logging file " << filename;
+    }
+
+    // Iterate round the expected message IDs and check that they appear in
+    // the string.
+    string line;    // Line read from the file
+
+    T i = start;    // Iterator
+    getline(infile, line);
+    int lineno = 1;
+
+    while ((i != finish) && (infile.good())) {
+
+        // Check that the message ID appears in the line.
+        EXPECT_TRUE(line.find(string(*i)) != string::npos)
+            << "Expected to find " << string(*i) << " on line " << lineno
+            << " of logging file " << filename;
+
+        // Go for the next line
+        ++i;
+        getline(infile, line);
+        ++lineno;
+    }
+
+    // Why did the loop end?
+    EXPECT_TRUE(i == finish) << "Did not reach the end of the message ID list";
+    EXPECT_TRUE(infile.eof()) << "Did not reach the end of the logging file";
+
+    // File will close when the instream is deleted at the end of this
+    // function.
+}
+
+// Check that the logger correctly creates something logging to a file.
+TEST_F(LoggerManagerTest, FileLogger) {
+
+    // Create a specification for the file logger and use the manager to
+    // connect the "filelogger" logger to it.
+    SpecificationForFileLogger file_spec;
+
+    // For the first test, we want to check that the file is created
+    // if it does not already exist.  So delete the temporary file before
+    // logging the first message.
+    unlink(file_spec.getFileName().c_str());
+
+    // Set up the file appenders.
+    LoggerManager manager;
+    manager.process(file_spec.getSpecification());
+
+    // Try logging to the file.  Local scope is set to ensure that the logger
+    // is destroyed before we reset the global logging.  We record what we
+    // put in the file for a later comparison.
+    vector<MessageID> ids;
+    {
+
+        // Scope-limit the logger to ensure it is destroyed after the brief
+        // check.  This adds weight to the idea that the logger will not
+        // keep the file open.
+        Logger logger(file_spec.getLoggerName());
+
+        LOG_FATAL(logger, MSG_DUPMSGID).arg("test");
+        ids.push_back(MSG_DUPMSGID);
+
+        LOG_FATAL(logger, MSG_DUPLNS).arg("test");
+        ids.push_back(MSG_DUPLNS);
+    }
+    LoggerManager::reset();
+
+    // At this point, the output file should contain two lines with messages
+    // MSG_DUPMSGID and MSG_DUPLNS messages - test this.
+    checkFileContents(file_spec.getFileName(), ids.begin(), ids.end());
+
+    // Re-open the file (we have to assume that it was closed when we
+    // reset the logger - there is no easy way to check) and check that
+    // new messages are appended to it.  We use the alternative
+    // invocation of process() here to check it works.
+    vector<LoggerSpecification> spec(1, file_spec.getSpecification());
+    manager.process(spec.begin(), spec.end());
+
+    // Create a new instance of the logger and log three more messages.
+    Logger logger(file_spec.getLoggerName());
+
+    LOG_FATAL(logger, MSG_IDNOTFND).arg("test");
+    ids.push_back(MSG_IDNOTFND);
+
+    LOG_FATAL(logger, MSG_INVMSGID).arg("test").arg("test2");
+    ids.push_back(MSG_INVMSGID);
+
+    LOG_FATAL(logger, MSG_NOMSGID).arg("42");
+    ids.push_back(MSG_NOMSGID);
+
+    // Close the file and check again
+    LoggerManager::reset();
+    checkFileContents(file_spec.getFileName(), ids.begin(), ids.end());
+}
+
+// Check if the file rolls over when it gets above a certain size.
+TEST_F(LoggerManagerTest, FileSizeRollover) {
+    // Set to a suitable minimum that log4cplus can copy with
+    static const size_t SIZE_LIMIT = 204800;
+
+    // Set up the name of the file.
+    SpecificationForFileLogger file_spec;
+    LoggerSpecification& spec = file_spec.getSpecification();
+
+    // Expand the option to ensure that a maximum version size is set.
+    LoggerSpecification::iterator opt = spec.begin();
+    EXPECT_TRUE(opt != spec.end());
+    opt->maxsize = SIZE_LIMIT;    // Bytes
+    opt->maxver = 2;
+
+    // The current current output file does not exist (the creation of file_spec
+    // ensures that.  Check that previous versions don't either.
+    vector<string> prev_name;
+    for (int i = 0; i < 3; ++i) {
+        prev_name.push_back(file_spec.getFileName() + "." +
+                            boost::lexical_cast<string>(i + 1));
+        (void) unlink(prev_name[i].c_str());
+    }
+
+    // Generate an argument for a message that ensures that the message when
+    // logged will be over that size.
+    string big_arg(SIZE_LIMIT, 'x');
+
+    // Set up the file logger
+    LoggerManager manager;
+    manager.process(spec);
+
+    // Log the message twice using different message IDs.  This should generate
+    // three files as for the log4cplus implementation, the files appear to
+    // be rolled after the message is logged.
+    {
+        Logger logger(file_spec.getLoggerName());
+        LOG_FATAL(logger, MSG_IDNOTFND).arg(big_arg);
+        LOG_FATAL(logger, MSG_DUPLNS).arg(big_arg);
+    }
+
+    // Check them.
+    LoggerManager::reset();     // Ensure files are closed
+
+    vector<MessageID> ids;
+    ids.push_back(MSG_IDNOTFND);
+    checkFileContents(prev_name[1], ids.begin(), ids.end());
+
+    ids.clear();
+    ids.push_back(MSG_DUPLNS);
+    checkFileContents(prev_name[0], ids.begin(), ids.end());
+
+    // Log another message and check that the files have rotated and that
+    // a .3 version does not exist.
+    manager.process(spec);
+    {
+        Logger logger(file_spec.getLoggerName());
+        LOG_FATAL(logger, MSG_NOMSGTXT).arg(big_arg);
+    }
+
+    LoggerManager::reset();     // Ensure files are closed
+
+    // Check that the files have moved.
+    ids.clear();
+    ids.push_back(MSG_DUPLNS);
+    checkFileContents(prev_name[1], ids.begin(), ids.end());
+
+    ids.clear();
+    ids.push_back(MSG_NOMSGTXT);
+    checkFileContents(prev_name[0], ids.begin(), ids.end());
+
+    // ... and check that the .3 version does not exist.
+    ifstream file3(prev_name[2].c_str(), ifstream::in);
+    EXPECT_FALSE(file3.good());
+
+    // Tidy up
+    for (int i = 0; i < prev_name.size(); ++i) {
+       (void) unlink(prev_name[i].c_str());
+    }
+}

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_specification_unittest.cc

@@ -0,0 +1,96 @@
+// 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 <string>
+
+#include <gtest/gtest.h>
+
+#include <log/logger_specification.h>
+#include <log/output_option.h>
+
+using namespace isc::log;
+using namespace std;
+
+// Check default initialization.
+TEST(LoggerSpecificationTest, DefaultInitialization) {
+    LoggerSpecification spec;
+
+    EXPECT_EQ(string(""), spec.getName());
+    EXPECT_EQ(isc::log::INFO, spec.getSeverity());
+    EXPECT_EQ(0, spec.getDbglevel());
+    EXPECT_FALSE(spec.getAdditive());
+    EXPECT_EQ(0, spec.optionCount());
+}
+
+// Non-default initialization
+TEST(LoggerSpecificationTest, Initialization) {
+    LoggerSpecification spec("alpha", isc::log::ERROR, 42, true);
+
+    EXPECT_EQ(string("alpha"), spec.getName());
+    EXPECT_EQ(isc::log::ERROR, spec.getSeverity());
+    EXPECT_EQ(42, spec.getDbglevel());
+    EXPECT_TRUE(spec.getAdditive());
+    EXPECT_EQ(0, spec.optionCount());
+}
+
+// Get/Set tests
+TEST(LoggerSpecificationTest, SetGet) {
+    LoggerSpecification spec;
+
+    spec.setName("gamma");
+    EXPECT_EQ(string("gamma"), spec.getName());
+
+    spec.setSeverity(isc::log::FATAL);
+    EXPECT_EQ(isc::log::FATAL, spec.getSeverity());
+
+    spec.setDbglevel(7);
+    EXPECT_EQ(7, spec.getDbglevel());
+
+    spec.setAdditive(true);
+    EXPECT_TRUE(spec.getAdditive());
+
+    // Should not affect option count
+    EXPECT_EQ(0, spec.optionCount());
+}
+
+// Check option setting
+TEST(LoggerSpecificationTest, AddOption) {
+    OutputOption option1;
+    option1.destination = OutputOption::DEST_FILE;
+    option1.filename = "/tmp/example.log";
+    option1.maxsize = 123456;
+
+    OutputOption option2;
+    option2.destination = OutputOption::DEST_SYSLOG;
+    option2.facility = "LOCAL7";
+
+    LoggerSpecification spec;
+    spec.addOutputOption(option1);
+    spec.addOutputOption(option2);
+    EXPECT_EQ(2, spec.optionCount());
+
+    // Iterate through them
+    LoggerSpecification::const_iterator i = spec.begin();
+
+    EXPECT_EQ(OutputOption::DEST_FILE, i->destination);
+    EXPECT_EQ(string("/tmp/example.log"), i->filename);
+    EXPECT_EQ(123456, i->maxsize);
+
+    ++i;
+    EXPECT_EQ(OutputOption::DEST_SYSLOG, i->destination);
+    EXPECT_EQ(string("LOCAL7"), i->facility);
+
+    ++i;
+    EXPECT_TRUE(i == spec.end());
+}

src/lib/log/tests/logger_support_test.cc

@@ -1,106 +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.
-
-/// \brief Example Program
-///
-/// Simple example program showing how to use the logger.
-
-#include <stdlib.h>
-#include <unistd.h>
-#include <string.h>
-
-#include <iostream>
-
-#include <log/logger.h>
-#include <log/macros.h>
-#include <log/logger_support.h>
-#include <log/root_logger_name.h>
-
-// Include a set of message definitions.
-#include <log/messagedef.h>
-
-using namespace isc::log;
-
-// Declare logger to use an example.
-Logger logger_ex("example");
-
-// The program is invoked:
-//
-// logger_support_test [-s severity] [-d level ] [local_file]
-//
-// "severity" is one of "debug", "info", "warn", "error", "fatal"
-// "level" is the debug level, a number between 0 and 99
-// "local_file" is the name of a local file.
-//
-// The program sets the attributes on the root logger and logs a set of
-// messages.  Looking at the output determines whether the program worked.
-
-int main(int argc, char** argv) {
-
-    isc::log::Severity  severity = isc::log::INFO;  // Default logger severity
-    int                 dbglevel = -1;              // Logger debug level
-    const char*         localfile = NULL;           // Local message file
-    int                 option;                     // For getopt() processing
-    Logger              logger_dlm("dlm");          // Another example logger
-
-    // Parse options
-    while ((option = getopt(argc, argv, "s:d:")) != -1) {
-        switch (option) {
-            case 's':
-                if (strcmp(optarg, "debug") == 0) {
-                    severity = isc::log::DEBUG;
-                } else if (strcmp(optarg, "info") == 0) {
-                    severity = isc::log::INFO;
-                } else if (strcmp(optarg, "warn") == 0) {
-                    severity = isc::log::WARN;
-                } else if (strcmp(optarg, "error") == 0) {
-                    severity = isc::log::ERROR;
-                } else if (strcmp(optarg, "fatal") == 0) {
-                    severity = isc::log::FATAL;
-                } else {
-                    std::cout << "Unrecognised severity option: " <<
-                        optarg << "\n";
-                    exit(1);
-                }
-                break;
-
-            case 'd':
-                dbglevel = atoi(optarg);
-                break;
-
-            default:
-                std::cout << "Unrecognised option: " <<
-                    static_cast<char>(option) << "\n";
-        }
-    }
-
-    if (optind < argc) {
-        localfile = argv[optind];
-    }
-
-    // Update the logging parameters
-    initLogger("alpha", severity, dbglevel, localfile);
-
-    // Log a few messages
-    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");
-
-    return (0);
-}

src/lib/log/tests/logger_unittest.cc

@@ -17,30 +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;
 
-/// \brief Derived logger
-///
-/// Only exists to make the protected static methods in Logger public for
-/// test purposes.
-
-class DerivedLogger : public isc::log::Logger {
-public:
-    DerivedLogger(std::string name) : isc::log::Logger(name)
-    {}
-
-    static void reset() {
-        isc::log::Logger::reset();
-    }
-};
-
-
 /// \brief Logger Test
 ///
 /// As the logger is only a shell around the implementation, this tests also
@@ -48,11 +33,11 @@ public:
 
 class LoggerTest : public ::testing::Test {
 public:
-    LoggerTest()
-    {}
-    ~LoggerTest()
-    {
-        DerivedLogger::reset();
+    LoggerTest() {
+        // Initialization of logging is done in main()
+    }
+    ~LoggerTest() {
+        LoggerManager::reset();
     }
 };
 
@@ -62,11 +47,10 @@ public:
 TEST_F(LoggerTest, Name) {
 
     // Create a logger
-    setRootLoggerName("test1");
     Logger logger("alpha");
 
     // ... and check the name
-    EXPECT_EQ(string("test1.alpha"), logger.getName());
+    EXPECT_EQ(getRootLoggerName() + string(".alpha"), logger.getName());
 }
 
 // This test attempts to get two instances of a logger with the same name
@@ -74,10 +58,6 @@ TEST_F(LoggerTest, Name) {
 
 TEST_F(LoggerTest, GetLogger) {
 
-    // Set the root logger name (not strictly needed, but this will be the
-    // case in the program(.
-    setRootLoggerName("test2");
-
     const string name1 = "alpha";
     const string name2 = "beta";
 
@@ -98,7 +78,6 @@ TEST_F(LoggerTest, GetLogger) {
 TEST_F(LoggerTest, Severity) {
 
     // Create a logger
-    setRootLoggerName("test3");
     Logger logger("alpha");
 
     // Now check the levels
@@ -129,7 +108,6 @@ TEST_F(LoggerTest, Severity) {
 TEST_F(LoggerTest, DebugLevels) {
 
     // Create a logger
-    setRootLoggerName("test4");
     Logger logger("alpha");
 
     // Debug level should be 0 if not at debug severity
@@ -171,11 +149,45 @@ TEST_F(LoggerTest, DebugLevels) {
 
 TEST_F(LoggerTest, SeverityInheritance) {
 
-    // Create to 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>.
+    // 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");
+
+    // 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());
 
-    setRootLoggerName("test5");
+    // 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");
 
@@ -203,11 +215,9 @@ TEST_F(LoggerTest, SeverityInheritance) {
 
 TEST_F(LoggerTest, EffectiveSeverityInheritance) {
 
-    // Create to 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>.
-
-    setRootLoggerName("test6");
+    // 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("test6");
     Logger child("test6.beta");
 
@@ -242,7 +252,6 @@ TEST_F(LoggerTest, EffectiveSeverityInheritance) {
 
 TEST_F(LoggerTest, IsXxxEnabled) {
 
-    setRootLoggerName("test7");
     Logger logger("test7");
 
     logger.setSeverity(isc::log::INFO);
@@ -313,7 +322,6 @@ TEST_F(LoggerTest, IsXxxEnabled) {
 
 TEST_F(LoggerTest, IsDebugEnabledLevel) {
 
-    setRootLoggerName("test8");
     Logger logger("test8");
 
     int MID_LEVEL = (MIN_DEBUG_LEVEL + MAX_DEBUG_LEVEL) / 2;

src/lib/log/tests/output_option_unittest.cc

@@ -0,0 +1,66 @@
+// 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 <string>
+
+#include <gtest/gtest.h>
+
+#include <log/output_option.h>
+
+using namespace isc::log;
+using namespace std;
+
+// As OutputOption is a struct, the only meaningful test is to check that it
+// initializes correctly.
+
+TEST(OutputOptionTest, Initialization) {
+    OutputOption option;
+
+    EXPECT_EQ(OutputOption::DEST_CONSOLE, option.destination);
+    EXPECT_EQ(OutputOption::STR_STDERR, option.stream);
+    EXPECT_FALSE(option.flush);
+    EXPECT_EQ(string("LOCAL0"), option.facility);
+    EXPECT_EQ(string(""), option.filename);
+    EXPECT_EQ(0, option.maxsize);
+    EXPECT_EQ(0, option.maxver);
+}
+
+TEST(OutputOption, getDestination) {
+    EXPECT_EQ(OutputOption::DEST_CONSOLE, getDestination("console"));
+    EXPECT_EQ(OutputOption::DEST_CONSOLE, getDestination("CONSOLE"));
+    EXPECT_EQ(OutputOption::DEST_CONSOLE, getDestination("CoNSoLE"));
+    EXPECT_EQ(OutputOption::DEST_FILE, getDestination("file"));
+    EXPECT_EQ(OutputOption::DEST_FILE, getDestination("FILE"));
+    EXPECT_EQ(OutputOption::DEST_FILE, getDestination("fIlE"));
+    EXPECT_EQ(OutputOption::DEST_SYSLOG, getDestination("syslog"));
+    EXPECT_EQ(OutputOption::DEST_SYSLOG, getDestination("SYSLOG"));
+    EXPECT_EQ(OutputOption::DEST_SYSLOG, getDestination("SYSlog"));
+
+    // bad values should default to DEST_CONSOLE
+    EXPECT_EQ(OutputOption::DEST_CONSOLE, getDestination("SOME_BAD_VALUE"));
+}
+
+TEST(OutputOption, getStream) {
+    EXPECT_EQ(OutputOption::STR_STDOUT, getStream("stdout"));
+    EXPECT_EQ(OutputOption::STR_STDOUT, getStream("STDOUT"));
+    EXPECT_EQ(OutputOption::STR_STDOUT, getStream("STdouT"));
+    EXPECT_EQ(OutputOption::STR_STDERR, getStream("stderr"));
+    EXPECT_EQ(OutputOption::STR_STDERR, getStream("STDERR"));
+    EXPECT_EQ(OutputOption::STR_STDERR, getStream("StDeRR"));
+
+    // bad values should default to stdout
+    EXPECT_EQ(OutputOption::STR_STDOUT, getStream("some bad value"));
+    EXPECT_EQ(OutputOption::STR_STDOUT, getStream(""));
+}
+

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

@@ -1,90 +0,0 @@
-#!/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.
-
-failcount=0
-localmes=@abs_builddir@/localdef_mes_$$
-tempfile=@abs_builddir@/run_time_init_test_tempfile_$$
-
-passfail() {
-    if [ $1 -eq 0 ]; then
-        echo "pass"
-    else
-        echo "FAIL"
-    fi
-    failcount=`expr $failcount + $1`
-}
-
-# Create the local message file for testing
-
-cat > $localmes << .
-\$PREFIX MSG_
-% NOTHERE     this message is not in the global dictionary
-% READERR     replacement read error, parameters: '%1' and '%2'
-% RDLOCMES    replacement read local message file, parameter is '%1'
-.
-
-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
-.
-./logger_support_test | cut -d' ' -f3- | diff $tempfile -
-passfail $?
-
-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
-.
-./logger_support_test -s error | cut -d' ' -f3- | diff $tempfile -
-passfail $?
-
-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
-.
-./logger_support_test -s debug -d 25 | cut -d' ' -f3- | diff $tempfile -
-passfail $?
-
-echo -n "4. 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'
-.
-./logger_support_test -s warn $localmes | cut -d' ' -f3- | diff $tempfile -
-passfail $?
-
-rm -f $localmes
-rm -f $tempfile
-
-if [ $failcount -eq 0 ]; then
-    echo "PASS: run_time_init_test"
-elif [ $failcount -eq 1 ]; then
-    echo "FAIL: run_time_init_test - 1 test failed"
-else
-    echo "FAIL: run_time_init_test - $failcount tests failed"
-fi
-
-exit $failcount

src/lib/log/tests/run_unittests.cc

@@ -15,8 +15,11 @@
 #include <gtest/gtest.h>
 #include <util/unittests/run_all.h>
 
+#include <log/logger_support.h>
+
 int
 main(int argc, char* argv[]) {
     ::testing::InitGoogleTest(&argc, argv);
+    isc::log::initLogger();
     return (isc::util::unittests::run_all());
 }

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

@@ -0,0 +1,91 @@
+#!/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="Severity test"
+echo $testname
+
+failcount=0
+tempfile=@abs_builddir@/severity_test_tempfile_$$
+
+passfail() {
+    if [ $1 -eq 0 ]; then
+        echo " pass"
+    else
+        echo " FAIL"
+        failcount=`expr $failcount + $1`
+    fi
+}
+
+echo -n "1. runInitTest default parameters:"
+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
+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 -n "2. Severity filter:"
+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
+.
+./logger_example -c stdout -s error | cut -d' ' -f3- | diff $tempfile -
+passfail $?
+
+echo -n "3. Debug level:"
+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
+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 $?
+
+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
+
+exit $failcount

src/lib/log/tests/tempdir.h.in

@@ -0,0 +1,29 @@
+// 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 __TEMPDIR_H
+#define __TEMPDIR_H
+
+/// \brief Define temporary directory
+///
+/// Defines the temporary directory in which temporary files used by the
+/// unit tests are created.
+
+#include <string>
+
+namespace {
+std::string TEMP_DIR("@builddir@");
+}
+
+#endif // __TEMPDIR_H

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/datasrc/sqlite3_ds.py

@@ -235,13 +235,13 @@ def load(dbfile, zone, reader):
         zone += '.'
 
     conn, cur = open(dbfile)
-    old_zone_id = get_zoneid(zone, cur)
+    try:
+        old_zone_id = get_zoneid(zone, cur)
 
-    temp = str(random.randrange(100000))
-    cur.execute("INSERT INTO zones (name, rdclass) VALUES (?, 'IN')", [temp])
-    new_zone_id = cur.lastrowid
+        temp = str(random.randrange(100000))
+        cur.execute("INSERT INTO zones (name, rdclass) VALUES (?, 'IN')", [temp])
+        new_zone_id = cur.lastrowid
 
-    try:
         for name, ttl, rdclass, rdtype, rdata in reader():
             sigtype = ''
             if rdtype.lower() == 'rrsig':
@@ -266,20 +266,20 @@ def load(dbfile, zone, reader):
                                VALUES (?, ?, ?, ?, ?, ?)""",
                             [new_zone_id, name, reverse_name(name), ttl,
                              rdtype, rdata])
+
+        if old_zone_id:
+            cur.execute("DELETE FROM zones WHERE id=?", [old_zone_id])
+            cur.execute("UPDATE zones SET name=? WHERE id=?", [zone, new_zone_id])
+            conn.commit()
+            cur.execute("DELETE FROM records WHERE zone_id=?", [old_zone_id])
+            cur.execute("DELETE FROM nsec3 WHERE zone_id=?", [old_zone_id])
+            conn.commit()
+        else:
+            cur.execute("UPDATE zones SET name=? WHERE id=?", [zone, new_zone_id])
+            conn.commit()
     except Exception as e:
         fail = "Error while loading " + zone + ": " + e.args[0]
         raise Sqlite3DSError(fail)
-
-    if old_zone_id:
-        cur.execute("DELETE FROM zones WHERE id=?", [old_zone_id])
-        cur.execute("UPDATE zones SET name=? WHERE id=?", [zone, new_zone_id])
-        conn.commit()
-        cur.execute("DELETE FROM records WHERE zone_id=?", [old_zone_id])
-        cur.execute("DELETE FROM nsec3 WHERE zone_id=?", [old_zone_id])
-        conn.commit()
-    else:
-        cur.execute("UPDATE zones SET name=? WHERE id=?", [zone, new_zone_id])
-        conn.commit()
-
-    cur.close()
-    conn.close()
+    finally:
+        cur.close()
+        conn.close()

src/lib/python/isc/datasrc/tests/Makefile.am

@@ -4,6 +4,7 @@ EXTRA_DIST = $(PYTESTS)
 
 EXTRA_DIST += testdata/brokendb.sqlite3
 EXTRA_DIST += testdata/example.com.sqlite3
+CLEANFILES = $(abs_builddir)/example.com.out.sqlite3
 
 # test using command-line arguments, so use check-local target instead of TESTS
 check-local:
@@ -16,5 +17,6 @@ endif
 	echo Running test: $$pytest ; \
 	env PYTHONPATH=$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python:$(abs_top_builddir)/src/lib/python/isc/log \
 	TESTDATA_PATH=$(abs_srcdir)/testdata \
+	TESTDATA_WRITE_PATH=$(abs_builddir) \
 	$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
 	done

src/lib/python/isc/datasrc/tests/sqlite3_ds_test.py

@@ -17,8 +17,31 @@ from isc.datasrc import sqlite3_ds
 import os
 import socket
 import unittest
+import sqlite3
 
 TESTDATA_PATH = os.environ['TESTDATA_PATH'] + os.sep
+TESTDATA_WRITE_PATH = os.environ['TESTDATA_WRITE_PATH'] + os.sep
+
+READ_ZONE_DB_FILE = TESTDATA_PATH + "example.com.sqlite3"
+WRITE_ZONE_DB_FILE = TESTDATA_WRITE_PATH + "example.com.out.sqlite3"
+BROKEN_DB_FILE = TESTDATA_PATH + "brokendb.sqlite3"
+
+def example_reader():
+    my_zone = [
+        ("example.com.",    "3600",    "IN",  "SOA", "ns.example.com. admin.example.com. 1234 3600 1800 2419200 7200"),
+        ("example.com.",    "3600",    "IN",  "NS", "ns.example.com."),
+        ("ns.example.com.", "3600",    "IN",  "A", "192.0.2.1")
+    ]
+    for rr in my_zone:
+        yield rr
+
+def example_reader_nested():
+    # this iterator is used in the 'locked' test; it will cause
+    # the load() method to try and write to the same database
+    sqlite3_ds.load(WRITE_ZONE_DB_FILE,
+                    ".",
+                    example_reader)
+    return example_reader()
 
 class TestSqlite3_ds(unittest.TestCase):
     def test_zone_exist(self):
@@ -33,11 +56,40 @@ class TestSqlite3_ds(unittest.TestCase):
         # Open a broken database file
         self.assertRaises(sqlite3_ds.Sqlite3DSError,
                           sqlite3_ds.zone_exist, "example.com",
-                          TESTDATA_PATH + "brokendb.sqlite3")
+                          BROKEN_DB_FILE)
         self.assertTrue(sqlite3_ds.zone_exist("example.com.",
-                            TESTDATA_PATH + "example.com.sqlite3"))
+                        READ_ZONE_DB_FILE))
         self.assertFalse(sqlite3_ds.zone_exist("example.org.",
-                            TESTDATA_PATH + "example.com.sqlite3"))
+                         READ_ZONE_DB_FILE))
+
+    def test_load_db(self):
+        sqlite3_ds.load(WRITE_ZONE_DB_FILE, ".", example_reader)
+
+    def test_locked_db(self):
+        # load it first to make sure it exists
+        sqlite3_ds.load(WRITE_ZONE_DB_FILE, ".", example_reader)
+
+        # and manually create a writing session as well
+        con = sqlite3.connect(WRITE_ZONE_DB_FILE);
+        cur = con.cursor()
+        cur.execute("delete from records")
+
+        self.assertRaises(sqlite3_ds.Sqlite3DSError,
+                          sqlite3_ds.load, WRITE_ZONE_DB_FILE, ".",
+                          example_reader)
+
+        con.rollback()
+
+        # and make sure lock does not stay
+        sqlite3_ds.load(WRITE_ZONE_DB_FILE, ".", example_reader)
+
+        # force locked db by nested loads
+        self.assertRaises(sqlite3_ds.Sqlite3DSError,
+                          sqlite3_ds.load, WRITE_ZONE_DB_FILE, ".",
+                          example_reader_nested)
+
+        # and make sure lock does not stay
+        sqlite3_ds.load(WRITE_ZONE_DB_FILE, ".", example_reader)
 
 if __name__ == '__main__':
     unittest.main()

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);