[trac998] Merge branch 'master' into trac998

Conflicts:
	src/lib/acl/Makefile.am
	src/lib/acl/tests/Makefile.am

ChangeLog

@@ -1,3 +1,42 @@
+261.	[func]      stephen
+	Add new-style logging messages to b10-auth.
+	(Trac 738, git c021505a1a0d6ecb15a8fd1592b94baff6d115f4)
+
+260.	[func]      stephen
+	Remove comma between message identification and the message
+	text in the new-style logging messages.
+	(Trac 1031, git 1c7930a7ba19706d388e4f8dcf2a55a886b74cd2)
+
+259.	[bug]       stephen
+	Logging now correctly initialized in b10-auth.  Also, fixed
+	bug whereby querying for "version.bind txt ch" would cause
+	b10-auth to crash if BIND 10 was started with the "-v" switch.
+	(Trac 1022,1023, git 926a65fa08617be677a93e9e388df0f229b01067)
+
+258.	[build]		jelte
+	Now builds and runs with Python 3.2
+	(Trac #710, git dae1d2e24f993e1eef9ab429326652f40a006dfb)
+
+257.	[bug]           y-aharen
+	Fixed a bug an instance of IntervalTimerImpl may be destructed 
+	while deadline_timer is holding the handler. This fix addresses
+	occasional failure of IntervalTimerTest.destructIntervalTimer.
+	(Trac #957, git e59c215e14b5718f62699ec32514453b983ff603)
+
+256.	[bug]		jerry
+	src/bin/xfrin: update xfrin to check TSIG before other part of
+	incoming message.
+	(Trac955, git 261450e93af0b0406178e9ef121f81e721e0855c)
+
+255.	[func]		zhang likun
+	src/lib/cache:  remove empty code in lib/cache and the corresponding
+	suppression rule in	src/cppcheck-suppress.lst.
+	(Trac639, git 4f714bac4547d0a025afd314c309ca5cb603e212)
+
+254.	[bug]		jinmei
+	b10-xfrout: failed to send notifies over IPv6 correctly.
+	(Trac964, git 3255c92714737bb461fb67012376788530f16e40)
+
 253.    [func]		jelte
 	Add configuration options for logging through the virtual module
 	Logging.
@@ -17,10 +56,10 @@
 	(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)
+	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.

configure.ac

@@ -280,6 +280,35 @@ namespace isc {class Bar {Foo foo_;};} ],,
 	[AC_MSG_RESULT(yes)])
 CXXFLAGS="$CXXFLAGS_SAVED"
 
+# Python 3.2 has an unused parameter in one of its headers. This
+# has been reported, but not fixed as of yet, so we check if we need
+# to set -Wno-unused-parameter.
+if test $werror_ok = 1; then
+	CPPFLAGS_SAVED="$CPPFLAGS"
+	CPPFLAGS=${PYTHON_INCLUDES}
+	CXXFLAGS_SAVED="$CXXFLAGS"
+	CXXFLAGS="$CXXFLAGS $B10_CXXFLAGS -Werror"
+	AC_MSG_CHECKING([whether we need -Wno-unused-parameter for python])
+	AC_TRY_COMPILE(
+		[#include <Python.h>],
+		[],
+		[AC_MSG_RESULT(no)],
+		[
+		CXXFLAGS="$CXXFLAGS -Wno-unused-parameter"
+		AC_TRY_COMPILE([#include <Python.h>],
+		[],
+		[AC_MSG_RESULT(yes)
+		 PYTHON_CXXFLAGS="${PYTHON_CXXFLAGS} -Wno-unused-parameter"
+		 AC_SUBST(PYTHON_CXXFLAGS)
+		],
+		[AC_MSG_ERROR([Can't compile against Python.h])]
+                )
+                ]
+	)
+	CXXFLAGS="$CXXFLAGS_SAVED"
+	CPPFLAGS="$CPPFLAGS_SAVED"
+fi
+
 fi				dnl GXX = yes
 
 AM_CONDITIONAL(GCC_WERROR_OK, test $werror_ok = 1)
@@ -858,12 +887,6 @@ AC_OUTPUT([doc/version.ent
            src/bin/zonemgr/run_b10-zonemgr.sh
            src/bin/stats/stats.py
            src/bin/stats/stats_httpd.py
-           src/bin/stats/stats.spec
-           src/bin/stats/stats-schema.spec
-           src/bin/stats/stats-httpd.spec
-           src/bin/stats/stats-httpd-xml.tpl
-           src/bin/stats/stats-httpd-xsd.tpl
-           src/bin/stats/stats-httpd-xsl.tpl
            src/bin/bind10/bind10.py
            src/bin/bind10/run_bind10.sh
            src/bin/bind10/tests/bind10_test.py
@@ -885,8 +908,8 @@ AC_OUTPUT([doc/version.ent
            src/lib/config/tests/data_def_unittests_config.h
            src/lib/python/isc/config/tests/config_test
            src/lib/python/isc/cc/tests/cc_test
-           src/lib/python/isc/log/tests/log_test
            src/lib/python/isc/notify/tests/notify_out_test
+           src/lib/python/isc/log/tests/log_console.py
            src/lib/dns/gen-rdatacode.py
            src/lib/python/bind10_config.py
            src/lib/dns/tests/testdata/gen-wiredata.py
@@ -928,6 +951,7 @@ AC_OUTPUT([doc/version.ent
            chmod +x src/lib/log/tests/destination_test.sh
            chmod +x src/lib/log/tests/severity_test.sh
            chmod +x src/lib/util/python/mkpywrapper.py
+           chmod +x src/lib/python/isc/log/tests/log_console.py
            chmod +x tests/system/conf.sh
           ])
 AC_OUTPUT
@@ -954,6 +978,7 @@ Flags:
   B10_CXXFLAGS:  $B10_CXXFLAGS
 dnl includes too
   Python:        ${PYTHON_INCLUDES}
+                 ${PYTHON_CXXFLAGS}
                  ${PYTHON_LDFLAGS}
                  ${PYTHON_LIB}
   Boost:         ${BOOST_INCLUDES}

doc/guide/bind10-guide.xml

@@ -13,7 +13,7 @@
     <subtitle>Administrator Reference for BIND 10</subtitle>
 
     <copyright>
-      <year>2010</year><holder>Internet Systems Consortium, Inc.</holder>
+      <year>2010-2011</year><holder>Internet Systems Consortium, Inc.</holder>
     </copyright>
 
     <abstract>
@@ -81,15 +81,13 @@
 
       <para>
 	BIND 10 uses the Botan crypto library for C++. It requires
-	at least Botan version 1.8. To build BIND 10, install the
-	Botan libraries and development include headers.
+	at least Botan version 1.8.
       </para>
 
-<!--
-TODO
-Debian and Ubuntu:
- libgmp3-dev and libbz2-dev required for botan too
--->
+      <para>
+	BIND 10 uses the log4cplus C++ logging library. It requires
+	at least log4cplus version 1.0.3.
+      </para>
 
       <para>
 	The authoritative server requires SQLite 3.3.9 or newer.
@@ -303,6 +301,12 @@ var/
 
     <section>
       <title>Building Requirements</title>
+
+        <para>
+          In addition to the run-time requirements, building BIND 10
+          from source code requires various development include headers.
+        </para>
+
         <note>
           <simpara>
             Some operating systems have split their distribution packages into
@@ -320,6 +324,19 @@ var/
         </para>
 
         <para>
+	  To build BIND 10, also install the Botan (at least version
+	  1.8) and the log4cplus (at least version 1.0.3)
+          development include headers.
+        </para>
+
+<!--
+TODO
+Debian and Ubuntu:
+ libgmp3-dev and libbz2-dev required for botan too
+-->
+
+        <para>
+<!-- TODO: is this needed at build time? test time? -->
 	  The Python Library and Python _sqlite3 module are required to
           enable the Xfrout and Xfrin support.
         </para>
@@ -333,7 +350,7 @@ var/
           Building BIND 10 also requires a C++ compiler and
           standard development headers, make, and pkg-config.
           BIND 10 builds have been tested with GCC g++ 3.4.3, 4.1.2,
-          4.1.3, 4.2.1, 4.3.2, and 4.4.1.
+          4.1.3, 4.2.1, 4.3.2, and 4.4.1; Clang++ 2.8; and Sun C++ 5.10.
         </para>
     </section>
 

src/bin/auth/Makefile.am

@@ -16,7 +16,8 @@ endif
 
 pkglibexecdir = $(libexecdir)/@PACKAGE@
 
-CLEANFILES = *.gcno *.gcda auth.spec spec_config.h
+CLEANFILES  = *.gcno *.gcda auth.spec spec_config.h
+CLEANFILES += auth_messages.h auth_messages.cc
 
 man_MANS = b10-auth.8
 EXTRA_DIST = $(man_MANS) b10-auth.xml
@@ -34,16 +35,25 @@ auth.spec: auth.spec.pre
 spec_config.h: spec_config.h.pre
 	$(SED) -e "s|@@LOCALSTATEDIR@@|$(localstatedir)|" spec_config.h.pre >$@
 
-BUILT_SOURCES = spec_config.h
+auth_messages.h auth_messages.cc: auth_messages.mes
+	$(top_builddir)/src/lib/log/compiler/message $(top_srcdir)/src/bin/auth/auth_messages.mes
+
+BUILT_SOURCES = spec_config.h auth_messages.h auth_messages.cc
+
 pkglibexec_PROGRAMS = b10-auth
 b10_auth_SOURCES = query.cc query.h
 b10_auth_SOURCES += auth_srv.cc auth_srv.h
+b10_auth_SOURCES += auth_log.cc auth_log.h
 b10_auth_SOURCES += change_user.cc change_user.h
 b10_auth_SOURCES += auth_config.cc auth_config.h
 b10_auth_SOURCES += command.cc command.h
 b10_auth_SOURCES += common.h common.cc
 b10_auth_SOURCES += statistics.cc statistics.h
 b10_auth_SOURCES += main.cc
+
+nodist_b10_auth_SOURCES = auth_messages.h auth_messages.cc
+EXTRA_DIST += auth_messages.mes
+
 b10_auth_LDADD =  $(top_builddir)/src/lib/datasrc/libdatasrc.la
 b10_auth_LDADD += $(top_builddir)/src/lib/dns/libdns++.la
 b10_auth_LDADD += $(top_builddir)/src/lib/config/libcfgclient.la

src/bin/auth/auth_log.cc

@@ -0,0 +1,26 @@
+// 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.
+
+/// Defines the logger used by the top-level component of b10-auth.
+
+#include "auth_log.h"
+
+namespace isc {
+namespace auth {
+
+isc::log::Logger auth_logger("auth");
+
+} // namespace auth
+} // namespace isc
+

src/bin/auth/auth_log.h

@@ -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.
+
+#ifndef __AUTH_LOG__H
+#define __AUTH_LOG__H
+
+#include <log/macros.h>
+#include <auth/auth_messages.h>
+
+namespace isc {
+namespace auth {
+
+/// \brief Auth Logging
+///
+/// Defines the levels used to output debug messages in the "auth" part of
+/// the b10-auth program.  Higher numbers equate to more verbose (and detailed)
+/// output.
+
+// Debug messages indicating normal startup are logged at this debug level.
+const int DBG_AUTH_START = 10;
+
+// Debug level used to log setting information (such as configuration changes).
+const int DBG_AUTH_OPS = 30;
+
+// Trace detailed operations, including errors raised when processing invalid
+// packets.  (These are not logged at severities of WARN or higher for fear
+// that a set of deliberately invalid packets set to the authoritative server
+// could overwhelm the logging.)
+const int DBG_AUTH_DETAIL = 50;
+
+// This level is used to log the contents of packets received and sent.
+const int DBG_AUTH_MESSAGES = 70;
+
+/// Define the logger for the "auth" module part of b10-auth.  We could define
+/// a logger in each file, but we would want to define a common name to avoid
+/// spelling mistakes, so it is just one small step from there to define a
+/// module-common logger.
+extern isc::log::Logger auth_logger;
+
+} // namespace nsas
+} // namespace isc
+
+#endif // __AUTH_LOG__H

src/bin/auth/auth_messages.mes

@@ -0,0 +1,260 @@
+# 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.
+
+$NAMESPACE isc::auth
+
+% AUTH_AXFR_ERROR error handling AXFR request: %1
+This is a debug message produced by the authoritative server when it
+has encountered an error processing an AXFR request. The message gives
+the reason for the error, and the server will return a SERVFAIL code to
+the sender.
+
+% AUTH_AXFR_UDP AXFR query received over UDP
+This is a debug message output when the authoritative server has received
+an AXFR query over UDP. Use of UDP for AXFRs is not permitted by the
+protocol, so the server will return a FORMERR error to the sender.
+
+% AUTH_COMMAND_FAILED execution of command channel instruction '%1' failed: %2
+Execution of the specified command by the authoritative server failed. The
+message contains the reason for the failure.
+
+% AUTH_CONFIG_CHANNEL_CREATED configuration session channel created
+This is a debug message indicating that authoritative server has created
+the channel to the configuration manager.  It is issued during server
+startup is an indication that the initialization is proceeding normally.
+
+% AUTH_CONFIG_CHANNEL_ESTABLISHED configuration session channel established
+This is a debug message indicating that authoritative server
+has established communication the configuration manager over the
+previously-created channel. It is issued during server startup is an
+indication that the initialization is proceeding normally.
+
+% AUTH_CONFIG_CHANNEL_STARTED configuration session channel started
+This is a debug message, issued when the authoritative server has
+posted a request to be notified when new configuration information is
+available. It is issued during server startup is an indication that
+the initialization is proceeding normally.
+
+% AUTH_CONFIG_LOAD_FAIL load of configuration failed: %1
+An attempt to configure the server with information from the configuration
+database during the startup sequence has failed. (The reason for
+the failure is given in the message.) The server will continue its
+initialization although it may not be configured in the desired way.
+
+% AUTH_CONFIG_UPDATE_FAIL update of configuration failed: %1
+At attempt to update the configuration the server with information
+from the configuration database has failed, the reason being given in
+the message.
+
+% AUTH_DATA_SOURCE data source database file: %1
+This is a debug message produced by the authoritative server when it accesses a
+datebase data source, listing the file that is being accessed.
+
+% AUTH_DNS_SERVICES_CREATED DNS services created
+This is a debug message indicating that the component that will handling
+incoming queries for the authoritiative server (DNSServices) has been
+successfully created. It is issued during server startup is an indication
+that the initialization is proceeding normally.
+
+% AUTH_HEADER_PARSE_FAIL unable to parse header in received DNS packet: %1
+This is a debug message, generated by the authoritative server when an
+attempt to parse the header of a received DNS packet has failed. (The
+reason for the failure is given in the message.) The server will drop the
+packet.
+
+% AUTH_LOAD_TSIG loading TSIG keys
+This is a debug message indicating that the authoritiative server
+has requested the keyring holding TSIG keys from the configuration
+database. It is issued during server startup is an indication that the
+initialization is proceeding normally.
+
+% AUTH_LOAD_ZONE loaded zone %1/%2
+This debug message is issued during the processing of the 'loadzone' command
+when the authoritative server has successfully loaded the named zone of the
+named class.
+
+% AUTH_MEM_DATASRC_DISABLED memory data source is disabled for class %1
+This is a debug message reporting that the authoritative server has
+discovered that the memory data source is disabled for the given class.
+
+% AUTH_MEM_DATASRC_ENABLED memory data source is enabled for class %1
+This is a debug message reporting that the authoritative server has
+discovered that the memory data source is enabled for the given class.
+
+% AUTH_NO_STATS_SESSION session interface for statistics is not available
+The authoritative server had no session with the statistics module at the
+time it attempted to send it data: the attempt has been abandoned. This
+could be an error in configuration.
+
+% AUTH_NO_XFRIN received NOTIFY but XFRIN session is not running
+This is a debug message produced by the authoritative server when it receives
+a NOTIFY packet but the XFRIN process is not running. The packet will be
+dropped and nothing returned to the sender.
+
+% AUTH_NOTIFY_RRTYPE invalid question RR type (%1) in incoming NOTIFY
+This debug message is logged by the authoritative server when it receives
+a NOTIFY packet that an RR type of something other than SOA in the
+question section. (The RR type received is included in the message.) The
+server will return a FORMERR error to the sender.
+
+% AUTH_NOTIFY_QUESTIONS invalid number of questions (%1) in incoming NOTIFY
+This debug message is logged by the authoritative server when it receives
+a NOTIFY packet that contains zero or more than one question. (A valid
+NOTIFY packet contains one question.) The server will return a FORMERR
+error to the sender.
+
+% AUTH_PACKET_PARSE_ERROR unable to parse received DNS packet: %1
+This is a debug message, generated by the authoritative server when an
+attempt to parse a received DNS packet has failed due to something other
+than a protocol error. The reason for the failure is given in the message;
+the server will return a SERVFAIL error code to the sender.
+
+% AUTH_PACKET_PROTOCOL_ERROR DNS packet protocol error: %1. Returning %2
+This is a debug message, generated by the authoritative server when an
+attempt to parse a received DNS packet has failed due to a protocol error.
+The reason for the failure is given in the message, as is the error code
+that will be returned to the sender.
+
+% AUTH_PACKET_RECEIVED message received:\n%1
+This is a debug message output by the authoritative server when it
+receives a valid DNS packet.
+
+Note: This message includes the packet received, rendered in the form of
+multiple lines of text. For this reason, it is suggested that this log message
+not be routed to the syslog file, where the multiple lines could confuse
+programs that expect a format of one message per line.
+
+% AUTH_PROCESS_FAIL message processing failure: %1
+This message is generated by the authoritative server when it has
+encountered an internal error whilst processing a received packet:
+the cause of the error is included in the message.
+
+The server will return a SERVFAIL error code to the sender of the packet.
+However, this message indicates a potential error in the server.
+Please open a bug ticket for this issue.
+
+% AUTH_RECEIVED_COMMAND command '%1' received
+This is a debug message issued when the authoritative server has received
+a command on the command channel.
+
+% AUTH_RECEIVED_SENDSTATS command 'sendstats' received
+This is a debug message issued when the authoritative server has received
+a command from the statistics module to send it data. The 'sendstats'
+command is handled differently to other commands, which is why the debug
+message associated with it has its own code.
+
+% AUTH_RESPONSE_RECEIVED received response message, ignoring
+This is a debug message, this is output if the authoritative server
+receives a DNS packet with the QR bit set, i.e. a DNS response. The
+server ignores the packet as it only responds to question packets.
+
+% AUTH_SEND_ERROR_RESPONSE sending an error response (%1 bytes):\n%2
+This is a debug message recording that the authoritative server is sending
+an error response to the originator of the query. A previous message will
+have recorded details of the failure.
+
+Note: This message includes the packet sent, rendered in the form of
+multiple lines of text. For this reason, it is suggested that this log message
+not be routed to the syslog file, where the multiple lines could confuse
+programs that expect a format of one message per line.
+
+% AUTH_SEND_NORMAL_RESPONSE sending an error response (%1 bytes):\n%2
+This is a debug message recording that the authoritative server is sending
+a response to the originator of a query.
+
+Note: This message includes the packet sent, rendered in the form of
+multiple lines of text. For this reason, it is suggested that this log message
+not be routed to the syslog file, where the multiple lines could confuse
+programs that expect a format of one message per line.
+
+% AUTH_SERVER_CREATED server created
+An informational message indicating that the authoritative server process has
+been created and is initializing. The AUTH_SERVER_STARTED message will be
+output when initialization has successfully completed and the server starts
+accepting queries.
+
+% AUTH_SERVER_FAILED server failed: %1
+The authoritative server has encountered a fatal error and is terminating. The
+reason for the failure is included in the message.
+
+% AUTH_SERVER_STARTED server stated
+Initialization of the authoritative server has completed successfully
+and it is entering the main loop, waiting for queries to arrive.
+
+% AUTH_SQLITE3 nothing to do for loading sqlite3
+This is a debug message indicating that the authoritative server has
+found that the data source it is loading is an SQLite3 data source,
+so no further validation is needed.
+
+% AUTH_STATS_CHANNEL_CREATED STATS session channel created
+This is a debug message indicating that the authoritative server has
+created a channel to the statistics process.  It is issued during server
+startup is an indication that the initialization is proceeding normally.
+
+% AUTH_STATS_CHANNEL_ESTABLISHED STATS session channel established
+This is a debug message indicating that the authoritative server
+has established communication over the previously created statistics
+channel.  It is issued during server startup is an indication that the
+initialization is proceeding normally.
+
+% AUTH_STATS_COMMS communication error in sending statistics data: %1
+An error was encountered when the authoritiative server tried to send data
+to the statistics daemon. The message includes additional information
+describing the reason for the failure.
+
+% AUTH_STATS_TIMEOUT timeout while sending statistics data: %1
+The authoritative server sent data to the statistics daemon but received
+no acknowledgement within the specified time. The message includes
+additional information describing the reason for the failure.
+
+% AUTH_STATS_TIMER_DISABLED statistics timer has been disabled
+This is a debug message indicating that the statistics timer has been
+disabled in the authoritative server and no statistics information is
+being produced.
+
+% AUTH_STATS_TIMER_SET statistics timer set to %1 second(s)
+This is a debug message indicating that the statistics timer has been
+enabled and that the authoritative server will produce statistics data
+at the specified interval.
+
+% AUTH_UNSUPPORTED_OPCODE unsupported opcode: %1
+This is a debug message, produced when a received DNS packet being
+processed by the authoritative server has been found to contain an
+unsupported opcode. (The opcode is included in the message.) The server
+will return an error code of NOTIMPL to the sender.
+
+% AUTH_XFRIN_CHANNEL_CREATED XFRIN session channel created
+This is a debug message indicating that the authoritative server has
+created a channel to the XFRIN (Transfer-in) process.  It is issued
+during server startup is an indication that the initialization is
+proceeding normally.
+
+% AUTH_XFRIN_CHANNEL_ESTABLISHED XFRIN session channel established
+This is a debug message indicating that the authoritative server has
+established communication over the previously-created channel to the
+XFRIN (Transfer-in) process.  It is issued during server startup is an
+indication that the initialization is proceeding normally.
+
+% AUTH_ZONEMGR_COMMS error communicating with zone manager: %1
+This is a debug message output during the processing of a NOTIFY request.
+An error (listed in the message) has been encountered whilst communicating
+with the zone manager. The NOTIFY request will not be honored.
+
+% AUTH_ZONEMGR_ERROR received error response from zone manager: %1
+This is a debug message output during the processing of a NOTIFY
+request. The zone manager component has been informed of the request,
+but has returned an error response (which is included in the message). The
+NOTIFY request will not be honored.
+
+

src/bin/auth/auth_srv.cc

@@ -59,6 +59,7 @@
 #include <auth/auth_srv.h>
 #include <auth/query.h>
 #include <auth/statistics.h>
+#include <auth/auth_log.h>
 
 using namespace std;
 
@@ -104,7 +105,6 @@ public:
 
     /// These members are public because AuthSrv accesses them directly.
     ModuleCCSession* config_session_;
-    bool verbose_mode_;
     AbstractSession* xfrin_session_;
 
     /// In-memory data source.  Currently class IN only for simplicity.
@@ -143,11 +143,11 @@ private:
 
 AuthSrvImpl::AuthSrvImpl(const bool use_cache,
                          AbstractXfroutClient& xfrout_client) :
-    config_session_(NULL), verbose_mode_(false),
+    config_session_(NULL),
     xfrin_session_(NULL),
     memory_datasrc_class_(RRClass::IN()),
     statistics_timer_(io_service_),
-    counters_(verbose_mode_),
+    counters_(),
     keyring_(NULL),
     xfrout_connected_(false),
     xfrout_client_(xfrout_client)
@@ -251,7 +251,7 @@ public:
 
 void
 makeErrorMessage(MessagePtr message, OutputBufferPtr buffer,
-                 const Rcode& rcode, const bool verbose_mode,
+                 const Rcode& rcode, 
                  std::auto_ptr<TSIGContext> tsig_context =
                  std::auto_ptr<TSIGContext>())
 {
@@ -289,22 +289,9 @@ makeErrorMessage(MessagePtr message, OutputBufferPtr buffer,
     } else {
         message->toWire(renderer);
     }
-
-    if (verbose_mode) {
-        cerr << "[b10-auth] sending an error response (" <<
-            renderer.getLength() << " bytes):\n" << message->toText() << endl;
-    }
-}
-}
-
-void
-AuthSrv::setVerbose(const bool on) {
-    impl_->verbose_mode_ = on;
+    LOG_DEBUG(auth_logger, DBG_AUTH_MESSAGES, AUTH_SEND_ERROR_RESPONSE)
+              .arg(message->toText());
 }
-
-bool
-AuthSrv::getVerbose() const {
-    return (impl_->verbose_mode_);
 }
 
 IOService&
@@ -362,15 +349,12 @@ AuthSrv::setMemoryDataSrc(const isc::dns::RRClass& rrclass,
         isc_throw(InvalidParameter,
                   "Memory data source is not supported for RR class "
                   << rrclass);
-    }
-    if (impl_->verbose_mode_) {
-        if (!impl_->memory_datasrc_ && memory_datasrc) {
-            cerr << "[b10-auth] Memory data source is enabled for class "
-                 << rrclass << endl;
-        } else if (impl_->memory_datasrc_ && !memory_datasrc) {
-            cerr << "[b10-auth] Memory data source is disabled for class "
-                 << rrclass << endl;
-        }
+    } else if (!impl_->memory_datasrc_ && memory_datasrc) {
+        LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_MEM_DATASRC_ENABLED)
+                  .arg(rrclass);
+    } else if (impl_->memory_datasrc_ && !memory_datasrc) {
+        LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_MEM_DATASRC_DISABLED)
+                  .arg(rrclass);
     }
     impl_->memory_datasrc_ = memory_datasrc;
 }
@@ -392,18 +376,13 @@ AuthSrv::setStatisticsTimerInterval(uint32_t interval) {
     }
     if (interval == 0) {
         impl_->statistics_timer_.cancel();
+        LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_STATS_TIMER_DISABLED);
     } else {
         impl_->statistics_timer_.setup(boost::bind(&AuthSrv::submitStatistics,
                                                    this),
                                        interval * 1000);
-    }
-    if (impl_->verbose_mode_) {
-        if (interval == 0) {
-            cerr << "[b10-auth] Disabled statistics timer" << endl;
-        } else {
-            cerr << "[b10-auth] Set statistics timer to " << interval
-                 << " seconds" << endl;
-        }
+        LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_STATS_TIMER_SET)
+                  .arg(interval);
     }
 }
 
@@ -420,17 +399,13 @@ AuthSrv::processMessage(const IOMessage& io_message, MessagePtr message,
 
         // Ignore all responses.
         if (message->getHeaderFlag(Message::HEADERFLAG_QR)) {
-            if (impl_->verbose_mode_) {
-                cerr << "[b10-auth] received unexpected response, ignoring"
-                     << endl;
-            }
+            LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_RESPONSE_RECEIVED);
             server->resume(false);
             return;
         }
     } catch (const Exception& ex) {
-        if (impl_->verbose_mode_) {
-            cerr << "[b10-auth] DNS packet exception: " << ex.what() << endl;
-        }
+        LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_HEADER_PARSE_FAIL)
+                  .arg(ex.what());
         server->resume(false);
         return;
     }
@@ -439,27 +414,21 @@ AuthSrv::processMessage(const IOMessage& io_message, MessagePtr message,
         // Parse the message.
         message->fromWire(request_buffer);
     } catch (const DNSProtocolError& error) {
-        if (impl_->verbose_mode_) {
-            cerr << "[b10-auth] returning " <<  error.getRcode().toText()
-                 << ": " << error.what() << endl;
-        }
-        makeErrorMessage(message, buffer, error.getRcode(),
-                         impl_->verbose_mode_);
+        LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_PACKET_PROTOCOL_ERROR)
+                  .arg(error.getRcode().toText()).arg(error.what());
+        makeErrorMessage(message, buffer, error.getRcode());
         server->resume(true);
         return;
     } catch (const Exception& ex) {
-        if (impl_->verbose_mode_) {
-            cerr << "[b10-auth] returning SERVFAIL: " << ex.what() << endl;
-        }
-        makeErrorMessage(message, buffer, Rcode::SERVFAIL(),
-                         impl_->verbose_mode_);
+        LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_PACKET_PARSE_ERROR)
+                  .arg(ex.what());
+        makeErrorMessage(message, buffer, Rcode::SERVFAIL());
         server->resume(true);
         return;
     } // other exceptions will be handled at a higher layer.
 
-    if (impl_->verbose_mode_) {
-        cerr << "[b10-auth] received a message:\n" << message->toText() << endl;
-    }
+    LOG_DEBUG(auth_logger, DBG_AUTH_MESSAGES, AUTH_PACKET_RECEIVED)
+              .arg(message->toText());
 
     // Perform further protocol-level validation.
     // TSIG first
@@ -481,20 +450,16 @@ AuthSrv::processMessage(const IOMessage& io_message, MessagePtr message,
 
     bool sendAnswer = true;
     if (tsig_error != TSIGError::NOERROR()) {
-        makeErrorMessage(message, buffer, tsig_error.toRcode(),
-                         impl_->verbose_mode_, tsig_context);
+        makeErrorMessage(message, buffer, tsig_error.toRcode(), tsig_context);
     } else if (message->getOpcode() == Opcode::NOTIFY()) {
         sendAnswer = impl_->processNotify(io_message, message, buffer,
                                           tsig_context);
     } else if (message->getOpcode() != Opcode::QUERY()) {
-        if (impl_->verbose_mode_) {
-            cerr << "[b10-auth] unsupported opcode" << endl;
-        }
-        makeErrorMessage(message, buffer, Rcode::NOTIMP(),
-                         impl_->verbose_mode_, tsig_context);
+        LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_UNSUPPORTED_OPCODE)
+                  .arg(message->getOpcode().toText());
+        makeErrorMessage(message, buffer, Rcode::NOTIMP(), tsig_context);
     } else if (message->getRRCount(Message::SECTION_QUESTION) != 1) {
-        makeErrorMessage(message, buffer, Rcode::FORMERR(),
-                         impl_->verbose_mode_, tsig_context);
+        makeErrorMessage(message, buffer, Rcode::FORMERR(), tsig_context);
     } else {
         ConstQuestionPtr question = *message->beginQuestion();
         const RRType &qtype = question->getType();
@@ -502,8 +467,7 @@ AuthSrv::processMessage(const IOMessage& io_message, MessagePtr message,
             sendAnswer = impl_->processAxfrQuery(io_message, message, buffer,
                                                  tsig_context);
         } else if (qtype == RRType::IXFR()) {
-            makeErrorMessage(message, buffer, Rcode::NOTIMP(),
-                             impl_->verbose_mode_, tsig_context);
+            makeErrorMessage(message, buffer, Rcode::NOTIMP(), tsig_context);
         } else {
             sendAnswer = impl_->processNormalQuery(io_message, message, buffer,
                                                    tsig_context);
@@ -550,11 +514,8 @@ AuthSrvImpl::processNormalQuery(const IOMessage& io_message, MessagePtr message,
             data_sources_.doQuery(query);
         }
     } catch (const Exception& ex) {
-        if (verbose_mode_) {
-            cerr << "[b10-auth] Internal error, returning SERVFAIL: " <<
-                ex.what() << endl;
-        }
-        makeErrorMessage(message, buffer, Rcode::SERVFAIL(), verbose_mode_);
+        LOG_ERROR(auth_logger, AUTH_PROCESS_FAIL).arg(ex.what());
+        makeErrorMessage(message, buffer, Rcode::SERVFAIL());
         return (true);
     }
 
@@ -567,12 +528,8 @@ AuthSrvImpl::processNormalQuery(const IOMessage& io_message, MessagePtr message,
     } else {
         message->toWire(renderer);
     }
-
-    if (verbose_mode_) {
-        cerr << "[b10-auth] sending a response ("
-             << renderer.getLength()
-             << " bytes):\n" << message->toText() << endl;
-    }
+    LOG_DEBUG(auth_logger, DBG_AUTH_MESSAGES, AUTH_SEND_NORMAL_RESPONSE)
+              .arg(renderer.getLength()).arg(message->toText());
 
     return (true);
 }
@@ -586,11 +543,8 @@ AuthSrvImpl::processAxfrQuery(const IOMessage& io_message, MessagePtr message,
     incCounter(io_message.getSocket().getProtocol());
 
     if (io_message.getSocket().getProtocol() == IPPROTO_UDP) {
-        if (verbose_mode_) {
-            cerr << "[b10-auth] AXFR query over UDP isn't allowed" << endl;
-        }
-        makeErrorMessage(message, buffer, Rcode::FORMERR(), verbose_mode_,
-                         tsig_context);
+        LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_AXFR_UDP);
+        makeErrorMessage(message, buffer, Rcode::FORMERR(), tsig_context);
         return (true);
     }
 
@@ -613,12 +567,9 @@ AuthSrvImpl::processAxfrQuery(const IOMessage& io_message, MessagePtr message,
             xfrout_connected_ = false;
         }
 
-        if (verbose_mode_) {
-            cerr << "[b10-auth] Error in handling XFR request: " << err.what()
-                 << endl;
-        }
-        makeErrorMessage(message, buffer, Rcode::SERVFAIL(), verbose_mode_,
-                         tsig_context);
+        LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_AXFR_ERROR)
+                  .arg(err.what());
+        makeErrorMessage(message, buffer, Rcode::SERVFAIL(), tsig_context);
         return (true);
     }
 
@@ -633,22 +584,16 @@ AuthSrvImpl::processNotify(const IOMessage& io_message, MessagePtr message,
     // The incoming notify must contain exactly one question for SOA of the
     // zone name.
     if (message->getRRCount(Message::SECTION_QUESTION) != 1) {
-        if (verbose_mode_) {
-                cerr << "[b10-auth] invalid number of questions in notify: "
-                     << message->getRRCount(Message::SECTION_QUESTION) << endl;
-        }
-        makeErrorMessage(message, buffer, Rcode::FORMERR(), verbose_mode_,
-                         tsig_context);
+        LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_NOTIFY_QUESTIONS)
+                  .arg(message->getRRCount(Message::SECTION_QUESTION));
+        makeErrorMessage(message, buffer, Rcode::FORMERR(), tsig_context);
         return (true);
     }
     ConstQuestionPtr question = *message->beginQuestion();
     if (question->getType() != RRType::SOA()) {
-        if (verbose_mode_) {
-                cerr << "[b10-auth] invalid question RR type in notify: "
-                     << question->getType() << endl;
-        }
-        makeErrorMessage(message, buffer, Rcode::FORMERR(), verbose_mode_,
-                         tsig_context);
+        LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_NOTIFY_RRTYPE)
+                  .arg(question->getType().toText());
+        makeErrorMessage(message, buffer, Rcode::FORMERR(), tsig_context);
         return (true);
     }
 
@@ -664,10 +609,7 @@ AuthSrvImpl::processNotify(const IOMessage& io_message, MessagePtr message,
     // silent about such cases, but there doesn't seem to be anything we can
     // improve at the primary server side by sending an error anyway.
     if (xfrin_session_ == NULL) {
-        if (verbose_mode_) {
-            cerr << "[b10-auth] "
-                "session interface for xfrin is not available" << endl;
-        }
+        LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_NO_XFRIN);
         return (false);
     }
 
@@ -693,16 +635,12 @@ AuthSrvImpl::processNotify(const IOMessage& io_message, MessagePtr message,
         int rcode;
         parsed_answer = parseAnswer(rcode, answer);
         if (rcode != 0) {
-            if (verbose_mode_) {
-                cerr << "[b10-auth] failed to notify Zonemgr: "
-                     << parsed_answer->str() << endl;
-            }
+            LOG_ERROR(auth_logger, AUTH_ZONEMGR_ERROR)
+                      .arg(parsed_answer->str());
             return (false);
         }
     } catch (const Exception& ex) {
-        if (verbose_mode_) {
-            cerr << "[b10-auth] failed to notify Zonemgr: " << ex.what() << endl;
-        }
+        LOG_ERROR(auth_logger, AUTH_ZONEMGR_COMMS).arg(ex.what());
         return (false);
     }
 
@@ -762,10 +700,7 @@ AuthSrvImpl::setDbFile(ConstElementPtr config) {
     } else {
         return (answer);
     }
-
-    if (verbose_mode_) {
-        cerr << "[b10-auth] Data source database file: " << db_file_ << endl;
-    }
+    LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_DATA_SOURCE).arg(db_file_);
 
     // create SQL data source
     // Note: the following step is tricky to be exception-safe and to ensure
@@ -795,9 +730,7 @@ AuthSrv::updateConfig(ConstElementPtr new_config) {
         }
         return (impl_->setDbFile(new_config));
     } catch (const isc::Exception& error) {
-        if (impl_->verbose_mode_) {
-            cerr << "[b10-auth] error: " << error.what() << endl;
-        }
+        LOG_ERROR(auth_logger, AUTH_CONFIG_UPDATE_FAIL).arg(error.what());
         return (isc::config::createAnswer(1, error.what()));
     }
 }

src/bin/auth/auth_srv.h

@@ -124,27 +124,6 @@ public:
                         isc::util::OutputBufferPtr buffer,
                         isc::asiodns::DNSServer* server);
 
-    /// \brief Set verbose flag
-    ///
-    /// \param on The new value of the verbose flag
-
-    /// \brief Enable or disable verbose logging.
-    ///
-    /// This method never throws an exception.
-    ///
-    /// \param on \c true to enable verbose logging; \c false to disable
-    /// verbose logging.
-    void setVerbose(const bool on);
-
-    /// \brief Returns the logging verbosity of the \c AuthSrv object.
-    ///
-    /// This method never throws an exception.
-    ///
-    /// \return \c true if verbose logging is enabled; otherwise \c false.
-
-    /// \brief Get the current value of the verbose flag
-    bool getVerbose() const;
-
     /// \brief Updates the data source for the \c AuthSrv object.
     ///
     /// This method installs or replaces the data source that the \c AuthSrv

src/bin/auth/benchmarks/Makefile.am

@@ -12,6 +12,9 @@ query_bench_SOURCES += ../query.h  ../query.cc
 query_bench_SOURCES += ../auth_srv.h ../auth_srv.cc
 query_bench_SOURCES += ../auth_config.h ../auth_config.cc
 query_bench_SOURCES += ../statistics.h ../statistics.cc
+query_bench_SOURCES += ../auth_log.h ../auth_log.cc
+
+nodist_query_bench_SOURCES = ../auth_messages.h ../auth_messages.cc
 
 query_bench_LDADD = $(top_builddir)/src/lib/dns/libdns++.la
 query_bench_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la

src/bin/auth/command.cc

@@ -27,16 +27,18 @@
 
 #include <config/ccsession.h>
 
+#include <auth/auth_log.h>
 #include <auth/auth_srv.h>
 #include <auth/command.h>
 
-using namespace std;
-using boost::shared_ptr;
 using boost::scoped_ptr;
-using namespace isc::dns;
+using boost::shared_ptr;
+using namespace isc::auth;
+using namespace isc::config;
 using namespace isc::data;
 using namespace isc::datasrc;
-using namespace isc::config;
+using namespace isc::dns;
+using namespace std;
 
 namespace {
 /// An exception that is thrown if an error occurs while handling a command
@@ -115,9 +117,7 @@ public:
 class SendStatsCommand : public AuthCommand {
 public:
     virtual void exec(AuthSrv& server, isc::data::ConstElementPtr) {
-        if (server.getVerbose()) {
-            cerr << "[b10-auth] command 'sendstats' received" << endl;
-        }
+        LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_RECEIVED_SENDSTATS);
         server.submitStatistics();
     }
 };
@@ -140,11 +140,8 @@ public:
                                                       oldzone->getOrigin()));
         newzone->load(oldzone->getFileName());
         oldzone->swap(*newzone);
-
-        if (server.getVerbose()) {
-            cerr << "[b10-auth] Loaded zone '" << newzone->getOrigin()
-                 << "'/" << newzone->getClass() << endl;
-        }
+        LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_LOAD_ZONE)
+                  .arg(newzone->getOrigin()).arg(newzone->getClass());
     }
 
 private:
@@ -164,10 +161,7 @@ private:
         ConstElementPtr datasrc_elem = args->get("datasrc");
         if (datasrc_elem) {
             if (datasrc_elem->stringValue() == "sqlite3") {
-                if (server.getVerbose()) {
-                    cerr << "[b10-auth] Nothing to do for loading sqlite3"
-                         << endl;
-                }
+                LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_SQLITE3);
                 return (false);
             } else if (datasrc_elem->stringValue() != "memory") {
                 // (note: at this point it's guaranteed that datasrc_elem
@@ -233,18 +227,13 @@ ConstElementPtr
 execAuthServerCommand(AuthSrv& server, const string& command_id,
                       ConstElementPtr args)
 {
-    if (server.getVerbose()) {
-        cerr << "[b10-auth] Received '" << command_id << "' command" << endl;
-    }
-
+    LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_RECEIVED_COMMAND).arg(command_id);
     try {
         scoped_ptr<AuthCommand>(createAuthCommand(command_id))->exec(server,
                                                                      args);
     } catch (const isc::Exception& ex) {
-        if (server.getVerbose()) {
-            cerr << "[b10-auth] Command '" << command_id
-                 << "' execution failed: " << ex.what() << endl;
-        }
+        LOG_ERROR(auth_logger, AUTH_COMMAND_FAILED).arg(command_id)
+                                                   .arg(ex.what());
         return (createAnswer(1, ex.what()));
     }
 

src/bin/auth/main.cc

@@ -44,25 +44,26 @@
 #include <auth/command.h>
 #include <auth/change_user.h>
 #include <auth/auth_srv.h>
+#include <auth/auth_log.h>
 #include <asiodns/asiodns.h>
 #include <asiolink/asiolink.h>
-#include <log/dummylog.h>
+#include <log/logger_support.h>
 #include <server_common/keyring.h>
 
 using namespace std;
-using namespace isc::data;
+using namespace isc::asiodns;
+using namespace isc::asiolink;
+using namespace isc::auth;
 using namespace isc::cc;
 using namespace isc::config;
+using namespace isc::data;
 using namespace isc::dns;
+using namespace isc::log;
 using namespace isc::util;
 using namespace isc::xfr;
-using namespace isc::asiolink;
-using namespace isc::asiodns;
 
 namespace {
 
-bool verbose_mode = false;
-
 /* need global var for config/command handlers.
  * todo: turn this around, and put handlers in the authserver
  * class itself? */
@@ -88,6 +89,7 @@ usage() {
     cerr << "\t-v: verbose output" << endl;
     exit(1);
 }
+
 } // end of anonymous namespace
 
 int
@@ -95,6 +97,7 @@ main(int argc, char* argv[]) {
     int ch;
     const char* uid = NULL;
     bool cache = true;
+    bool verbose = false;
 
     while ((ch = getopt(argc, argv, ":nu:v")) != -1) {
         switch (ch) {
@@ -105,8 +108,7 @@ main(int argc, char* argv[]) {
             uid = optarg;
             break;
         case 'v':
-            verbose_mode = true;
-            isc::log::denabled = true;
+            verbose = true;
             break;
         case '?':
         default:
@@ -118,6 +120,11 @@ main(int argc, char* argv[]) {
         usage();
     }
 
+    // Initialize logging.  If verbose, we'll use maximum verbosity.
+    isc::log::initLogger("b10-auth",
+                         (verbose ? isc::log::DEBUG : isc::log::INFO),
+                         isc::log::MAX_DEBUG_LEVEL, NULL);
+
     int ret = 0;
 
     // XXX: we should eventually pass io_service here.
@@ -138,8 +145,7 @@ main(int argc, char* argv[]) {
         }
 
         auth_server = new AuthSrv(cache, xfrout_client);
-        auth_server->setVerbose(verbose_mode);
-        cout << "[b10-auth] Server created." << endl;
+        LOG_INFO(auth_logger, AUTH_SERVER_CREATED);
 
         SimpleCallback* checkin = auth_server->getCheckinProvider();
         IOService& io_service = auth_server->getIOService();
@@ -148,10 +154,10 @@ main(int argc, char* argv[]) {
 
         DNSService dns_service(io_service, checkin, lookup, answer);
         auth_server->setDNSService(dns_service);
-        cout << "[b10-auth] DNSServices created." << endl;
+        LOG_DEBUG(auth_logger, DBG_AUTH_START, AUTH_DNS_SERVICES_CREATED);
 
         cc_session = new Session(io_service.get_io_service());
-        cout << "[b10-auth] Configuration session channel created." << endl;
+        LOG_DEBUG(auth_logger, DBG_AUTH_START, AUTH_CONFIG_CHANNEL_CREATED);
 
         // We delay starting listening to new commands/config just before we
         // go into the main loop to avoid confusion due to mixture of
@@ -161,19 +167,19 @@ main(int argc, char* argv[]) {
         config_session = new ModuleCCSession(specfile, *cc_session,
                                              my_config_handler,
                                              my_command_handler, false);
-        cout << "[b10-auth] Configuration channel established." << endl;
+        LOG_DEBUG(auth_logger, DBG_AUTH_START, AUTH_CONFIG_CHANNEL_ESTABLISHED);
 
         xfrin_session = new Session(io_service.get_io_service());
-        cout << "[b10-auth] Xfrin session channel created." << endl;
+        LOG_DEBUG(auth_logger, DBG_AUTH_START, AUTH_XFRIN_CHANNEL_CREATED);
         xfrin_session->establish(NULL);
         xfrin_session_established = true;
-        cout << "[b10-auth] Xfrin session channel established." << endl;
+        LOG_DEBUG(auth_logger, DBG_AUTH_START, AUTH_XFRIN_CHANNEL_ESTABLISHED);
 
         statistics_session = new Session(io_service.get_io_service());
-        cout << "[b10-auth] Statistics session channel created." << endl;
+        LOG_DEBUG(auth_logger, DBG_AUTH_START, AUTH_STATS_CHANNEL_CREATED);
         statistics_session->establish(NULL);
         statistics_session_established = true;
-        cout << "[b10-auth] Statistics session channel established." << endl;
+        LOG_DEBUG(auth_logger, DBG_AUTH_START, AUTH_STATS_CHANNEL_ESTABLISHED);
 
         auth_server->setXfrinSession(xfrin_session);
         auth_server->setStatisticsSession(statistics_session);
@@ -182,33 +188,34 @@ main(int argc, char* argv[]) {
         // all initial configurations, but as a short term workaround we
         // handle the traditional "database_file" setup by directly calling
         // updateConfig().
-        // if server load configure failed, we won't exit, give user second chance
-        // to correct the configure.
+        // if server load configure failed, we won't exit, give user second
+        // chance to correct the configure.
         auth_server->setConfigSession(config_session);
         try {
             configureAuthServer(*auth_server, config_session->getFullConfig());
             auth_server->updateConfig(ElementPtr());
         } catch (const AuthConfigError& ex) {
-            cout << "[bin10-auth] Server load config failed:" << ex.what() << endl;
+            LOG_ERROR(auth_logger, AUTH_CONFIG_LOAD_FAIL).arg(ex.what());
         }
 
         if (uid != NULL) {
             changeUser(uid);
         }
 
-        cout << "[b10-auth] Loading TSIG keys" << endl;
+        LOG_DEBUG(auth_logger, DBG_AUTH_START, AUTH_LOAD_TSIG);
         isc::server_common::initKeyring(*config_session);
         auth_server->setTSIGKeyRing(&isc::server_common::keyring);
 
         // Now start asynchronous read.
         config_session->start();
-        cout << "[b10-auth] Configuration channel started." << endl;
+        LOG_DEBUG(auth_logger, DBG_AUTH_START, AUTH_CONFIG_CHANNEL_STARTED);
 
-        cout << "[b10-auth] Server started." << endl;
+        // Successfully initialized.
+        LOG_INFO(auth_logger, AUTH_SERVER_STARTED);
         io_service.run();
 
     } catch (const std::exception& ex) {
-        cerr << "[b10-auth] Server failed: " << ex.what() << endl;
+        LOG_FATAL(auth_logger, AUTH_SERVER_FAILED).arg(ex.what());
         ret = 1;
     }
 

src/bin/auth/statistics.cc

@@ -13,6 +13,7 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <auth/statistics.h>
+#include <auth/auth_log.h>
 
 #include <cc/data.h>
 #include <cc/session.h>
@@ -20,6 +21,8 @@
 #include <sstream>
 #include <iostream>
 
+using namespace isc::auth;
+
 // TODO: We need a namespace ("auth_server"?) to hold
 // AuthSrv and AuthCounters.
 
@@ -29,10 +32,7 @@ private:
     AuthCountersImpl(const AuthCountersImpl& source);
     AuthCountersImpl& operator=(const AuthCountersImpl& source);
 public:
-    // References verbose_mode flag in AuthSrvImpl
-    // TODO: Fix this short term workaround for logging
-    // after we have logging framework
-    AuthCountersImpl(const bool& verbose_mode);
+    AuthCountersImpl();
     ~AuthCountersImpl();
     void inc(const AuthCounters::CounterType type);
     bool submitStatistics() const;
@@ -42,15 +42,13 @@ public:
 private:
     std::vector<uint64_t> counters_;
     isc::cc::AbstractSession* statistics_session_;
-    const bool& verbose_mode_;
 };
 
-AuthCountersImpl::AuthCountersImpl(const bool& verbose_mode) :
+AuthCountersImpl::AuthCountersImpl() :
     // initialize counter
     // size: AuthCounters::COUNTER_TYPES, initial value: 0
     counters_(AuthCounters::COUNTER_TYPES, 0),
-    statistics_session_(NULL),
-    verbose_mode_(verbose_mode)
+    statistics_session_(NULL)
 {}
 
 AuthCountersImpl::~AuthCountersImpl()
@@ -64,11 +62,7 @@ AuthCountersImpl::inc(const AuthCounters::CounterType type) {
 bool
 AuthCountersImpl::submitStatistics() const {
     if (statistics_session_ == NULL) {
-        if (verbose_mode_) {
-            std::cerr << "[b10-auth] "
-                      << "session interface for statistics"
-                      << " is not available" << std::endl;
-        }
+        LOG_ERROR(auth_logger, AUTH_NO_STATS_SESSION);
         return (false);
     }
     std::stringstream statistics_string;
@@ -95,18 +89,10 @@ AuthCountersImpl::submitStatistics() const {
         // currently it just returns empty message
         statistics_session_->group_recvmsg(env, answer, false, seq);
     } catch (const isc::cc::SessionError& ex) {
-        if (verbose_mode_) {
-            std::cerr << "[b10-auth] "
-                      << "communication error in sending statistics data: "
-                      << ex.what() << std::endl;
-        }
+        LOG_ERROR(auth_logger, AUTH_STATS_COMMS).arg(ex.what());
         return (false);
     } catch (const isc::cc::SessionTimeout& ex) {
-        if (verbose_mode_) {
-            std::cerr << "[b10-auth] "
-                      << "timeout happened while sending statistics data: "
-                      << ex.what() << std::endl;
-        }
+        LOG_ERROR(auth_logger, AUTH_STATS_TIMEOUT).arg(ex.what());
         return (false);
     }
     return (true);
@@ -125,8 +111,7 @@ AuthCountersImpl::getCounter(const AuthCounters::CounterType type) const {
     return (counters_.at(type));
 }
 
-AuthCounters::AuthCounters(const bool& verbose_mode) :
-    impl_(new AuthCountersImpl(verbose_mode))
+AuthCounters::AuthCounters() : impl_(new AuthCountersImpl())
 {}
 
 AuthCounters::~AuthCounters() {

src/bin/auth/statistics.h

@@ -61,15 +61,10 @@ public:
     };
     /// The constructor.
     ///
-    /// \param verbose_mode reference to verbose_mode_ of AuthSrvImpl
-    ///
     /// This constructor is mostly exception free. But it may still throw
     /// a standard exception if memory allocation fails inside the method.
     ///
-    /// \todo Fix this short term workaround for logging
-    /// after we have logging framework.
-    ///
-    AuthCounters(const bool& verbose_mode);
+    AuthCounters();
     /// The destructor.
     ///
     /// This method never throws an exception.

src/bin/auth/tests/Makefile.am

@@ -22,6 +22,7 @@ TESTS += run_unittests
 run_unittests_SOURCES = $(top_srcdir)/src/lib/dns/tests/unittest_util.h
 run_unittests_SOURCES += $(top_srcdir)/src/lib/dns/tests/unittest_util.cc
 run_unittests_SOURCES += ../auth_srv.h ../auth_srv.cc
+run_unittests_SOURCES += ../auth_log.h ../auth_log.cc
 run_unittests_SOURCES += ../query.h ../query.cc
 run_unittests_SOURCES += ../change_user.h ../change_user.cc
 run_unittests_SOURCES += ../auth_config.h ../auth_config.cc
@@ -36,6 +37,9 @@ run_unittests_SOURCES += query_unittest.cc
 run_unittests_SOURCES += change_user_unittest.cc
 run_unittests_SOURCES += statistics_unittest.cc
 run_unittests_SOURCES += run_unittests.cc
+
+nodist_run_unittests_SOURCES = ../auth_messages.h ../auth_messages.cc
+
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)
 run_unittests_LDADD = $(GTEST_LDADD)

src/bin/auth/tests/auth_srv_unittest.cc

@@ -190,15 +190,6 @@ TEST_F(AuthSrvTest, unsupportedRequest) {
     unsupportedRequest();
 }
 
-// Simple API check
-TEST_F(AuthSrvTest, verbose) {
-    EXPECT_FALSE(server.getVerbose());
-    server.setVerbose(true);
-    EXPECT_TRUE(server.getVerbose());
-    server.setVerbose(false);
-    EXPECT_FALSE(server.getVerbose());
-}
-
 // Multiple questions.  Should result in FORMERR.
 TEST_F(AuthSrvTest, multiQuestion) {
     multiQuestion();

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/auth/tests/statistics_unittest.cc

@@ -69,13 +69,12 @@ private:
     };
 
 protected:
-    AuthCountersTest() : verbose_mode_(false), counters(verbose_mode_) {
+    AuthCountersTest() : counters() {
         counters.setStatisticsSession(&statistics_session_);
     }
     ~AuthCountersTest() {
     }
     MockSession statistics_session_;
-    bool verbose_mode_;
     AuthCounters counters;
 };
 

src/bin/bind10/Makefile.am

@@ -27,3 +27,8 @@ bind10: bind10.py
 
 pytest:
 	$(SHELL) tests/bind10_test
+
+CLEANDIRS = __pycache__
+
+clean-local:
+	rm -rf $(CLEANDIRS)

src/bin/bind10/run_bind10.sh.in

@@ -30,7 +30,7 @@ export PYTHONPATH
 # required by loadable python modules.
 SET_ENV_LIBRARY_PATH=@SET_ENV_LIBRARY_PATH@
 if test $SET_ENV_LIBRARY_PATH = yes; then
-	@ENV_LIBRARY_PATH@=@abs_top_builddir@/src/lib/dns/.libs:@abs_top_builddir@/src/lib/cryptolink/.libs:@abs_top_builddir@/src/lib/util/.libs:@abs_top_builddir@/src/lib/util/io/.libs:@abs_top_builddir@/src/lib/exceptions/.libs:$@ENV_LIBRARY_PATH@
+	@ENV_LIBRARY_PATH@=@abs_top_builddir@/src/lib/dns/.libs:@abs_top_builddir@/src/lib/cryptolink/.libs:@abs_top_builddir@/src/lib/cc/.libs:@abs_top_builddir@/src/lib/config/.libs:@abs_top_builddir@/src/lib/log/.libs:@abs_top_builddir@/src/lib/util/.libs:@abs_top_builddir@/src/lib/util/io/.libs:@abs_top_builddir@/src/lib/exceptions/.libs:$@ENV_LIBRARY_PATH@
 	export @ENV_LIBRARY_PATH@
 fi
 

src/bin/bind10/tests/Makefile.am

@@ -4,6 +4,13 @@ PYCOVERAGE_RUN = @PYCOVERAGE_RUN@
 PYTESTS = bind10_test.py
 EXTRA_DIST = $(PYTESTS)
 
+# If necessary (rare cases), explicitly specify paths to dynamic libraries
+# required by loadable python modules.
+LIBRARY_PATH_PLACEHOLDER =
+if SET_ENV_LIBRARY_PATH
+LIBRARY_PATH_PLACEHOLDER += $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/cc/.libs:$(abs_top_builddir)/src/lib/config/.libs:$(abs_top_builddir)/src/lib/log/.libs:$(abs_top_builddir)/src/lib/util/.libs:$(abs_top_builddir)/src/lib/exceptions/.libs:$$$(ENV_LIBRARY_PATH)
+endif
+
 # test using command-line arguments, so use check-local target instead of TESTS
 check-local:
 if ENABLE_PYTHON_COVERAGE
@@ -13,6 +20,7 @@ if ENABLE_PYTHON_COVERAGE
 endif
 	for pytest in $(PYTESTS) ; do \
 	echo Running test: $$pytest ; \
+	$(LIBRARY_PATH_PLACEHOLDER) \
 	env PYTHONPATH=$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python:$(abs_top_builddir)/src/bin/bind10 \
 	BIND10_MSGQ_SOCKET_FILE=$(abs_top_builddir)/msgq_socket \
 		$(PYCOVERAGE_RUN) $(abs_builddir)/$$pytest || exit ; \

src/bin/bindctl/Makefile.am

@@ -25,3 +25,8 @@ bindctl: bindctl_main.py
 	       -e "s|@@SYSCONFDIR@@|@sysconfdir@|" \
 	       -e "s|@@LIBEXECDIR@@|$(pkglibexecdir)|" bindctl_main.py >$@
 	chmod a+x $@
+
+CLEANDIRS = __pycache__
+
+clean-local:
+	rm -rf $(CLEANDIRS)

src/bin/bindctl/run_bindctl.sh.in

@@ -23,6 +23,14 @@ BINDCTL_PATH=@abs_top_builddir@/src/bin/bindctl
 PYTHONPATH=@abs_top_srcdir@/src/bin:@abs_top_builddir@/src/lib/python:@abs_top_builddir@/src/bin:@abs_top_srcdir@/src/lib/python
 export PYTHONPATH
 
+# If necessary (rare cases), explicitly specify paths to dynamic libraries
+# required by loadable python modules.
+SET_ENV_LIBRARY_PATH=@SET_ENV_LIBRARY_PATH@
+if test $SET_ENV_LIBRARY_PATH = yes; then
+	@ENV_LIBRARY_PATH@=@abs_top_builddir@/src/lib/dns/.libs:@abs_top_builddir@/src/lib/cryptolink/.libs:@abs_top_builddir@/src/lib/cc/.libs:@abs_top_builddir@/src/lib/config/.libs:@abs_top_builddir@/src/lib/log/.libs:@abs_top_builddir@/src/lib/util/.libs:@abs_top_builddir@/src/lib/util/io/.libs:@abs_top_builddir@/src/lib/exceptions/.libs:$@ENV_LIBRARY_PATH@
+	export @ENV_LIBRARY_PATH@
+fi
+
 B10_FROM_SOURCE=@abs_top_srcdir@
 export B10_FROM_SOURCE
 

src/bin/bindctl/tests/Makefile.am

@@ -2,6 +2,13 @@ PYCOVERAGE_RUN = @PYCOVERAGE_RUN@
 PYTESTS = bindctl_test.py cmdparse_test.py
 EXTRA_DIST = $(PYTESTS)
 
+# If necessary (rare cases), explicitly specify paths to dynamic libraries
+# required by loadable python modules.
+LIBRARY_PATH_PLACEHOLDER =
+if SET_ENV_LIBRARY_PATH
+LIBRARY_PATH_PLACEHOLDER += $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/cc/.libs:$(abs_top_builddir)/src/lib/config/.libs:$(abs_top_builddir)/src/lib/log/.libs:$(abs_top_builddir)/src/lib/util/.libs:$(abs_top_builddir)/src/lib/exceptions/.libs:$$$(ENV_LIBRARY_PATH)
+endif
+
 # test using command-line arguments, so use check-local target instead of TESTS
 check-local:
 if ENABLE_PYTHON_COVERAGE
@@ -11,6 +18,7 @@ if ENABLE_PYTHON_COVERAGE
 endif
 	for pytest in $(PYTESTS) ; do \
 	echo Running test: $$pytest ; \
+	$(LIBRARY_PATH_PLACEHOLDER) \
 	env PYTHONPATH=$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python:$(abs_top_builddir)/src/bin/bindctl:$(abs_top_srcdir)/src/bin  \
 	$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
 	done

src/bin/cfgmgr/Makefile.am

@@ -28,3 +28,8 @@ install-data-local:
 	$(mkinstalldirs) $(DESTDIR)/@localstatedir@/@PACKAGE@
 # TODO: permissions handled later
 
+
+CLEANDIRS = __pycache__
+
+clean-local:
+	rm -rf $(CLEANDIRS)

src/bin/cfgmgr/plugins/Makefile.am

@@ -4,3 +4,9 @@ EXTRA_DIST += logging.spec b10logging.py
 
 config_plugindir = @prefix@/share/@PACKAGE@/config_plugins
 config_plugin_DATA = tsig_keys.py tsig_keys.spec
+config_plugin_DATA += b10logging.py logging.spec
+
+CLEANDIRS = __pycache__
+
+clean-local:
+	rm -rf $(CLEANDIRS)

src/bin/cfgmgr/plugins/tests/Makefile.am

@@ -7,7 +7,7 @@ EXTRA_DIST = $(PYTESTS)
 # required by loadable python modules.
 LIBRARY_PATH_PLACEHOLDER =
 if SET_ENV_LIBRARY_PATH
-LIBRARY_PATH_PLACEHOLDER += $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/dns/.libs:$(abs_top_builddir)/src/lib/cryptolink/.libs:$(abs_top_builddir)/src/lib/util/.libs:$(abs_top_builddir)/src/lib/exceptions/.libs
+LIBRARY_PATH_PLACEHOLDER += $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/cc/.libs:$(abs_top_builddir)/src/lib/config/.libs:$(abs_top_builddir)/src/lib/log/.libs:$(abs_top_builddir)/src/lib/dns/.libs:$(abs_top_builddir)/src/lib/cryptolink/.libs:$(abs_top_builddir)/src/lib/util/.libs:$(abs_top_builddir)/src/lib/exceptions/.libs
 endif
 
 # test using command-line arguments, so use check-local target instead of TESTS

src/bin/cfgmgr/tests/Makefile.am

@@ -3,6 +3,13 @@ PYTESTS = b10-cfgmgr_test.py
 
 EXTRA_DIST = $(PYTESTS) testdata/plugins/testplugin.py
 
+# If necessary (rare cases), explicitly specify paths to dynamic libraries
+# required by loadable python modules.
+LIBRARY_PATH_PLACEHOLDER =
+if SET_ENV_LIBRARY_PATH
+LIBRARY_PATH_PLACEHOLDER += $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/cc/.libs:$(abs_top_builddir)/src/lib/config/.libs:$(abs_top_builddir)/src/lib/log/.libs:$(abs_top_builddir)/src/lib/util/.libs:$(abs_top_builddir)/src/lib/exceptions/.libs:$$$(ENV_LIBRARY_PATH)
+endif
+
 # test using command-line arguments, so use check-local target instead of TESTS
 check-local:
 if ENABLE_PYTHON_COVERAGE
@@ -13,6 +20,12 @@ endif
 	for pytest in $(PYTESTS) ; do \
 	echo Running test: $$pytest ; \
 	env TESTDATA_PATH=$(abs_srcdir)/testdata \
+	$(LIBRARY_PATH_PLACEHOLDER) \
 	env PYTHONPATH=$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python:$(abs_top_builddir)/src/bin/cfgmgr \
 	$(PYCOVERAGE_RUN) $(abs_builddir)/$$pytest || exit ; \
 	done
+
+CLEANDIRS = testdata/plugins/__pycache__
+
+clean-local:
+	rm -rf $(CLEANDIRS)

src/bin/cmdctl/Makefile.am

@@ -51,3 +51,8 @@ install-data-local:
 	done
 
 endif
+
+CLEANDIRS = __pycache__
+
+clean-local:
+	rm -rf $(CLEANDIRS)

src/bin/cmdctl/tests/Makefile.am

@@ -2,6 +2,13 @@ PYCOVERAGE_RUN=@PYCOVERAGE_RUN@
 PYTESTS = cmdctl_test.py
 EXTRA_DIST = $(PYTESTS)
 
+# If necessary (rare cases), explicitly specify paths to dynamic libraries
+# required by loadable python modules.
+LIBRARY_PATH_PLACEHOLDER =
+if SET_ENV_LIBRARY_PATH
+LIBRARY_PATH_PLACEHOLDER += $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/cc/.libs:$(abs_top_builddir)/src/lib/config/.libs:$(abs_top_builddir)/src/lib/log/.libs:$(abs_top_builddir)/src/lib/util/.libs:$(abs_top_builddir)/src/lib/exceptions/.libs:$$$(ENV_LIBRARY_PATH)
+endif
+
 # test using command-line arguments, so use check-local target instead of TESTS
 check-local:
 if ENABLE_PYTHON_COVERAGE
@@ -11,6 +18,7 @@ if ENABLE_PYTHON_COVERAGE
 endif
 	for pytest in $(PYTESTS) ; do \
 	echo Running test: $$pytest ; \
+	$(LIBRARY_PATH_PLACEHOLDER) \
 	env PYTHONPATH=$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python:$(abs_top_builddir)/src/bin/cmdctl \
 	CMDCTL_SPEC_PATH=$(abs_top_builddir)/src/bin/cmdctl \
 	CMDCTL_SRC_PATH=$(abs_top_srcdir)/src/bin/cmdctl \

src/bin/loadzone/run_loadzone.sh.in

@@ -21,6 +21,14 @@ export PYTHON_EXEC
 PYTHONPATH=@abs_top_builddir@/src/lib/python
 export PYTHONPATH
 
+# If necessary (rare cases), explicitly specify paths to dynamic libraries
+# required by loadable python modules.
+SET_ENV_LIBRARY_PATH=@SET_ENV_LIBRARY_PATH@
+if test $SET_ENV_LIBRARY_PATH = yes; then
+	@ENV_LIBRARY_PATH@=@abs_top_builddir@/src/lib/dns/.libs:@abs_top_builddir@/src/lib/cryptolink/.libs:@abs_top_builddir@/src/lib/cc/.libs:@abs_top_builddir@/src/lib/config/.libs:@abs_top_builddir@/src/lib/log/.libs:@abs_top_builddir@/src/lib/util/.libs:@abs_top_builddir@/src/lib/util/io/.libs:@abs_top_builddir@/src/lib/exceptions/.libs:$@ENV_LIBRARY_PATH@
+	export @ENV_LIBRARY_PATH@
+fi
+
 BIND10_MSGQ_SOCKET_FILE=@abs_top_builddir@/msgq_socket
 export BIND10_MSGQ_SOCKET_FILE
 

src/bin/loadzone/tests/correct/Makefile.am

@@ -13,8 +13,15 @@ EXTRA_DIST += ttl2.db
 EXTRA_DIST += ttlext.db
 EXTRA_DIST += example.db
 
+# If necessary (rare cases), explicitly specify paths to dynamic libraries
+# required by loadable python modules.
+LIBRARY_PATH_PLACEHOLDER =
+if SET_ENV_LIBRARY_PATH
+LIBRARY_PATH_PLACEHOLDER += $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/cc/.libs:$(abs_top_builddir)/src/lib/config/.libs:$(abs_top_builddir)/src/lib/log/.libs:$(abs_top_builddir)/src/lib/util/.libs:$(abs_top_builddir)/src/lib/exceptions/.libs:$$$(ENV_LIBRARY_PATH)
+endif
+
 # TODO: maybe use TESTS?
 # test using command-line arguments, so use check-local target instead of TESTS
 check-local:
 	echo Running test: correct_test.sh 
-	$(SHELL) $(abs_builddir)/correct_test.sh
+	$(LIBRARY_PATH_PLACEHOLDER) $(SHELL) $(abs_builddir)/correct_test.sh

src/bin/loadzone/tests/error/Makefile.am

@@ -12,8 +12,15 @@ EXTRA_DIST += keyerror3.db
 EXTRA_DIST += originerr1.db
 EXTRA_DIST += originerr2.db
 
+# If necessary (rare cases), explicitly specify paths to dynamic libraries
+# required by loadable python modules.
+LIBRARY_PATH_PLACEHOLDER =
+if SET_ENV_LIBRARY_PATH
+LIBRARY_PATH_PLACEHOLDER += $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/cc/.libs:$(abs_top_builddir)/src/lib/config/.libs:$(abs_top_builddir)/src/lib/log/.libs:$(abs_top_builddir)/src/lib/util/.libs:$(abs_top_builddir)/src/lib/exceptions/.libs:$$$(ENV_LIBRARY_PATH)
+endif
+
 # TODO: use TESTS ?
 # test using command-line arguments, so use check-local target instead of TESTS
 check-local:
 	echo Running test: error_test.sh
-	$(SHELL) $(abs_builddir)/error_test.sh
+	$(LIBRARY_PATH_PLACEHOLDER) $(SHELL) $(abs_builddir)/error_test.sh

src/bin/msgq/Makefile.am

@@ -20,3 +20,8 @@ endif
 b10-msgq: msgq.py
 	$(SED) "s|@@PYTHONPATH@@|@pyexecdir@|" msgq.py >$@
 	chmod a+x $@
+
+CLEANDIRS = __pycache__
+
+clean-local:
+	rm -rf $(CLEANDIRS)

src/bin/msgq/tests/Makefile.am

@@ -2,6 +2,13 @@ PYCOVERAGE_RUN = @PYCOVERAGE_RUN@
 PYTESTS = msgq_test.py
 EXTRA_DIST = $(PYTESTS)
 
+# If necessary (rare cases), explicitly specify paths to dynamic libraries
+# required by loadable python modules.
+LIBRARY_PATH_PLACEHOLDER =
+if SET_ENV_LIBRARY_PATH
+LIBRARY_PATH_PLACEHOLDER += $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/cc/.libs:$(abs_top_builddir)/src/lib/config/.libs:$(abs_top_builddir)/src/lib/log/.libs:$(abs_top_builddir)/src/lib/util/.libs:$(abs_top_builddir)/src/lib/exceptions/.libs:$$$(ENV_LIBRARY_PATH)
+endif
+
 # test using command-line arguments, so use check-local target instead of TESTS
 check-local:
 if ENABLE_PYTHON_COVERAGE
@@ -11,6 +18,7 @@ if ENABLE_PYTHON_COVERAGE
 endif
 	for pytest in $(PYTESTS) ; do \
 	echo Running test: $$pytest ; \
+	$(LIBRARY_PATH_PLACEHOLDER) \
 	env PYTHONPATH=$(abs_top_builddir)/src/bin/msgq:$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python \
 	BIND10_TEST_SOCKET_FILE=$(builddir)/test_msgq_socket.sock \
 	$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \

src/bin/resolver/resolverdef.mes

@@ -118,7 +118,7 @@ of the body of the message failed due to some non-protocol related reason
 (although the parsing of the header succeeded).  The message parameters give
 a textual description of the problem and the RCODE returned.
 
-% PRINTMSG      print message command, aeguments are: %1
+% PRINTMSG      print message command, arguments are: %1
 This message is logged when a "print_message" command is received over the
 command channel.
 
@@ -169,7 +169,7 @@ resolver gives up trying to resolve a query.  Retry count: the number of times
 the resolver will retry a query to an upstream server if it gets a timeout.
 
 The client and lookup timeouts require a bit more explanation. The
-resolution of the clent query might require a large number of queries to
+resolution of the client query might require a large number of queries to
 upstream nameservers.  Even if none of these queries timeout, the total time
 taken to perform all the queries may exceed the client timeout.  When this
 happens, a SERVFAIL is returned to the client, but the resolver continues

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/stats/Makefile.am

@@ -34,3 +34,8 @@ b10-stats: stats.py
 b10-stats-httpd: stats_httpd.py
 	$(SED) -e "s|@@PYTHONPATH@@|@pyexecdir@|" stats_httpd.py >$@
 	chmod a+x $@
+
+CLEANDIRS = __pycache__
+
+clean-local:
+	rm -rf $(CLEANDIRS)

src/bin/stats/stats_httpd.py.in

@@ -437,8 +437,15 @@ class StatsHttpd:
             (k, v) = (str(k), str(v))
             elem = xml.etree.ElementTree.Element(k)
             elem.text = v
+            # The coding conversion is tricky. xml..tostring() of Python 3.2
+            # returns bytes (not string) regardless of the coding, while
+            # tostring() of Python 3.1 returns a string.  To support both
+            # cases transparently, we first make sure tostring() returns
+            # bytes by specifying utf-8 and then convert the result to a
+            # plain string (code below assume it).
             xml_list.append(
-                xml.etree.ElementTree.tostring(elem))
+                str(xml.etree.ElementTree.tostring(elem, encoding='utf-8'),
+                    encoding='us-ascii'))
         xml_string = "".join(xml_list)
         self.xml_body = self.open_template(XML_TEMPLATE_LOCATION).substitute(
             xml_string=xml_string,

src/bin/stats/tests/Makefile.am

@@ -17,3 +17,8 @@ endif
 	B10_FROM_SOURCE=$(abs_top_srcdir) \
 	$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
 	done
+
+CLEANDIRS = __pycache__
+
+clean-local:
+	rm -rf $(CLEANDIRS)

src/bin/stats/tests/http/Makefile.am

@@ -1,2 +1,6 @@
 EXTRA_DIST = __init__.py server.py
 CLEANFILES = __init__.pyc server.pyc
+CLEANDIRS = __pycache__
+
+clean-local:
+	rm -rf $(CLEANDIRS)

src/bin/stats/tests/isc/Makefile.am

@@ -1,3 +1,8 @@
 SUBDIRS = cc config util
 EXTRA_DIST = __init__.py
 CLEANFILES = __init__.pyc
+
+CLEANDIRS = __pycache__
+
+clean-local:
+	rm -rf $(CLEANDIRS)

src/bin/stats/tests/isc/cc/Makefile.am

@@ -1,2 +1,7 @@
 EXTRA_DIST = __init__.py session.py
 CLEANFILES = __init__.pyc session.pyc
+
+CLEANDIRS = __pycache__
+
+clean-local:
+	rm -rf $(CLEANDIRS)

src/bin/stats/tests/isc/config/Makefile.am

@@ -1,2 +1,7 @@
 EXTRA_DIST = __init__.py ccsession.py
 CLEANFILES = __init__.pyc ccsession.pyc
+
+CLEANDIRS = __pycache__
+
+clean-local:
+	rm -rf $(CLEANDIRS)

src/bin/stats/tests/isc/util/Makefile.am

@@ -1,2 +1,7 @@
 EXTRA_DIST = __init__.py process.py
 CLEANFILES = __init__.pyc process.pyc
+
+CLEANDIRS = __pycache__
+
+clean-local:
+	rm -rf $(CLEANDIRS)

src/bin/tests/Makefile.am

@@ -3,6 +3,13 @@ PYTESTS = process_rename_test.py
 # .py will be generated by configure, so we don't have to include it
 # in EXTRA_DIST.
 
+# If necessary (rare cases), explicitly specify paths to dynamic libraries
+# required by loadable python modules.
+LIBRARY_PATH_PLACEHOLDER =
+if SET_ENV_LIBRARY_PATH
+LIBRARY_PATH_PLACEHOLDER += $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/cc/.libs:$(abs_top_builddir)/src/lib/config/.libs:$(abs_top_builddir)/src/lib/log/.libs:$(abs_top_builddir)/src/lib/util/.libs:$(abs_top_builddir)/src/lib/exceptions/.libs:$$$(ENV_LIBRARY_PATH)
+endif
+
 # test using command-line arguments, so use check-local target instead of TESTS
 check-local:
 if ENABLE_PYTHON_COVERAGE
@@ -12,6 +19,7 @@ if ENABLE_PYTHON_COVERAGE
 endif
 	for pytest in $(PYTESTS) ; do \
 	echo Running test: $$pytest ; \
+	$(LIBRARY_PATH_PLACEHOLDER) \
 	env PYTHONPATH=$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python:$(abs_top_builddir)/src/lib/dns/python/.libs \
 	$(PYCOVERAGE_RUN) $(abs_builddir)/$$pytest || exit ; \
 	done

src/bin/xfrin/Makefile.am

@@ -25,3 +25,8 @@ b10-xfrin: xfrin.py
 	$(SED) -e "s|@@PYTHONPATH@@|@pyexecdir@|" \
 	       -e "s|@@LOCALSTATEDIR@@|$(localstatedir)|" xfrin.py >$@
 	chmod a+x $@
+
+CLEANDIRS = __pycache__
+
+clean-local:
+	rm -rf $(CLEANDIRS)

src/bin/xfrin/tests/Makefile.am

@@ -6,7 +6,7 @@ EXTRA_DIST = $(PYTESTS)
 # required by loadable python modules.
 LIBRARY_PATH_PLACEHOLDER =
 if SET_ENV_LIBRARY_PATH
-LIBRARY_PATH_PLACEHOLDER += $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/dns/.libs:$(abs_top_builddir)/src/lib/cryptolink/.libs:$(abs_top_builddir)/src/lib/util/.libs:$(abs_top_builddir)/src/lib/exceptions/.libs:$(abs_top_builddir)/src/lib/xfr/.libs:$$$(ENV_LIBRARY_PATH)
+LIBRARY_PATH_PLACEHOLDER += $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/cc/.libs:$(abs_top_builddir)/src/lib/config/.libs:$(abs_top_builddir)/src/lib/log/.libs:$(abs_top_builddir)/src/lib/dns/.libs:$(abs_top_builddir)/src/lib/cryptolink/.libs:$(abs_top_builddir)/src/lib/util/.libs:$(abs_top_builddir)/src/lib/exceptions/.libs:$(abs_top_builddir)/src/lib/xfr/.libs:$$$(ENV_LIBRARY_PATH)
 endif
 
 # test using command-line arguments, so use check-local target instead of TESTS
@@ -18,7 +18,7 @@ if ENABLE_PYTHON_COVERAGE
 endif
 	for pytest in $(PYTESTS) ; do \
 	echo Running test: $$pytest ; \
-	env PYTHONPATH=$(abs_top_builddir)/src/lib/dns/python/.libs:$(abs_top_builddir)/src/bin/xfrin:$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python \
 	$(LIBRARY_PATH_PLACEHOLDER) \
+	env PYTHONPATH=$(abs_top_builddir)/src/lib/dns/python/.libs:$(abs_top_builddir)/src/bin/xfrin:$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python \
 	$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
 	done

src/bin/xfrin/tests/xfrin_test.py

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

src/bin/xfrin/xfrin.py.in

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

src/bin/xfrout/Makefile.am

@@ -28,3 +28,8 @@ b10-xfrout: xfrout.py
 	$(SED) -e "s|@@PYTHONPATH@@|@pyexecdir@|" \
 	       -e "s|@@LOCALSTATEDIR@@|$(localstatedir)|" xfrout.py >$@
 	chmod a+x $@
+
+CLEANDIRS = __pycache__
+
+clean-local:
+	rm -rf $(CLEANDIRS)

src/bin/xfrout/tests/Makefile.am

@@ -6,7 +6,7 @@ EXTRA_DIST = $(PYTESTS)
 # required by loadable python modules.
 LIBRARY_PATH_PLACEHOLDER =
 if SET_ENV_LIBRARY_PATH
-LIBRARY_PATH_PLACEHOLDER += $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/dns/.libs:$(abs_top_builddir)/src/lib/cryptolink/.libs:$(abs_top_builddir)/src/lib/util/.libs:$(abs_top_builddir)/src/lib/exceptions/.libs:$(abs_top_builddir)/src/lib/util/io/.libs:$$$(ENV_LIBRARY_PATH)
+LIBRARY_PATH_PLACEHOLDER += $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/cc/.libs:$(abs_top_builddir)/src/lib/config/.libs:$(abs_top_builddir)/src/lib/log/.libs:$(abs_top_builddir)/src/lib/dns/.libs:$(abs_top_builddir)/src/lib/cryptolink/.libs:$(abs_top_builddir)/src/lib/util/.libs:$(abs_top_builddir)/src/lib/exceptions/.libs:$(abs_top_builddir)/src/lib/util/io/.libs:$$$(ENV_LIBRARY_PATH)
 endif
 
 # test using command-line arguments, so use check-local target instead of TESTS
@@ -18,7 +18,7 @@ if ENABLE_PYTHON_COVERAGE
 endif
 	for pytest in $(PYTESTS) ; do \
 	echo Running test: $$pytest ; \
-	env PYTHONPATH=$(abs_top_builddir)/src/bin/xfrout:$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python:$(abs_top_builddir)/src/lib/dns/python/.libs:$(abs_top_builddir)/src/lib/util/io/.libs \
 	$(LIBRARY_PATH_PLACEHOLDER) \
-	$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
+	env PYTHONPATH=$(abs_top_builddir)/src/bin/xfrout:$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python:$(abs_top_builddir)/src/lib/dns/python/.libs:$(abs_top_builddir)/src/lib/util/io/.libs \
+	$(PYCOVERAGE_RUN) $(abs_builddir)/$$pytest || exit ; \
 	done

src/bin/xfrout/tests/xfrout_test.py.in

@@ -116,8 +116,8 @@ class TestXfroutSession(unittest.TestCase):
 
     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, TSIGKeyRing())
+        #self.log = isc.log.NSLogger('xfrout', '',  severity = 'critical', log_to_console = False )
+        self.xfrsess = MyXfroutSession(self.sock, None, Dbserver(), 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')
 
@@ -520,7 +520,7 @@ class MyUnixSockServer(UnixSockServer):
         self._shutdown_event = threading.Event()
         self._max_transfers_out = 10
         self._cc = MyCCSession()
-        self._log = isc.log.NSLogger('xfrout', '', severity = 'critical', log_to_console = False )
+        #self._log = isc.log.NSLogger('xfrout', '', severity = 'critical', log_to_console = False )
 
 class TestUnixSockServer(unittest.TestCase):
     def setUp(self):

src/bin/xfrout/xfrout.py.in

@@ -26,7 +26,7 @@ from isc.datasrc import sqlite3_ds
 from socketserver import *
 import os
 from isc.config.ccsession import *
-from isc.log.log import *
+#from isc.log.log import *
 from isc.cc import SessionError, SessionTimeout
 from isc.notify import notify_out
 import isc.util.process
@@ -88,13 +88,13 @@ def get_rrset_len(rrset):
 
 
 class XfroutSession():
-    def __init__(self, sock_fd, request_data, server, log, tsig_key_ring):
+    def __init__(self, sock_fd, request_data, server, 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._log = log
         self._tsig_key_ring = tsig_key_ring
         self._tsig_ctx = None
         self._tsig_len = 0
@@ -110,7 +110,8 @@ class XfroutSession():
             self.dns_xfrout_start(self._sock_fd, self._request_data)
             #TODO, avoid catching all exceptions
         except Exception as e:
-            self._log.log_message("error", str(e))
+            #self._log.log_message("error", str(e))
+            pass
 
         os.close(self._sock_fd)
 
@@ -137,7 +138,7 @@ class XfroutSession():
             rcode = self._check_request_tsig(msg, mdata)
 
         except Exception as err:
-            self._log.log_message("error", str(err))
+            #self._log.log_message("error", str(err))
             return Rcode.FORMERR(), None
 
         return rcode, msg
@@ -244,16 +245,17 @@ class XfroutSession():
         zone_name = self._get_query_zone_name(msg)
         rcode_ = self._check_xfrout_available(zone_name)
         if rcode_ != Rcode.NOERROR():
-            self._log.log_message("info", "transfer of '%s/IN' failed: %s",
-                                  zone_name, rcode_.to_text())
+            #self._log.log_message("info", "transfer of '%s/IN' failed: %s",
+            #                      zone_name, rcode_.to_text())
             return self. _reply_query_with_error_rcode(msg, sock_fd, rcode_)
 
         try:
-            self._log.log_message("info", "transfer of '%s/IN': AXFR started" % zone_name)
+            #self._log.log_message("info", "transfer of '%s/IN': AXFR started" % zone_name)
             self._reply_xfrout_query(msg, sock_fd, zone_name)
-            self._log.log_message("info", "transfer of '%s/IN': AXFR end" % zone_name)
+            #self._log.log_message("info", "transfer of '%s/IN': AXFR end" % zone_name)
         except Exception as err:
-            self._log.log_message("error", str(err))
+            #self._log.log_message("error", str(err))
+            pass
 
         self._server.decrease_transfers_counter()
         return
@@ -317,7 +319,7 @@ class XfroutSession():
 
         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")
+                #self._log.log_message("info", "xfrout process is being shutdown")
                 return
             # TODO: RRType.SOA() ?
             if RRType(rr_data[5]) == RRType("SOA"): #ignore soa record
@@ -357,7 +359,7 @@ class XfroutSession():
 class UnixSockServer(socketserver_mixin.NoPollMixIn, ThreadingUnixStreamServer):
     '''The unix domain socket server which accept xfr query sent from auth server.'''
 
-    def __init__(self, sock_file, handle_class, shutdown_event, config_data, cc, log):
+    def __init__(self, sock_file, handle_class, shutdown_event, config_data, cc):
         self._remove_unused_sock_file(sock_file)
         self._sock_file = sock_file
         socketserver_mixin.NoPollMixIn.__init__(self)
@@ -366,7 +368,7 @@ class UnixSockServer(socketserver_mixin.NoPollMixIn, ThreadingUnixStreamServer):
         self._transfers_counter = 0
         self._shutdown_event = shutdown_event
         self._write_sock, self._read_sock = socket.socketpair()
-        self._log = log
+        #self._log = log
         self.update_config_data(config_data)
         self._cc = cc
 
@@ -394,7 +396,7 @@ class UnixSockServer(socketserver_mixin.NoPollMixIn, ThreadingUnixStreamServer):
         try:
             request, client_address = self.get_request()
         except socket.error:
-            self._log.log_message("error", "Failed to fetch request")
+            #self._log.log_message("error", "Failed to fetch request")
             return
 
         # Check self._shutdown_event to ensure the real shutdown comes.
@@ -408,7 +410,7 @@ class UnixSockServer(socketserver_mixin.NoPollMixIn, ThreadingUnixStreamServer):
                     (rlist, wlist, xlist) = ([], [], [])
                     continue
                 else:
-                    self._log.log_message("error", "Error with select(): %s" %e)
+                    #self._log.log_message("error", "Error with select(): %s" %e)
                     break
 
             # self.server._shutdown_event will be set by now, if it is not a false
@@ -419,8 +421,8 @@ class UnixSockServer(socketserver_mixin.NoPollMixIn, ThreadingUnixStreamServer):
             try:
                 self.process_request(request)
             except:
-                self._log.log_message("error", "Exception happened during processing of %s"
-                                      % str(client_address))
+                #self._log.log_message("error", "Exception happened during processing of %s"
+                #                      % str(client_address))
                 break
 
     def _handle_request_noblock(self):
@@ -438,8 +440,8 @@ class UnixSockServer(socketserver_mixin.NoPollMixIn, ThreadingUnixStreamServer):
             # This may happen when one xfrout process try to connect to
             # xfrout unix socket server, to check whether there is another
             # xfrout running.
-            if sock_fd == FD_COMM_ERROR:
-                self._log.log_message("error", "Failed to receive the file descriptor for XFR connection")
+            #if sock_fd == FD_COMM_ERROR:
+                #self._log.log_message("error", "Failed to receive the file descriptor for XFR connection")
             return
 
         # receive request msg
@@ -456,7 +458,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.tsig_key_ring)
+        self.RequestHandlerClass(sock_fd, request_data, self, self.tsig_key_ring)
 
     def _remove_unused_sock_file(self, sock_file):
         '''Try to remove the socket file. If the file is being used
@@ -464,8 +466,8 @@ class UnixSockServer(socketserver_mixin.NoPollMixIn, ThreadingUnixStreamServer):
         If it's not a socket file or nobody is listening
         , it will be removed. If it can't be removed, exit from python. '''
         if self._sock_file_in_use(sock_file):
-            self._log.log_message("error", "Fail to start xfrout process, unix socket file '%s'"
-                                 " is being used by another xfrout process\n" % sock_file)
+            #self._log.log_message("error", "Fail to start xfrout process, unix socket file '%s'"
+            #                     " is being used by another xfrout process\n" % sock_file)
             sys.exit(0)
         else:
             if not os.path.exists(sock_file):
@@ -474,7 +476,7 @@ class UnixSockServer(socketserver_mixin.NoPollMixIn, ThreadingUnixStreamServer):
             try:
                 os.unlink(sock_file)
             except OSError as err:
-                self._log.log_message("error", "[b10-xfrout] Fail to remove file %s: %s\n" % (sock_file, err))
+                #self._log.log_message("error", "[b10-xfrout] Fail to remove file %s: %s\n" % (sock_file, err))
                 sys.exit(0)
 
     def _sock_file_in_use(self, sock_file):
@@ -495,17 +497,18 @@ class UnixSockServer(socketserver_mixin.NoPollMixIn, ThreadingUnixStreamServer):
         try:
             os.unlink(self._sock_file)
         except Exception as e:
-            self._log.log_message('error', str(e))
+            #self._log.log_message('error', str(e))
+            pass
 
     def update_config_data(self, new_config):
         '''Apply the new config setting of xfrout module. '''
-        self._log.log_message('info', 'update config data start.')
+        #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._log.log_message('info', 'max transfer out : %d', self._max_transfers_out)
         self._lock.release()
-        self._log.log_message('info', 'update config data complete.')
+        #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. """
@@ -521,7 +524,7 @@ class UnixSockServer(socketserver_mixin.NoPollMixIn, ThreadingUnixStreamServer):
                 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)
+                #self._log.log_message('error', '%s' % errmsg)
 
     def get_db_file(self):
         file, is_default = self._cc.get_remote_config_value("Auth", "database_file")
@@ -553,16 +556,16 @@ class UnixSockServer(socketserver_mixin.NoPollMixIn, ThreadingUnixStreamServer):
 class XfroutServer:
     def __init__(self):
         self._unix_socket_server = None
-        self._log = None
+        #self._log = None
         self._listen_sock_file = UNIX_SOCKET_FILE
         self._shutdown_event = threading.Event()
-        self._cc = isc.config.ModuleCCSession(SPECFILE_LOCATION, self.config_handler, self.command_handler)
+        self._cc = isc.config.ModuleCCSession(SPECFILE_LOCATION, self.config_handler, self.command_handler, None, True)
         self._config_data = self._cc.get_full_config()
         self._cc.start()
         self._cc.add_remote_config(AUTH_SPECFILE_LOCATION);
-        self._log = isc.log.NSLogger(self._config_data.get('log_name'), self._config_data.get('log_file'),
-                                self._config_data.get('log_severity'), self._config_data.get('log_versions'),
-                                self._config_data.get('log_max_bytes'), True)
+        #self._log = isc.log.NSLogger(self._config_data.get('log_name'), self._config_data.get('log_file'),
+        #                        self._config_data.get('log_severity'), self._config_data.get('log_versions'),
+        #                        self._config_data.get('log_max_bytes'), True)
         self._start_xfr_query_listener()
         self._start_notifier()
 
@@ -570,13 +573,13 @@ class XfroutServer:
         '''Start a new thread to accept xfr query. '''
         self._unix_socket_server = UnixSockServer(self._listen_sock_file, XfroutSession,
                                                   self._shutdown_event, self._config_data,
-                                                  self._cc, self._log);
+                                                  self._cc)
         listener = threading.Thread(target=self._unix_socket_server.serve_forever)
         listener.start()
 
     def _start_notifier(self):
         datasrc = self._unix_socket_server.get_db_file()
-        self._notifier = notify_out.NotifyOut(datasrc, self._log)
+        self._notifier = notify_out.NotifyOut(datasrc)
         self._notifier.dispatcher()
 
     def send_notify(self, zone_name, zone_class):
@@ -591,8 +594,8 @@ class XfroutServer:
                 continue
             self._config_data[key] = new_config[key]
 
-        if self._log:
-            self._log.update_config(new_config)
+        #if self._log:
+        #    self._log.update_config(new_config)
 
         if self._unix_socket_server:
             self._unix_socket_server.update_config_data(self._config_data)
@@ -621,7 +624,7 @@ class XfroutServer:
 
     def command_handler(self, cmd, args):
         if cmd == "shutdown":
-            self._log.log_message("info", "Received shutdown command.")
+            #self._log.log_message("info", "Received shutdown command.")
             self.shutdown()
             answer = create_answer(0)
 
@@ -629,8 +632,8 @@ class XfroutServer:
             zone_name = args.get('zone_name')
             zone_class = args.get('zone_class')
             if zone_name and zone_class:
-                self._log.log_message("info", "zone '%s/%s': receive notify others command" \
-                                       % (zone_name, zone_class))
+                #self._log.log_message("info", "zone '%s/%s': receive notify others command" \
+                #                       % (zone_name, zone_class))
                 self.send_notify(zone_name, zone_class)
                 answer = create_answer(0)
             else:

src/bin/zonemgr/Makefile.am

@@ -26,3 +26,8 @@ b10-zonemgr: zonemgr.py
 	$(SED) -e "s|@@PYTHONPATH@@|@pyexecdir@|" \
 	       -e "s|@@LOCALSTATEDIR@@|$(localstatedir)|" zonemgr.py >$@
 	chmod a+x $@
+
+CLEANDIRS = __pycache__
+
+clean-local:
+	rm -rf $(CLEANDIRS)

src/bin/zonemgr/tests/Makefile.am

@@ -3,6 +3,13 @@ PYTESTS = zonemgr_test.py
 EXTRA_DIST = $(PYTESTS)
 CLEANFILES = initdb.file
 
+# If necessary (rare cases), explicitly specify paths to dynamic libraries
+# required by loadable python modules.
+LIBRARY_PATH_PLACEHOLDER =
+if SET_ENV_LIBRARY_PATH
+LIBRARY_PATH_PLACEHOLDER += $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/cc/.libs:$(abs_top_builddir)/src/lib/config/.libs:$(abs_top_builddir)/src/lib/log/.libs:$(abs_top_builddir)/src/lib/util/.libs:$(abs_top_builddir)/src/lib/exceptions/.libs:$$$(ENV_LIBRARY_PATH)
+endif
+
 # test using command-line arguments, so use check-local target instead of TESTS
 check-local:
 if ENABLE_PYTHON_COVERAGE
@@ -12,6 +19,7 @@ if ENABLE_PYTHON_COVERAGE
 endif
 	for pytest in $(PYTESTS) ; do \
 	echo Running test: $$pytest ; \
+	$(LIBRARY_PATH_PLACEHOLDER) \
 	env PYTHONPATH=$(abs_top_builddir)/src/bin/zonemgr:$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python:$(abs_top_builddir)/src/lib/dns/.libs:$(abs_top_builddir)/src/lib/dns/python/.libs:$(abs_top_builddir)/src/lib/xfr/.libs \
 	$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
 	done

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/acl/Makefile.am

@@ -2,16 +2,26 @@ SUBDIRS = . tests
 
 AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib
 AM_CPPFLAGS += $(BOOST_INCLUDES)
-AM_CPPFLAGS += -I$(top_srcdir)/src/lib/util -I$(top_builddir)/src/lib/util
 
 AM_CXXFLAGS = $(B10_CXXFLAGS)
 
-CLEANFILES = *.gcno *.gcda
-
+# The core library
 lib_LTLIBRARIES = libacl.la
-libacl_la_SOURCES  = check.h
+libacl_la_SOURCES  = acl.h
+libacl_la_SOURCES += check.h
 libacl_la_SOURCES += ip_check.h ip_check.cc
+libacl_la_SOURCES += loader.h loader.cc
+
+libacl_la_LIBADD = $(top_builddir)/src/lib/exceptions/libexceptions.la
+libacl_la_LIBADD += $(top_builddir)/src/lib/cc/libcc.la
+libacl_la_LIBADD += $(top_builddir)/src/lib/util/libutil.la
+
+# DNS specialized one
+lib_LTLIBRARIES += libdnsacl.la
 
-libacl_la_CXXFLAGS = $(AM_CXXFLAGS)
-libacl_la_CPPFLAGS = $(AM_CPPFLAGS)
-libacl_la_LIBADD = $(top_builddir)/src/lib/util/libutil.la
+libdnsacl_la_SOURCES = dns.h dns.cc
+
+libdnsacl_la_LIBADD = libacl.la
+libdnsacl_la_LIBADD += $(top_builddir)/src/lib/dns/libdns++.la
+
+CLEANFILES = *.gcno *.gcda

src/lib/acl/acl.h

@@ -0,0 +1,140 @@
+// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef ACL_ACL_H
+#define ACL_ACL_H
+
+#include "check.h"
+#include <vector>
+
+#include <boost/shared_ptr.hpp>
+#include <boost/noncopyable.hpp>
+
+namespace isc {
+namespace acl {
+
+/**
+ * \brief Default actions an ACL could perform.
+ *
+ * This is the default for the ACL class. It is possible to specify any other
+ * data type, as the ACL class does nothing about them, but these look
+ * reasonable, so they are provided for convenience. It is not specified what
+ * exactly these mean and it's up to whoever uses them.
+ */
+enum BasicAction {
+    ACCEPT,
+    REJECT,
+    DROP
+};
+
+/**
+ * \brief The ACL itself.
+ *
+ * It holds bunch of ordered entries, each one consisting of a check (
+ * of any kind, it might be even compound) and an action that is returned
+ * whenever the action matches. They are tested in the order and first
+ * match counts.
+ *
+ * This is non-copyable. It seems that there's no need to copy them (even
+ * when it would be technically possible), so we forbid it just to prevent
+ * copying it by accident. If there really is legitimate use, this restriction
+ * can be removed.
+ *
+ * The class is template. It is possible to specify on which context the checks
+ * match and which actions it returns. The actions must be copyable
+ * for this to work and it is expected to be something small, usually an enum
+ * (but other objects are also possible).
+ *
+ * \note There are protected functions. In fact, you should consider them
+ *     private, they are protected so tests can get inside. This class
+ *     is not expected to be subclassed in real applications.
+ */
+template<typename Context, typename Action = BasicAction> class ACL :
+    public boost::noncopyable {
+public:
+    /**
+     * \brief Constructor.
+     *
+     * \param default_action It is the action that is returned when the checked
+     *     things "falls off" the end of the list (when no rule matched).
+     */
+    ACL(const Action& default_action) : default_action_(default_action)
+    {}
+
+    /**
+     * \brief Pointer to the check.
+     *
+     * We use the shared pointer, because we are not able to copy the checks.
+     * However, we might need to copy the entries (when we concatenate ACLs
+     * together in future).
+     */
+    typedef boost::shared_ptr<const Check<Context> > ConstCheckPtr;
+
+    /**
+     * \brief The actual main function that decides.
+     *
+     * This is the function that takes the entries one by one, checks
+     * the context against conditions and if it matches, returns the
+     * action that belongs to the first matched entry or default action
+     * if nothing matches.
+     * \param context The thing that should be checked. It is directly
+     *     passed to the checks.
+     */
+    const Action& execute(const Context& context) const {
+        const typename Entries::const_iterator end(entries_.end());
+        for (typename Entries::const_iterator i(entries_.begin()); i != end;
+             ++i) {
+            if (i->first->matches(context)) {
+                return (i->second);
+            }
+        }
+        return (default_action_);
+    }
+
+    /**
+     * \brief Add new entry at the end of the list.
+     *
+     * \note We currently allow only adding at the end. This is enough for now,
+     * but we may need more when we start implementing some kind optimisations,
+     * including replacements, reorderings and removals.
+     *
+     * \param check The check to test if the thing matches.
+     * \param action The action to return when the thing matches this check.
+     */
+    void append(ConstCheckPtr check, const Action& action) {
+        entries_.push_back(Entry(check, action));
+    }
+private:
+    // Just type abbreviations.
+    typedef std::pair<ConstCheckPtr, Action> Entry;
+    typedef std::vector<Entry> Entries;
+    /// \brief The default action, when nothing mathes.
+    const Action default_action_;
+    /// \brief The entries we have.
+    Entries entries_;
+protected:
+    /**
+     * \brief Get the default action.
+     *
+     * This is for testing purposes only.
+     */
+    const Action& getDefaultAction() const {
+        return (default_action_);
+    }
+};
+
+}
+}
+
+#endif

src/lib/acl/check.h

@@ -44,6 +44,11 @@ namespace acl {
  * (eg. the packet, information where it came from, to what port, ...).
  */
 template<typename Context> class Check {
+protected:
+    /// \brief Constructor.
+    ///
+    /// Just to make sure this thing is not directly instantiated.
+    Check() { }
 public:
     /**
      * \brief The check itself.

src/lib/acl/dns.cc

@@ -0,0 +1,34 @@
+// 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 "dns.h"
+
+namespace isc {
+namespace acl {
+namespace dns {
+
+Loader&
+getLoader() {
+    static Loader* loader(NULL);
+    if (loader == NULL) {
+        loader = new Loader(REJECT);
+        // TODO: This is the place where we register default check creators
+        // like IP check, etc, once we have them.
+    }
+    return (*loader);
+}
+
+}
+}
+}

src/lib/acl/dns.h

@@ -0,0 +1,89 @@
+// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef ACL_DNS_H
+#define ACL_DNS_H
+
+#include "loader.h"
+
+#include <asiolink/io_address.h>
+#include <dns/message.h>
+
+namespace isc {
+namespace acl {
+namespace dns {
+
+/**
+ * \brief DNS request to be checked.
+ *
+ * This plays the role of Context of the generic template ACLs (in namespace
+ * isc::acl).
+ *
+ * It is simple structure holding just the bunch of information. Therefore
+ * the names don't end up with a slash, there are no methods so they can't be
+ * confused with local variables.
+ *
+ * \todo Do we want a constructor to set this in a shorter manner? So we can
+ *     call the ACLs directly?
+ */
+struct RequestContext {
+    /// \brief The DNS message (payload).
+    isc::dns::ConstMessagePtr message;
+    /// \brief The remote IP address (eg. the client).
+    asiolink::IOAddress remote_address;
+    /// \brief The local IP address (ours, of the interface where we received).
+    asiolink::IOAddress local_address;
+    /// \brief The remote port.
+    uint16_t remote_port;
+    /// \brief The local port.
+    uint16_t local_port;
+    /**
+     * \brief Name of the TSIG key the message is signed with.
+     *
+     * This will be either the name of the TSIG key the message is signed with,
+     * or empty string, if the message is not signed. It is true we could get
+     * the information from the message itself, but because at the time when
+     * the ACL is checked, the signature has been verified already, so passing
+     * it around is probably cheaper.
+     *
+     * It is expected that messages with invalid signatures are handled before
+     * ACL.
+     */
+    std::string tsig_key_name;
+};
+
+/// \brief DNS based check.
+typedef acl::Check<RequestContext> Check;
+/// \brief DNS based compound check.
+typedef acl::CompoundCheck<RequestContext> CompoundCheck;
+/// \brief DNS based ACL.
+typedef acl::ACL<RequestContext> ACL;
+/// \brief DNS based ACL loader.
+typedef acl::Loader<RequestContext> Loader;
+
+/**
+ * \brief Loader singleton access function.
+ *
+ * This function returns a loader of ACLs. It is expected applications
+ * will use this function instead of creating their own loaders, because
+ * one is enough, this one will have registered default checks and it
+ * is known one, so any plugins can registrer additional checks as well.
+ */
+Loader& getLoader();
+
+}
+}
+}
+
+#endif

src/lib/acl/loader.cc

@@ -0,0 +1,46 @@
+// 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 "loader.h"
+
+using namespace std;
+
+namespace isc {
+namespace acl {
+
+BasicAction defaultActionLoader(data::ConstElementPtr actionEl) {
+    try {
+        const string action(actionEl->stringValue());
+        if (action == "ACCEPT") {
+            return (ACCEPT);
+        } else if (action == "REJECT") {
+            return (REJECT);
+        } else if (action == "DROP") {
+            return (DROP);
+        } else {
+            throw LoaderError(__FILE__, __LINE__,
+                              string("Unknown action '" + action + "'").
+                                  c_str(),
+                              actionEl);
+        }
+    }
+    catch (const data::TypeError&) {
+        throw LoaderError(__FILE__, __LINE__,
+                          "Invalid element type for action, must be string",
+                          actionEl);
+    }
+}
+
+}
+}

src/lib/acl/loader.h

@@ -0,0 +1,404 @@
+// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef ACL_LOADER_H
+#define ACL_LOADER_H
+
+#include "acl.h"
+#include <cc/data.h>
+#include <boost/function.hpp>
+#include <boost/shared_ptr.hpp>
+#include <map>
+
+namespace isc {
+namespace acl {
+
+/**
+ * \brief Exception for bad ACL specifications.
+ *
+ * This will be thrown by the Loader if the ACL description is malformed
+ * in some way.
+ *
+ * It also can hold optional JSON element where was the error detected, so
+ * it can be examined.
+ *
+ * Checks may subclass this exception for similar errors if they see it fit.
+ */
+class LoaderError : public BadValue {
+private:
+    const data::ConstElementPtr element_;
+public:
+    /**
+     * \brief Constructor.
+     *
+     * Should be used with isc_throw if the fourth argument isn't used.
+     *
+     * \param file The file where the throw happened.
+     * \param line Similar as file, just for the line number.
+     * \param what Human readable description of what happened.
+     * \param element This might be passed to hold the JSON element where
+     *     the error was detected.
+     */
+    LoaderError(const char* file, size_t line, const char* what,
+                data::ConstElementPtr element = data::ConstElementPtr()) :
+        BadValue(file, line, what),
+        element_(element)
+    {}
+    ~ LoaderError() throw() {}
+    /**
+     * \brief Get the element.
+     *
+     * This returns the element where the error was detected. Note that it
+     * might be NULL in some situations.
+     */
+    const data::ConstElementPtr& element() const {
+        return (element_);
+    }
+};
+
+/**
+ * \brief Loader of the default actions of ACLs.
+ *
+ * Declared outside the Loader class, as this one does not need to be
+ * templated. This will throw LoaderError if the parameter isn't string
+ * or if it doesn't contain one of the accepted values.
+ *
+ * \param action The JSON representation of the action. It must be a string
+ *     and contain one of "ACCEPT", "REJECT" or "DENY".
+ * \note We could define different names or add aliases if needed.
+ */
+BasicAction defaultActionLoader(data::ConstElementPtr action);
+
+/**
+ * \brief Loader of ACLs.
+ *
+ * The goal of this class is to convert JSON description of an ACL to object
+ * of the ACL class (including the checks inside it).
+ *
+ * The class can be used to load the checks only. This is supposed to be used
+ * by compound checks to create the subexpressions.
+ *
+ * To allow any kind of checks to exist in the application, creators are
+ * registered for the names of the checks.
+ *
+ * An ACL definition looks like this:
+ * \verbatim
+ * [
+ *   {
+ *      "action": "ACCEPT",
+ *      "match-type": <parameter>
+ *   },
+ *   {
+ *      "action": "REJECT",
+ *      "match-type": <parameter>
+ *      "another-match-type": [<parameter1>, <parameter2>]
+*    },
+*    {
+*       "action": "DROP"
+*    }
+ * ]
+ * \endverbatim
+ *
+ * This is a list of elements. Each element must have an "action"
+ * entry/keyword. That one specifies which action is returned if this
+ * element matches (the value of the key is passed to the action loader
+ * (see the constructor). It may be any piece of JSON which the action
+ * loader expects.
+ *
+ * The rest of the element are matches. The left side is the name of the
+ * match type (for example match for source IP address or match for message
+ * size). The <parameter> is whatever is needed to describe the match and
+ * depends on the match type, the loader passes it verbatim to creator
+ * of that match type.
+ *
+ * There may be multiple match types in single element. In such case, all
+ * of the matches must match for the element to take action (so, in the second
+ * element, both "match-type" and "another-match-type" must be satisfied).
+ * If there's no match in the element, the action is taken/returned without
+ * conditions, every time (makes sense as the last entry, as the ACL will
+ * never get past it).
+ *
+ * The second entry shows another thing - if there's a list as the value
+ * for some match and the match itself is not expecting a list, it is taken
+ * as an "or" - a match for at last one of the choices in the list must match.
+ * So, for the second entry, both "match-type" and "another-match-type" must
+ * be satisfied, but the another one is satisfied by either parameter1 or
+ * parameter2.
+ */
+template<typename Context, typename Action = BasicAction> class Loader {
+public:
+    /**
+     * \brief Constructor.
+     *
+     * \param default_action The default action for created ACLs.
+     * \param actionLoader is the loader which will be used to convert actions
+     *     from their JSON representation. The default value is suitable for
+     *     the BasicAction enum. If you did not specify the second
+     *     template argument, you don't need to specify this loader.
+     */
+    Loader(const Action& defaultAction,
+           const boost::function1<Action, data::ConstElementPtr>
+               &actionLoader = &defaultActionLoader) :
+        default_action_(defaultAction),
+        action_loader_(actionLoader)
+    {}
+    /**
+     * \brief Creator of the checks.
+     *
+     * This can be registered within the Loader and will be used to create the
+     * checks. It is expected multiple creators (for multiple types, one can
+     * handle even multiple names) will be created and registered to support
+     * range of things we could check. This allows for customizing/extending
+     * the loader.
+     */
+    class CheckCreator {
+    public:
+        /**
+         * \brief List of names supported by this loader.
+         *
+         * List of all names for which this loader is able to create the
+         * checks. There can be multiple names, to support both aliases
+         * to the same checks and creators capable of creating multiple
+         * types of checks.
+         */
+        virtual std::vector<std::string> names() const = 0;
+        /**
+         * \brief Creates the check.
+         *
+         * This function does the actual creation. It is passed all the
+         * relevant data and is supposed to return shared pointer to the
+         * check.
+         *
+         * It is expected to throw the LoaderError exception when the
+         * definition is invalid.
+         *
+         * \param name The type name of the check. If the creator creates
+         *     only one type of check, it can safely ignore this parameter.
+         * \param definition The part of JSON describing the parameters of
+         *     check. As there's no way for the loader to know how the
+         *     parameters might look like, they are not checked in any way.
+         *     Therefore it's up to the creator (or the check being created)
+         *     to validate the data and throw if it is bad.
+         * \param Current loader calling this creator. This can be used
+         *     to load subexpressions in case of compound check.
+         */
+        virtual boost::shared_ptr<Check<Context> > create(
+            const std::string& name, data::ConstElementPtr definition,
+            const Loader<Context, Action>& loader) = 0;
+        /**
+         * \brief Is list or-abbreviation allowed?
+         *
+         * If this returns true and the parameter (eg. the value we check
+         * against, the one that is passed as the second parameter of create)
+         * is list, the loader will call the create method with each element of
+         * the list and aggregate all the results in OR compound check. If it
+         * is false, the parameter is passed verbatim no matter if it is or
+         * isn't a list. For example, IP check will have this as true (so
+         * multiple IP addresses can be passed as options), but AND operator
+         * will return false and handle the list of subexpressions itself.
+         *
+         * The rationale behind this is that it is common to specify list of
+         * something that matches (eg. list of IP addresses).
+         */
+        virtual bool allowListAbbreviation() const {
+            return (true);
+        }
+    };
+    /**
+     * \brief Register another check creator.
+     *
+     * Adds a creator to the list of known ones. The creator's list of names
+     * must be disjoint with the names already known to the creator or the
+     * LoaderError exception is thrown. In such case, the creator is not
+     * registered under any of the names. In case of other exceptions, like
+     * bad_alloc, only weak exception safety is guaranteed.
+     *
+     * \param creator Shared pointer to the creator.
+     * \note We don't support deregistration yet, but it is expected it will
+     *     be needed in future, when we have some kind of plugins. These
+     *     plugins might want to unload, in which case they would need to
+     *     deregister their creators. It is expected they would pass the same
+     *     pointer to such method as they pass here.
+     */
+    void registerCreator(boost::shared_ptr<CheckCreator> creator) {
+        // First check we can insert all the names
+        typedef std::vector<std::string> Strings;
+        const Strings names(creator->names());
+        for (Strings::const_iterator i(names.begin()); i != names.end();
+             ++i) {
+            if (creators_.find(*i) != creators_.end()) {
+                isc_throw(LoaderError, "The loader already contains creator "
+                          "named " << *i);
+            }
+        }
+        // Now insert them
+        for (Strings::const_iterator i(names.begin()); i != names.end();
+             ++i) {
+            creators_[*i] = creator;
+        }
+    }
+    /**
+     * \brief Load a check.
+     *
+     * This parses a check dict (block, the one element of ACL) and calls a
+     * creator (or creators, if more than one check is found inside) for it. It
+     * ignores the "action" key, as it is a reserved keyword used to specify
+     * actions inside the ACL.
+     *
+     * This may throw LoaderError if it is not a dict or if some of the type
+     * names is not known (there's no creator registered for it). The
+     * exceptions from creators aren't caught.
+     *
+     * \param description The JSON description of the check.
+     */
+    boost::shared_ptr<Check<Context> > loadCheck(const data::ConstElementPtr&
+                                                 description)
+    {
+        // Get the description as a map
+        typedef std::map<std::string, data::ConstElementPtr> Map;
+        Map map;
+        try {
+            map = description->mapValue();
+        }
+        catch (const data::TypeError&) {
+            isc_throw_1(LoaderError, "Check description is not a map",
+                        description);
+        }
+        // Call the internal part with extracted map
+        return (loadCheck(description, map));
+    }
+    /**
+     * \brief Load an ACL.
+     *
+     * This parses an ACL list, creates the checks and actions of each element
+     * and returns it. It may throw LoaderError if it isn't a list or the
+     * "action" key is missing in some element. Also, no exceptions from
+     * loadCheck (therefore from whatever creator is used) and from the
+     * actionLoader passed to constructor are not caught.
+     *
+     * \param description The JSON list of ACL.
+     */
+    boost::shared_ptr<ACL<Context, Action> > load(const data::ConstElementPtr&
+                                                  description)
+    {
+        // We first check it's a list, so we can use the list reference
+        // (the list may be huge)
+        if (description->getType() != data::Element::list) {
+            isc_throw_1(LoaderError, "ACL not a list", description);
+        }
+        // First create an empty ACL
+        const List &list(description->listValue());
+        boost::shared_ptr<ACL<Context, Action> > result(
+            new ACL<Context, Action>(default_action_));
+        // Run trough the list of elements
+        for (List::const_iterator i(list.begin()); i != list.end(); ++i) {
+            Map map;
+            try {
+                map = (*i)->mapValue();
+            }
+            catch (const data::TypeError&) {
+                isc_throw_1(LoaderError, "ACL element not a map", *i);
+            }
+            // Create an action for the element
+            const Map::const_iterator action(map.find("action"));
+            if (action == map.end()) {
+                isc_throw_1(LoaderError, "No action in ACL element", *i);
+            }
+            const Action acValue(action_loader_(action->second));
+            // Now create the check if there's one
+            if (map.size() >= 2) { // One is the action, another one the check
+                result->append(loadCheck(*i, map), acValue);
+            } else {
+                // In case there's no check, this matches every time. We
+                // simulate it by our own private "True" check.
+                result->append(boost::shared_ptr<Check<Context> >(new True()),
+                               acValue);
+            }
+        }
+        return (result);
+    }
+private:
+    // Some type aliases to save typing
+    typedef std::map<std::string, boost::shared_ptr<CheckCreator> > Creators;
+    typedef std::map<std::string, data::ConstElementPtr> Map;
+    typedef std::vector<data::ConstElementPtr> List;
+    // Private members
+    Creators creators_;
+    const Action default_action_;
+    const boost::function1<Action, data::ConstElementPtr> action_loader_;
+    /**
+     * \brief Internal version of loadCheck.
+     *
+     * This is the internal part, shared between load and loadCheck.
+     * \param description The bit of JSON (used in exceptions).
+     * \param map The extracted map describing the check. It does change
+     *     the map.
+     */
+    boost::shared_ptr<Check<Context> > loadCheck(const data::ConstElementPtr&
+                                                 description, Map& map)
+    {
+        // Remove the action keyword
+        map.erase("action");
+        // Now, do we have any definition? Or is it and abbreviation?
+        switch (map.size()) {
+            case 0:
+                isc_throw_1(LoaderError, "Check description is empty",
+                            description);
+            case 1: {
+                // Get the first and only item
+                const Map::const_iterator checkDesc(map.begin());
+                const std::string& name(checkDesc->first);
+                const typename Creators::const_iterator
+                    creatorIt(creators_.find(name));
+                if (creatorIt == creators_.end()) {
+                    isc_throw_1(LoaderError, "No creator for ACL check " <<
+                                name, description);
+                }
+                if (creatorIt->second->allowListAbbreviation() &&
+                    checkDesc->second->getType() == data::Element::list) {
+                    isc_throw_1(LoaderError,
+                                "Not implemented (OR-abbreviated form)",
+                                checkDesc->second);
+                }
+                // Create the check and return it
+                return (creatorIt->second->create(name, checkDesc->second,
+                                                  *this));
+            }
+            default:
+                isc_throw_1(LoaderError,
+                            "Not implemented (AND-abbreviated form)",
+                            description);
+        }
+    }
+    /**
+     * \brief Check that always matches.
+     *
+     * This one is used internally for ACL elements without condition. We may
+     * want to make this publicly accesible sometime maybe, but for now,
+     * there's no need.
+     */
+    class True : public Check<Context> {
+    public:
+        virtual bool matches(const Context&) const { return (true); };
+        virtual unsigned cost() const { return (1); }
+        // We don't write "true" here, as this one was created using empty
+        // input
+        virtual std::string toText() const { return ""; }
+    };
+};
+
+}
+}
+
+#endif

src/lib/acl/tests/Makefile.am

@@ -1,27 +1,27 @@
 AM_CPPFLAGS = -I$(top_builddir)/src/lib -I$(top_srcdir)/src/lib
-AM_CXXFLAGS = $(B10_CXXFLAGS)
-
-if USE_STATIC_LINK
-AM_LDFLAGS = -static
-endif
-
-CLEANFILES = *.gcno *.gcda
+AM_CPPFLAGS += $(BOOST_INCLUDES)
 
 TESTS =
 if HAVE_GTEST
 TESTS += run_unittests
 run_unittests_SOURCES = run_unittests.cc
+run_unittests_SOURCES += acl_test.cc
 run_unittests_SOURCES += check_test.cc
+run_unittests_SOURCES += dns_test.cc
 run_unittests_SOURCES += ip_check_unittest.cc
+run_unittests_SOURCES += loader_test.cc
+run_unittests_SOURCES += logcheck.h
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)
 
 run_unittests_LDADD = $(GTEST_LDADD)
+run_unittests_LDADD += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la
 run_unittests_LDADD += $(top_builddir)/src/lib/acl/libacl.la
 run_unittests_LDADD += $(top_builddir)/src/lib/util/libutil.la
-run_unittests_LDADD += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la
+run_unittests_LDADD += $(top_builddir)/src/lib/cc/libcc.la
 run_unittests_LDADD += $(top_builddir)/src/lib/log/liblog.la
 run_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
+run_unittests_LDADD += $(top_builddir)/src/lib/acl/libdnsacl.la
 endif
 
 noinst_PROGRAMS = $(TESTS)

src/lib/acl/tests/acl_test.cc

@@ -0,0 +1,84 @@
+// 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 "logcheck.h"
+
+namespace {
+
+// Test version of the Acl class. It adds few methods to examine the protected
+// data, but does not change the implementation.
+class TestACL : public ACL<Log> {
+public:
+    TestACL() :
+        ACL<Log>(DROP)
+    {}
+    // Check the stored default action there
+    void checkDefaultAction(BasicAction ac) {
+        EXPECT_EQ(getDefaultAction(), ac);
+    }
+};
+
+// The test fixture. Contains some members so they don't need to be manually
+// created each time and some convenience functions.
+class ACLTest : public ::testing::Test {
+public:
+    ACLTest() :
+        next_check_(0)
+    {}
+    TestACL acl_;
+    Log log_;
+    size_t next_check_;
+    shared_ptr<Check<Log> > getCheck(bool accepts) {
+        return (shared_ptr<Check<Log> >(new ConstCheck(accepts,
+                                                       next_check_++)));
+    }
+};
+
+/*
+ * This tests the default action and that nothing is run if nothing is
+ * inserted (it's hard to imagine otherwise though).
+ *
+ * We use the default ACL unchanged from the test class.
+ */
+TEST_F(ACLTest, emptyRule) {
+    acl_.checkDefaultAction(DROP);
+    EXPECT_EQ(DROP, acl_.execute(log_));
+    // No test was run
+    log_.checkFirst(0);
+}
+
+/*
+ * This tests the default action in case no check matches.
+ */
+TEST_F(ACLTest, noMatch) {
+    acl_.append(getCheck(false), ACCEPT);
+    acl_.append(getCheck(false), REJECT);
+    EXPECT_EQ(DROP, acl_.execute(log_));
+    // The first two checks were actually run (and didn't match)
+    log_.checkFirst(2);
+}
+
+/*
+ * Checks that it takes the first matching check and returns the
+ * value. Also checks that the others aren't run at all.
+ */
+TEST_F(ACLTest, firstMatch) {
+    acl_.append(getCheck(false), ACCEPT);
+    acl_.append(getCheck(true), REJECT);
+    acl_.append(getCheck(true), ACCEPT);
+    EXPECT_EQ(REJECT, acl_.execute(log_));
+    log_.checkFirst(2);
+}
+
+}

src/lib/acl/tests/dns_test.cc

@@ -0,0 +1,35 @@
+// 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 <acl/dns.h>
+#include <gtest/gtest.h>
+
+using namespace isc::acl::dns;
+
+namespace {
+
+// Tests that the getLoader actually returns something, returns the same every
+// time and the returned value can be used to anything. It is not much of a
+// test, but the getLoader is not much of a function.
+TEST(DNSACL, getLoader) {
+    Loader* l(&getLoader());
+    ASSERT_TRUE(l != NULL);
+    EXPECT_EQ(l, &getLoader());
+    EXPECT_NO_THROW(l->load(isc::data::Element::fromJSON(
+        "[{\"action\": \"DROP\"}]")));
+    // TODO Test that the things we should register by default, like IP based
+    // check, are loaded.
+}
+
+}

src/lib/acl/tests/loader_test.cc

@@ -0,0 +1,426 @@
+// 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 "logcheck.h"
+#include <acl/loader.h>
+#include <string>
+#include <gtest/gtest.h>
+
+using namespace std;
+using namespace boost;
+using isc::data::ConstElementPtr;
+
+namespace {
+
+// Just for convenience, create JSON objects from JSON string
+ConstElementPtr el(const string& JSON) {
+    return (isc::data::Element::fromJSON(JSON));
+}
+
+// We don't use the EXPECT_THROW macro, as it doesn't allow us
+// to examine the exception. We want to check the element is stored
+// there as well.
+void testActionLoaderException(const string& JSON) {
+    SCOPED_TRACE("Should throw with input: " + JSON);
+    ConstElementPtr elem(el(JSON));
+    try {
+        defaultActionLoader(elem);
+        FAIL() << "It did not throw";
+    }
+    catch (const LoaderError& error) {
+        // Yes, comparing for pointer equality, that is enough, it
+        // should return the exact instance of the JSON object
+        EXPECT_EQ(elem, error.element());
+    }
+}
+
+// Test the defaultActionLoader function
+TEST(LoaderHelpers, DefaultActionLoader) {
+    // First the three valid inputs
+    EXPECT_EQ(ACCEPT, defaultActionLoader(el("\"ACCEPT\"")));
+    EXPECT_EQ(REJECT, defaultActionLoader(el("\"REJECT\"")));
+    EXPECT_EQ(DROP, defaultActionLoader(el("\"DROP\"")));
+    // Now few invalid ones
+    // String, but unknown one
+    testActionLoaderException("\"UNKNOWN\"");
+    testActionLoaderException("42");
+    testActionLoaderException("true");
+    testActionLoaderException("null");
+    testActionLoaderException("[]");
+    testActionLoaderException("{}");
+}
+
+// A check that doesn't check anything but remembers it's own name
+// and data
+class NamedCheck : public Check<Log> {
+public:
+    NamedCheck(const string& name, ConstElementPtr data) :
+        name_(name),
+        data_(data)
+    {}
+    virtual bool matches(const Log&) const { return (true); }
+    const string name_;
+    const ConstElementPtr data_;
+};
+
+// The creator of NamedCheck
+class NamedCreator : public Loader<Log>::CheckCreator {
+public:
+    NamedCreator(const string& name, bool abbreviatedList = true) :
+        abbreviated_list_(abbreviatedList)
+    {
+        names_.push_back(name);
+    }
+    NamedCreator(const vector<string>& names) :
+        names_(names),
+        abbreviated_list_(true)
+    {}
+    vector<string> names() const {
+        return (names_);
+    }
+    shared_ptr<Check<Log> > create(const string& name, ConstElementPtr data,
+                                   const Loader<Log>&)
+    {
+        bool found(false);
+        for (vector<string>::const_iterator i(names_.begin());
+             i != names_.end(); ++i) {
+            if (*i == name) {
+                found = true;
+                break;
+            }
+        }
+        EXPECT_TRUE(found) << "Name " << name << " passed to creator which "
+            "doesn't handle it.";
+        return (shared_ptr<Check<Log> >(new NamedCheck(name, data)));
+    }
+    bool allowListAbbreviation() const {
+        return (abbreviated_list_);
+    }
+private:
+    vector<string> names_;
+    const bool abbreviated_list_;
+};
+
+// To be thrown in tests internally
+class TestCreatorError {};
+
+// This will throw every time it should create something
+class ThrowCreator : public Loader<Log>::CheckCreator {
+public:
+    vector<string> names() const {
+        vector<string> result;
+        result.push_back("throw");
+        return (result);
+    }
+    shared_ptr<Check<Log> > create(const string&, ConstElementPtr,
+                                   const Loader<Log>&)
+    {
+        throw TestCreatorError();
+    }
+};
+
+// This throws whenever the match is called on it
+class ThrowCheck : public Check<Log> {
+public:
+    virtual bool matches(const Log&) const {
+        throw TestCreatorError();
+    }
+};
+
+// And creator for it
+class ThrowCheckCreator : public Loader<Log>::CheckCreator {
+public:
+    vector<string> names() const {
+        vector<string> result;
+        result.push_back("throwcheck");
+        return (result);
+    }
+    shared_ptr<Check<Log> > create(const string&, ConstElementPtr,
+                                   const Loader<Log>&)
+    {
+        return (shared_ptr<Check<Log> >(new ThrowCheck()));
+    }
+};
+
+class LogCreator : public Loader<Log>::CheckCreator {
+public:
+    vector<string> names() const {
+        vector<string> result;
+        result.push_back("logcheck");
+        return (result);
+    }
+    /*
+     * For simplicity, we just take two values as a list, first is the
+     * logging cell used, the second is result of the check. No error checking
+     * is done, if there's bug in the test, it will throw TypeError for us.
+     */
+    shared_ptr<Check<Log> > create(const string&, ConstElementPtr definition,
+                                   const Loader<Log>&)
+    {
+        vector<ConstElementPtr> list(definition->listValue());
+        int logpos(list[0]->intValue());
+        bool accept(list[1]->boolValue());
+        return (shared_ptr<ConstCheck>(new ConstCheck(accept, logpos)));
+    }
+    // We take a list, so don't interpret it for us
+    virtual bool allowListAbbreviation() const { return (false); }
+};
+
+class LoaderTest : public ::testing::Test {
+public:
+    LoaderTest() :
+        loader_(REJECT)
+    {}
+    Loader<Log> loader_;
+    Log log_;
+    // Some convenience functions to set up
+
+    // Create a NamedCreator, convert to shared pointer
+    shared_ptr<NamedCreator> namedCreator(const string& name,
+                                          bool abbreviatedList = true)
+    {
+        return (shared_ptr<NamedCreator>(new NamedCreator(name,
+                                                          abbreviatedList)));
+    }
+    // Create and add a NamedCreator
+    void addNamed(const string& name, bool abbreviatedList = true) {
+        EXPECT_NO_THROW(loader_.registerCreator(
+            namedCreator(name, abbreviatedList)));
+    }
+    // Load a check and convert it to named check to examine it
+    shared_ptr<NamedCheck> loadCheck(const string& definition) {
+        SCOPED_TRACE("Loading check " + definition);
+        shared_ptr<Check<Log> > loaded;
+        EXPECT_NO_THROW(loaded = loader_.loadCheck(el(definition)));
+        shared_ptr<NamedCheck> result(dynamic_pointer_cast<NamedCheck>(
+            loaded));
+        EXPECT_TRUE(result);
+        return (result);
+    }
+    // The loadCheck throws an exception
+    void checkException(const string& JSON) {
+        SCOPED_TRACE("Loading check exception: " + JSON);
+        ConstElementPtr input(el(JSON));
+        // Not using EXPECT_THROW, we want to examine the exception
+        try {
+            loader_.loadCheck(input);
+            FAIL() << "Should have thrown";
+        }
+        catch (const LoaderError& e) {
+            // It should be identical copy, so checking pointers
+            EXPECT_EQ(input, e.element());
+        }
+    }
+    // Insert the throw, throwcheck and logcheck checks into the loader
+    void aclSetup() {
+        try {
+            loader_.registerCreator(shared_ptr<ThrowCreator>(new
+                                                             ThrowCreator()));
+            loader_.registerCreator(shared_ptr<ThrowCheckCreator>(
+                new ThrowCheckCreator()));
+            loader_.registerCreator(shared_ptr<LogCreator>(new LogCreator()));
+        }
+        // We ignore this exception here, because it happens when we try to
+        // insert the creators multiple times. This is harmless.
+        catch (const LoaderError&) {}
+    }
+    // Create an ACL, run it, check it's result and how many first
+    // log items it marked
+    //
+    // Works with preset names throw and logcheck
+    void aclRun(const string& JSON, BasicAction expectedResult,
+                size_t logged)
+    {
+        SCOPED_TRACE("Running ACL for " + JSON);
+        aclSetup();
+        shared_ptr<ACL<Log> > acl;
+        EXPECT_NO_THROW(acl = loader_.load(el(JSON)));
+        EXPECT_EQ(expectedResult, acl->execute(log_));
+        log_.checkFirst(logged);
+    }
+    // Check it throws an error when creating the ACL
+    void aclException(const string& JSON) {
+        SCOPED_TRACE("Trying to load bad " + JSON);
+        aclSetup();
+        EXPECT_THROW(loader_.load(el(JSON)), LoaderError);
+    }
+};
+
+// Test that it does not accept duplicate creator
+TEST_F(LoaderTest, CreatorDuplicity) {
+    addNamed("name");
+    EXPECT_THROW(loader_.registerCreator(namedCreator("name")), LoaderError);
+}
+
+// Test that when it does not accept a duplicate, nothing is inserted
+TEST_F(LoaderTest, CreatorDuplicateUnchanged) {
+    addNamed("name1");
+    vector<string> names;
+    names.push_back("name2");
+    names.push_back("name1");
+    names.push_back("name3");
+    EXPECT_THROW(loader_.registerCreator(
+        shared_ptr<NamedCreator>(new NamedCreator(names))), LoaderError);
+    // It should now reject both name2 and name3 as not known
+    checkException("{\"name2\": null}");
+    checkException("{\"name3\": null}");
+}
+
+// Test that we can register a creator and load a check with the name
+TEST_F(LoaderTest, SimpleCheckLoad) {
+    addNamed("name");
+    shared_ptr<NamedCheck> check(loadCheck("{\"name\": 42}"));
+    EXPECT_EQ("name", check->name_);
+    EXPECT_TRUE(check->data_->equals(*el("42")));
+}
+
+// As above, but there are multiple creators registered within the loader
+TEST_F(LoaderTest, MultiCreatorCheckLoad) {
+    addNamed("name1");
+    addNamed("name2");
+    shared_ptr<NamedCheck> check(loadCheck("{\"name2\": 42}"));
+    EXPECT_EQ("name2", check->name_);
+    EXPECT_TRUE(check->data_->equals(*el("42")));
+}
+
+// Similar to above, but there's a creator with multiple names
+TEST_F(LoaderTest, MultiNameCheckLoad) {
+    addNamed("name1");
+    vector<string> names;
+    names.push_back("name2");
+    names.push_back("name3");
+    EXPECT_NO_THROW(loader_.registerCreator(shared_ptr<NamedCreator>(
+        new NamedCreator(names))));
+    shared_ptr<NamedCheck> check(loadCheck("{\"name3\": 42}"));
+    EXPECT_EQ("name3", check->name_);
+    EXPECT_TRUE(check->data_->equals(*el("42")));
+}
+
+// Invalid format is rejected
+TEST_F(LoaderTest, InvalidFormatCheck) {
+    checkException("[]");
+    checkException("42");
+    checkException("\"hello\"");
+    checkException("null");
+}
+
+// Empty check is rejected
+TEST_F(LoaderTest, EmptyCheck) {
+    checkException("{}");
+}
+
+// The name isn't known
+TEST_F(LoaderTest, UnkownName) {
+    checkException("{\"unknown\": null}");
+}
+
+// Exception from the creator is propagated
+TEST_F(LoaderTest, CheckPropagate) {
+    loader_.registerCreator(shared_ptr<ThrowCreator>(new ThrowCreator()));
+    EXPECT_THROW(loader_.loadCheck(el("{\"throw\": null}")), TestCreatorError);
+}
+
+// The abbreviated form is not yet implemented
+// (we need the operators to be implemented)
+TEST_F(LoaderTest, AndAbbrev) {
+    addNamed("name1");
+    addNamed("name2");
+    EXPECT_THROW(loader_.loadCheck(el("{\"name1\": 1, \"name2\": 2}")),
+                 LoaderError);
+}
+
+TEST_F(LoaderTest, OrAbbrev) {
+    addNamed("name1");
+    EXPECT_THROW(loader_.loadCheck(el("{\"name1\": [1, 2]}")),
+                 LoaderError);
+}
+
+// But this is not abbreviated form, this should be passed directly to the
+// creator
+TEST_F(LoaderTest, ListCheck) {
+    addNamed("name1", false);
+    shared_ptr<NamedCheck> check(loadCheck("{\"name1\": [1, 2]}"));
+    EXPECT_EQ("name1", check->name_);
+    EXPECT_TRUE(check->data_->equals(*el("[1, 2]")));
+}
+
+// Check the action key is ignored as it should be
+TEST_F(LoaderTest, CheckNoAction) {
+    addNamed("name1");
+    shared_ptr<NamedCheck> check(loadCheck("{\"name1\": 1, \"action\": 2}"));
+    EXPECT_EQ("name1", check->name_);
+    EXPECT_TRUE(check->data_->equals(*el("1")));
+}
+
+// The empty ACL can be created and run, providing the default action
+TEST_F(LoaderTest, EmptyACL) {
+    aclRun("[]", REJECT, 0);
+}
+
+// We can create a simple ACL, which will return the correct default
+// action
+TEST_F(LoaderTest, NoMatchACL) {
+    aclRun("[{\"logcheck\": [0, false], \"action\": \"ACCEPT\"}]",
+           REJECT, 1);
+}
+
+// We can created more complicated ACL, it will match at the second
+// check
+TEST_F(LoaderTest, MatchACL) {
+    aclRun("["
+           "  {\"logcheck\": [0, false], \"action\": \"DROP\"},"
+           "  {\"logcheck\": [1, true], \"action\": \"ACCEPT\"}"
+           "]", ACCEPT, 2);
+}
+
+// ACL without a check (matches unconditionally)
+// We add another one check after it, to make sure it is really not run
+TEST_F(LoaderTest, NoCheckACL) {
+    aclRun("["
+           "  {\"action\": \"DROP\"},"
+           "  {\"throwcheck\": 1, \"action\": \"ACCEPT\"}"
+           "]", DROP, 0);
+}
+
+// Malformed things are rejected
+TEST_F(LoaderTest, InvalidACLFormat) {
+    // Not a list
+    aclException("{}");
+    aclException("42");
+    aclException("true");
+    aclException("null");
+    aclException("\"hello\"");
+    // Malformed element
+    aclException("[42]");
+    aclException("[\"hello\"]");
+    aclException("[[]]");
+    aclException("[true]");
+    aclException("[null]");
+}
+
+// If there's no action keyword, it is rejected
+TEST_F(LoaderTest, NoAction) {
+    aclException("[{}]");
+    aclException("[{\"logcheck\": [0, true]}]");
+}
+
+// Exceptions from check creation is propagated
+TEST_F(LoaderTest, ACLPropagate) {
+    aclSetup();
+    EXPECT_THROW(loader_.load(el("[{\"action\": \"ACCEPT\", \"throw\": 1}]")),
+                 TestCreatorError);
+
+}
+
+}

src/lib/acl/tests/logcheck.h

@@ -0,0 +1,86 @@
+// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <gtest/gtest.h>
+#include <acl/acl.h>
+#include <cassert>
+
+// This is not a public header, it is used only inside the tests. Therefore
+// we lower the standards a bit and use anonymous namespace in the header
+// and "using", just for convenience. This is just to share little bit of code
+// between multiple tests.
+using namespace isc::acl;
+using boost::shared_ptr;
+
+namespace {
+
+// This is arbitrary guess of size for the log. If it's too small for your
+// test, just make it bigger.
+const size_t LOG_SIZE = 10;
+
+// This will remember which checks did run already.
+struct Log {
+    // The actual log cells, if i-th check did run
+    mutable bool run[LOG_SIZE];
+    Log() {
+        // Nothing run yet
+        for (size_t i(0); i < LOG_SIZE; ++ i) {
+            run[i] = false;
+        }
+    }
+    // Checks that the first amount of checks did run and the rest didn't.
+    void checkFirst(size_t amount) const {
+        ASSERT_LE(amount, LOG_SIZE) << "Wrong test: amount bigger than size "
+            "of log";
+        {
+            SCOPED_TRACE("Checking that the first amount of checks did run");
+            for (size_t i(0); i < amount; ++ i) {
+                EXPECT_TRUE(run[i]) << "Check #" << i << " did not run.";
+            }
+        }
+
+        {
+            SCOPED_TRACE("Checking that the rest did not run");
+            for (size_t i(amount); i < LOG_SIZE; ++ i) {
+                EXPECT_FALSE(run[i]) << "Check #" << i << "did run.";
+            }
+        }
+    }
+};
+
+// This returns true or false every time, no matter what is passed to it.
+// But it logs that it did run.
+class ConstCheck : public Check<Log> {
+public:
+    ConstCheck(bool accepts, size_t logNum) :
+        logNum_(logNum),
+        accepts_(accepts)
+    {
+        assert(logNum < LOG_SIZE); // If this fails, the LOG_SIZE is too small
+    }
+    virtual bool matches(const Log& log) const {
+        /*
+         * This is abuse of the context. It is designed to carry the
+         * information to check, not to modify it. However, this is the
+         * easiest way to do the test, so we go against the design.
+         */
+        log.run[logNum_] = true;
+        return (accepts_);
+    }
+private:
+    size_t logNum_;
+    bool accepts_;
+};
+
+}

src/lib/asiodns/asiodef.mes

@@ -16,7 +16,7 @@ $PREFIX ASIODNS_
 $NAMESPACE isc::asiodns
 
 % FETCHCOMP   upstream fetch to %1(%2) has now completed
-A debug message, this records the the upstream fetch (a query made by the
+A debug message, this records that the upstream fetch (a query made by the
 resolver on behalf of its client) to the specified address has completed.
 
 % FETCHSTOP   upstream fetch to %1(%2) has been stopped
@@ -27,12 +27,12 @@ enabled.
 % OPENSOCK    error %1 opening %2 socket to %3(%4)
 The asynchronous I/O code encountered an error when trying to open a socket
 of the specified protocol in order to send a message to the target address.
-The the number of the system error that cause the problem is given in the
+The number of the system error that cause the problem is given in the
 message.
 
 % RECVSOCK    error %1 reading %2 data from %3(%4)
-The asynchronous I/O code encountered an error when trying read data from
-the specified address on the given protocol.  The the number of the system
+The asynchronous I/O code encountered an error when trying to read data from
+the specified address on the given protocol.  The number of the system
 error that cause the problem is given in the message.
 
 % SENDSOCK    error %1 sending data using %2 to %3(%4)

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/asiolink/interval_timer.cc

@@ -14,11 +14,9 @@
 
 #include <config.h>
 
-#include <unistd.h>             // for some IPC/network system calls
-#include <sys/socket.h>
-#include <netinet/in.h>
-
 #include <boost/bind.hpp>
+#include <boost/enable_shared_from_this.hpp>
+#include <boost/shared_ptr.hpp>
 
 #include <exceptions/exceptions.h>
 
@@ -29,7 +27,16 @@
 namespace isc {
 namespace asiolink {
 
-class IntervalTimerImpl {
+/// This class holds a call back function of asynchronous operations.
+/// To ensure the object is alive while an asynchronous operation refers
+/// to it, we use shared_ptr and enable_shared_from_this.
+/// The object will be destructed in case IntervalTimer has been destructed
+/// and no asynchronous operation refers to it.
+/// Please follow the link to get an example:
+/// http://think-async.com/asio/asio-1.4.8/doc/asio/tutorial/tutdaytime3.html#asio.tutorial.tutdaytime3.the_tcp_connection_class
+class IntervalTimerImpl :
+    public boost::enable_shared_from_this<IntervalTimerImpl>
+{
 private:
     // prohibit copy
     IntervalTimerImpl(const IntervalTimerImpl& source);
@@ -53,14 +60,18 @@ private:
     long interval_;
     // asio timer
     asio::deadline_timer timer_;
+    // interval_ will be set to this value in destructor in order to detect
+    // use-after-free type of bugs.
+    static const long INVALIDATED_INTERVAL = -1;
 };
 
 IntervalTimerImpl::IntervalTimerImpl(IOService& io_service) :
     interval_(0), timer_(io_service.get_io_service())
 {}
 
-IntervalTimerImpl::~IntervalTimerImpl()
-{}
+IntervalTimerImpl::~IntervalTimerImpl() {
+    interval_ = INVALIDATED_INTERVAL;
+}
 
 void
 IntervalTimerImpl::setup(const IntervalTimer::Callback& cbfunc,
@@ -81,42 +92,46 @@ IntervalTimerImpl::setup(const IntervalTimer::Callback& cbfunc,
     // At this point the timer is not running yet and will not expire.
     // After calling IOService::run(), the timer will expire.
     update();
-    return;
 }
 
 void
 IntervalTimerImpl::update() {
-    if (interval_ == 0) {
-        // timer has been canceled.  Do nothing.
-        return;
-    }
     try {
         // Update expire time to (current time + interval_).
         timer_.expires_from_now(boost::posix_time::millisec(interval_));
+        // Reset timer.
+        // Pass a function bound with a shared_ptr to this.
+        timer_.async_wait(boost::bind(&IntervalTimerImpl::callback,
+                                      shared_from_this(),
+                                      asio::placeholders::error));
     } catch (const asio::system_error& e) {
-        isc_throw(isc::Unexpected, "Failed to update timer");
+        isc_throw(isc::Unexpected, "Failed to update timer: " << e.what());
+    } catch (const boost::bad_weak_ptr&) {
+        // Can't happen. It means a severe internal bug.
+        assert(0);
     }
-    // Reset timer.
-    timer_.async_wait(boost::bind(&IntervalTimerImpl::callback, this, _1));
 }
 
 void
-IntervalTimerImpl::callback(const asio::error_code& cancelled) {
-    // Do not call cbfunc_ in case the timer was cancelled.
-    // The timer will be canelled in the destructor of asio::deadline_timer.
-    if (!cancelled) {
-        cbfunc_();
+IntervalTimerImpl::callback(const asio::error_code& ec) {
+    assert(interval_ != INVALIDATED_INTERVAL);
+    if (interval_ == 0 || ec) {
+        // timer has been canceled. Do nothing.
+    } else {
         // Set next expire time.
         update();
+        // Invoke the call back function.
+        cbfunc_();
     }
 }
 
-IntervalTimer::IntervalTimer(IOService& io_service) {
-    impl_ = new IntervalTimerImpl(io_service);
-}
+IntervalTimer::IntervalTimer(IOService& io_service) :
+    impl_(new IntervalTimerImpl(io_service))
+{}
 
 IntervalTimer::~IntervalTimer() {
-    delete impl_;
+    // Cancel the timer to make sure cbfunc_() will not be called any more.
+    cancel();
 }
 
 void

src/lib/asiolink/interval_timer.h

@@ -16,6 +16,7 @@
 #define __ASIOLINK_INTERVAL_TIMER_H 1
 
 #include <boost/function.hpp>
+#include <boost/shared_ptr.hpp>
 
 #include <asiolink/io_service.h>
 
@@ -42,9 +43,6 @@ class IntervalTimerImpl;
 /// The call back function will not be called if the instance of this class is
 /// destroyed before the timer is expired.
 ///
-/// Note: Destruction of an instance of this class while call back is pending
-/// causes throwing an exception from \c IOService.
-///
 /// Sample code:
 /// \code
 ///  void function_to_call_back() {
@@ -100,12 +98,12 @@ public:
     /// \param interval Interval in milliseconds (greater than 0)
     ///
     /// Note: IntervalTimer will not pass \c asio::error_code to
-    /// call back function. In case the timer is cancelled, the function
+    /// call back function. In case the timer is canceled, the function
     /// will not be called.
     ///
     /// \throw isc::InvalidParameter cbfunc is empty
     /// \throw isc::BadValue interval is less than or equal to 0
-    /// \throw isc::Unexpected ASIO library error
+    /// \throw isc::Unexpected internal runtime error
     void setup(const Callback& cbfunc, const long interval);
 
     /// Cancel the timer.
@@ -127,7 +125,7 @@ public:
     long getInterval() const;
 
 private:
-    IntervalTimerImpl* impl_;
+    boost::shared_ptr<IntervalTimerImpl> impl_;
 };
 
 } // namespace asiolink

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/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

@@ -247,7 +247,9 @@ readLoggersConf(std::vector<isc::log::LoggerSpecification>& specs,
 } // end anonymous namespace
 
 void
-my_logconfig_handler(const std::string&n, ConstElementPtr new_config, const ConfigData& config_data) {
+default_logconfig_handler(const std::string& module_name,
+                          ConstElementPtr new_config,
+                          const ConfigData& config_data) {
     config_data.getModuleSpec().validateConfig(new_config, true);
 
     std::vector<isc::log::LoggerSpecification> specs;
@@ -353,7 +355,7 @@ ModuleCCSession::ModuleCCSession(
 
     // Keep track of logging settings automatically
     if (handle_logging) {
-        addRemoteConfig("Logging", my_logconfig_handler, false);
+        addRemoteConfig("Logging", default_logconfig_handler, false);
     }
 
     if (start_immediately) {

src/lib/config/ccsession.h

@@ -354,6 +354,25 @@ private:
     ModuleSpec fetchRemoteSpec(const std::string& module, bool is_filename);
 };
 
+/// \brief Default handler for logging config updates
+///
+/// When CCSession is initialized with handle_logging set to true,
+/// this callback will be used to update the logger when a configuration
+/// change comes in.
+///
+/// This function updates the (global) loggers by initializing a
+/// LoggerManager and passing the settings as specified in the given
+/// configuration update.
+///
+/// \param module_name The name of the module
+/// \param new_config The modified configuration values
+/// \param config_data The full config data for the (remote) logging
+///                    module.
+void
+default_logconfig_handler(const std::string& module_name,
+                          isc::data::ConstElementPtr new_config,
+                          const ConfigData& config_data);
+
 }
 }
 #endif // __CCSESSION_H

src/lib/datasrc/cache.cc

@@ -100,6 +100,19 @@ public:
     /// \return \c RRsetPtr
     RRsetPtr getRRset() const { return (entry->rrset); }
 
+    /// \brief Returns name associated with cached node
+    ///
+    /// This is the name associated with the RRset if it is a positive
+    /// entry, and the associated question name if the RRSet is NULL
+    /// and this is a negative entry (together with an indication that
+    /// this is a negative entry).
+    string getNodeName() const {
+        if (getRRset()) {
+            return (getRRset()->getName().toText());
+        }
+        return (std::string("negative entry for ") + question.toText());
+    }
+
     /// \brief Returns the query response flags associated with the data.
     ///
     /// \return \c uint32_t
@@ -213,7 +226,7 @@ HotCacheImpl::HotCacheImpl(int slots, bool enabled) :
 inline void
 HotCacheImpl::insert(const CacheNodePtr node) {
     LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_CACHE_INSERT).
-        arg(node->getRRset()->getName());
+        arg(node->getNodeName());
     std::map<Question, CacheNodePtr>::const_iterator iter;
     iter = map_.find(node->question);
     if (iter != map_.end()) {
@@ -253,7 +266,7 @@ HotCacheImpl::promote(CacheNodePtr node) {
 void
 HotCacheImpl::remove(ConstCacheNodePtr node) {
     LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_CACHE_REMOVE).
-        arg(node->getRRset()->getName());
+        arg(node->getNodeName());
     lru_.erase(node->lru_entry_);
     map_.erase(node->question);
     --count_;

src/lib/datasrc/messagedef.mes

@@ -88,13 +88,13 @@ and put it into the additional section.
 Debug information. The domain is a CNAME (or a DNAME and we created a CNAME
 for it already), so it's being followed.
 
-% QUERY_EMPTY_CNAME cNAME at '%1' is empty
+% QUERY_EMPTY_CNAME CNAME at '%1' is empty
 There was an CNAME and it was being followed. But it contains no records,
 so there's nowhere to go. There will be no answer. This indicates a problem
 with supplied data.
 We tried to follow
 
-% QUERY_TOO_MANY_CNAMES cNAME chain limit exceeded at '%1'
+% QUERY_TOO_MANY_CNAMES CNAME chain limit exceeded at '%1'
 A CNAME led to another CNAME and it led to another, and so on. After 16
 CNAMEs, the software gave up. Long CNAME chains are discouraged, and this
 might possibly be a loop as well. Note that some of the CNAMEs might have
@@ -283,9 +283,9 @@ some other data. But the protocol forbids coexistence of CNAME with anything
 
 % MEM_CNAME_COEXIST can't add data to CNAME in domain '%1'
 This is the same problem as in MEM_CNAME_TO_NONEMPTY, but it happened the
-other way around -- adding some outher data to CNAME.
+other way around -- adding some other data to CNAME.
 
-% MEM_DNAME_NS dNAME and NS can't coexist in non-apex domain '%1'
+% MEM_DNAME_NS DNAME and NS can't coexist in non-apex domain '%1'
 It was requested for DNAME and NS records to be put into the same domain
 which is not the apex (the top of the zone). This is forbidden by RFC
 2672, section 3. This indicates a problem with provided data.
@@ -299,13 +299,13 @@ It was attempted to add the domain into a zone that shouldn't have it
 (eg. the domain is not subdomain of the zone origin). This indicates a
 problem with provided data.
 
-% MEM_WILDCARD_NS nS record in wildcard domain '%1'
+% MEM_WILDCARD_NS NS record in wildcard domain '%1'
 The software refuses to load NS records into a wildcard domain.  It isn't
 explicitly forbidden, but the protocol is ambiguous about how this should
 behave and BIND 9 refuses that as well. Please describe your intention using
 different tools.
 
-% MEM_WILDCARD_DNAME dNAME record in wildcard domain '%1'
+% MEM_WILDCARD_DNAME DNAME record in wildcard domain '%1'
 The software refuses to load DNAME records into a wildcard domain.  It isn't
 explicitly forbidden, but the protocol is ambiguous about how this should
 behave and BIND 9 refuses that as well. Please describe your intention using
@@ -421,15 +421,11 @@ Debug information. The SQLite data source is looking up records of given name
 and type in the database.
 
 % SQLITE_ENCLOSURE looking for zone containing '%1'
-Debug information. The SQLite data source is trying to identify, which zone
+Debug information. The SQLite data source is trying to identify which zone
 should hold this domain.
 
-% SQLITE_ENCLOSURE_BAD_CLASS class mismatch looking for a zone ('%1' and '%2')
-The SQLite data source can handle only one class at a time and it was asked
-to identify which zone is holding data of a different class.
-
 % SQLITE_ENCLOSURE_NOTFOUND no zone contains it
-Debug information. The last SQLITE_ENCLOSURE query was unsuccessful, there's
+Debug information. The last SQLITE_ENCLOSURE query was unsuccessful; there's
 no such zone in our data.
 
 % SQLITE_PREVIOUS looking for name previous to '%1'
@@ -476,13 +472,13 @@ Debug information. The SQLite data source is identifying if this domain is
 a referral and where it goes.
 
 % SQLITE_FINDREF_BAD_CLASS class mismatch looking for referral ('%1' and '%2')
-The SQLite data source was trying to identify, if there's a referral. But
+The SQLite data source was trying to identify if there's a referral. But
 it contains different class than the query was for.
 
-% SQLITE_CREATE sQLite data source created
+% SQLITE_CREATE SQLite data source created
 Debug information. An instance of SQLite data source is being created.
 
-% SQLITE_DESTROY sQLite data source destroyed
+% SQLITE_DESTROY SQLite data source destroyed
 Debug information. An instance of SQLite data source is being destroyed.
 
 % SQLITE_SETUP setting up SQLite database

src/lib/datasrc/sqlite3_datasrc.cc

@@ -351,8 +351,6 @@ Sqlite3DataSrc::findClosestEnclosure(DataSrcMatch& match) const {
     LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_SQLITE_ENCLOSURE).
         arg(match.getName());
     if (match.getClass() != getClass() && match.getClass() != RRClass::ANY()) {
-        LOG_ERROR(logger, DATASRC_SQLITE_ENCLOSURE_BAD_CLASS).arg(getClass()).
-            arg(match.getClass());
         return;
     }
 

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/dns/python/Makefile.am

@@ -16,6 +16,9 @@ pydnspp_la_SOURCES += tsigrecord_python.cc tsigrecord_python.h
 pydnspp_la_SOURCES += tsig_python.cc tsig_python.h
 
 pydnspp_la_CPPFLAGS = $(AM_CPPFLAGS) $(PYTHON_INCLUDES)
+# Note: PYTHON_CXXFLAGS may have some -Wno... workaround, which must be
+# placed after -Wextra defined in AM_CXXFLAGS
+pydnspp_la_CXXFLAGS = $(AM_CXXFLAGS) $(PYTHON_CXXFLAGS)
 pydnspp_la_LDFLAGS = $(PYTHON_LDFLAGS)
 
 # directly included from source files, so these don't have their own

src/lib/dns/python/tests/Makefile.am

@@ -41,3 +41,8 @@ endif
 	$(LIBRARY_PATH_PLACEHOLDER) \
 	$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
 	done
+
+CLEANDIRS = __pycache__
+
+clean-local:
+	rm -rf $(CLEANDIRS)

src/lib/exceptions/exceptions.h

@@ -163,6 +163,17 @@ public:
         oss__ << stream; \
         throw type(__FILE__, __LINE__, oss__.str().c_str()); \
     } while (1)
+
+///
+/// Similar as isc_throw, but allows the exception to have one additional
+/// parameter (the stream/text goes first)
+#define isc_throw_1(type, stream, param1) \
+    do { \
+        std::ostringstream oss__; \
+        oss__ << stream; \
+        throw type(__FILE__, __LINE__, oss__.str().c_str(), param1); \
+    } while (1)
+
 }
 #endif // __EXCEPTIONS_H
 

src/lib/log/compiler/message.cc

@@ -266,12 +266,12 @@ writePythonFile(const string& file, MessageDictionary& dictionary) {
         "# File created from " << message_file.fullName() << " on " <<
             currentTime() << "\n" <<
         "\n" <<
-        "import isc.log.message\n" <<
+        "import isc.log\n" <<
         "\n";
 
     vector<string> idents(sortedIdentifiers(dictionary));
     BOOST_FOREACH(const string& ident, idents) {
-        pyfile << ident << " = isc.log.message.create(\"" <<
+        pyfile << ident << " = isc.log.create_message(\"" <<
             ident << "\", \"" << quoteString(dictionary.getText(ident)) <<
             "\")\n";
     }