Merge branch 'master' into trac1271

ChangeLog

@@ -1,3 +1,33 @@
+305.	[bug]		jinmei
+	Python isc.dns, isc.datasrc, xfrin, xfrout: fixed reference leak
+	in Message.get_question(), Message.get_section(),
+	RRset.get_rdata(), and DataSourceClient.get_updater().
+	The leak caused severe memory leak in b10-xfrin, and (although no
+	one reported it) should have caused less visible leak in
+	b10-xfrout.  b10-xfrin had its own leak, which was also fixed.
+	(Trac #1028, git a72886e643864bb6f86ab47b115a55e0c7f7fcad)
+
+304.	[bug]		jelte
+	The run_bind10.sh test script now no longer runs processes from
+	an installed version of BIND 10, but will correctly use the
+	build tree paths.
+	(Trac #1246, git 1d43b46ab58077daaaf5cae3c6aa3e0eb76eb5d8)
+
+303.	[bug]		jinmei
+	Changed the installation path for the UNIX domain file used
+	for the communication between b10-auth and b10-xfrout to a
+	"@PACKAGE@" subdirectory (e.g. from /usr/local/var to
+	/usr/local/var/bind10-devel).  This should be transparent change
+	because this file is automatically created and cleaned up, but
+	if the old file somehow remains, it can now be safely removed.
+	(Trac #869, git 96e22f4284307b1d5f15e03837559711bb4f580c)
+
+302.	[bug]		jelte
+	msgq no longer crashes if the remote end is closed while msgq
+	tries to send data. It will now simply drop the message and close
+	the connection itself.
+	(Trac #1180, git 6e68b97b050e40e073f736d84b62b3e193dd870a)
+
 301.	[func]		stephen
 	Add system test for IXFR over TCP.
 	(Trac #1213, git 68ee3818bcbecebf3e6789e81ea79d551a4ff3e8)

src/bin/auth/auth_log.h

@@ -28,19 +28,19 @@ namespace auth {
 /// output.
 
 // Debug messages indicating normal startup are logged at this debug level.
-const int DBG_AUTH_START = 10;
+const int DBG_AUTH_START = DBGLVL_START_SHUT;
 
 // Debug level used to log setting information (such as configuration changes).
-const int DBG_AUTH_OPS = 30;
+const int DBG_AUTH_OPS = DBGLVL_COMMAND;
 
 // 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;
+const int DBG_AUTH_DETAIL = DBGLVL_TRACE_BASIC;
 
 // This level is used to log the contents of packets received and sent.
-const int DBG_AUTH_MESSAGES = 70;
+const int DBG_AUTH_MESSAGES = DBGLVL_TRACE_DETAIL_DATA;
 
 /// 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

src/bin/auth/common.cc

@@ -12,22 +12,25 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <string>
+
 #include <auth/common.h>
 #include <auth/spec_config.h>
 #include <stdlib.h>
 
 using std::string;
 
-string getXfroutSocketPath() {
+string
+getXfroutSocketPath() {
     if (getenv("B10_FROM_BUILD") != NULL) {
-        if (getenv("B10_FROM_SOURCE_LOCALSTATEDIR")) {
+        if (getenv("B10_FROM_SOURCE_LOCALSTATEDIR") != NULL) {
             return (string(getenv("B10_FROM_SOURCE_LOCALSTATEDIR")) +
                     "/auth_xfrout_conn");
         } else {
             return (string(getenv("B10_FROM_BUILD")) + "/auth_xfrout_conn");
         }
     } else {
-        if (getenv("BIND10_XFROUT_SOCKET_FILE")) {
+        if (getenv("BIND10_XFROUT_SOCKET_FILE") != NULL) {
             return (getenv("BIND10_XFROUT_SOCKET_FILE"));
         } else {
             return (UNIX_SOCKET_FILE);

src/bin/auth/spec_config.h.pre.in

@@ -1,16 +1,16 @@
-// Copyright (C) 2009  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.
-
-#define AUTH_SPECFILE_LOCATION "@prefix@/share/@PACKAGE@/auth.spec"
-#define UNIX_SOCKET_FILE "@@LOCALSTATEDIR@@/auth_xfrout_conn"
+// Copyright (C) 2009  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.
+
+#define AUTH_SPECFILE_LOCATION "@prefix@/share/@PACKAGE@/auth.spec"
+#define UNIX_SOCKET_FILE "@@LOCALSTATEDIR@@/@PACKAGE@/auth_xfrout_conn"

src/bin/bind10/bind10_src.py.in

@@ -44,10 +44,12 @@ import os
 # installed on the system
 if "B10_FROM_SOURCE" in os.environ:
     SPECFILE_LOCATION = os.environ["B10_FROM_SOURCE"] + "/src/bin/bind10/bob.spec"
+    ADD_LIBEXEC_PATH = False
 else:
     PREFIX = "@prefix@"
     DATAROOTDIR = "@datarootdir@"
     SPECFILE_LOCATION = "@datadir@/@PACKAGE@/bob.spec".replace("${datarootdir}", DATAROOTDIR).replace("${prefix}", PREFIX)
+    ADD_LIBEXEC_PATH = True
     
 import subprocess
 import signal
@@ -61,6 +63,7 @@ from optparse import OptionParser, OptionValueError
 import io
 import pwd
 import posix
+import copy
 
 import isc.cc
 import isc.util.process
@@ -74,8 +77,8 @@ logger = isc.log.Logger("boss")
 
 # Pending system-wide debug level definitions, the ones we
 # use here are hardcoded for now
-DBG_PROCESS = 10
-DBG_COMMANDS = 30
+DBG_PROCESS = logger.DBGLVL_TRACE_BASIC
+DBG_COMMANDS = logger.DBGLVL_TRACE_DETAIL
 
 # Assign this process some longer name
 isc.util.process.rename(sys.argv[0])
@@ -184,9 +187,9 @@ class ProcessInfo:
         # Environment variables for the child process will be a copy of those
         # of the boss process with any additional specific variables given
         # on construction (self.env).
-        spawn_env = os.environ
+        spawn_env = copy.deepcopy(os.environ)
         spawn_env.update(self.env)
-        if 'B10_FROM_SOURCE' not in os.environ:
+        if ADD_LIBEXEC_PATH:
             spawn_env['PATH'] = "@@LIBEXECDIR@@:" + spawn_env['PATH']
         self.process = subprocess.Popen(self.args,
                                         stdin=subprocess.PIPE,
@@ -369,8 +372,10 @@ class BoB:
 
     def start_creator(self):
         self.curproc = 'b10-sockcreator'
-        self.sockcreator = isc.bind10.sockcreator.Creator("@@LIBEXECDIR@@:" +
-                                                          os.environ['PATH'])
+        creator_path = os.environ['PATH']
+        if ADD_LIBEXEC_PATH:
+            creator_path = "@@LIBEXECDIR@@:" + creator_path
+        self.sockcreator = isc.bind10.sockcreator.Creator(creator_path)
 
     def stop_creator(self, kill=False):
         if self.sockcreator is None:
@@ -547,7 +552,8 @@ class BoB:
         logger.info(BIND10_STARTING_CC)
         self.ccs = isc.config.ModuleCCSession(SPECFILE_LOCATION, 
                                       self.config_handler,
-                                      self.command_handler)
+                                      self.command_handler,
+                                      socket_file = self.msgq_socket_file)
         self.ccs.start()
         logger.debug(DBG_PROCESS, BIND10_STARTED_CC)
 
@@ -636,7 +642,11 @@ class BoB:
         # a cleaner solution, but for a short term workaround we specify the
         # path here, unconditionally, and without even bothering which
         # environment variable should be used.
-        if not "B10_FROM_SOURCE" in os.environ:
+        #
+        # We reuse the ADD_LIBEXEC_PATH variable to see whether we need to
+        # do this, as the conditions that make this workaround needed are
+        # the same as for the libexec path addition
+        if ADD_LIBEXEC_PATH:
             cur_path = os.getenv('DYLD_LIBRARY_PATH')
             cur_path = '' if cur_path is None else ':' + cur_path
             c_channel_env['DYLD_LIBRARY_PATH'] = "@@LIBDIR@@" + cur_path

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

@@ -21,6 +21,7 @@ from bind10_src import ProcessInfo, BoB, parse_args, dump_pid, unlink_pid_file,
 import unittest
 import sys
 import os
+import copy
 import signal
 import socket
 from isc.net.addr import IPAddr
@@ -360,6 +361,10 @@ class TestStartStopProcessesBob(unittest.TestCase):
     of processes and that the right processes are started and stopped
     according to changes in configuration.
     """
+    def check_environment_unchanged(self):
+        # Check whether the environment has not been changed
+        self.assertEqual(original_os_environ, os.environ)
+
     def check_started(self, bob, core, auth, resolver):
         """
         Check that the right sets of services are started. The ones that
@@ -379,6 +384,7 @@ class TestStartStopProcessesBob(unittest.TestCase):
         self.assertEqual(bob.stats, core)
         self.assertEqual(bob.stats_httpd, core)
         self.assertEqual(bob.cmdctl, core)
+        self.check_environment_unchanged()
 
     def check_preconditions(self, bob):
         self.check_started(bob, False, False, False)
@@ -389,6 +395,7 @@ class TestStartStopProcessesBob(unittest.TestCase):
         should be started. Some processes still need to be running.
         """
         self.check_started(bob, True, False, False)
+        self.check_environment_unchanged()
 
     def check_started_both(self, bob):
         """
@@ -396,18 +403,21 @@ class TestStartStopProcessesBob(unittest.TestCase):
         (auth and resolver) are enabled.
         """
         self.check_started(bob, True, True, True)
+        self.check_environment_unchanged()
 
     def check_started_auth(self, bob):
         """
         Check the set of processes needed to run auth only is started.
         """
         self.check_started(bob, True, True, False)
+        self.check_environment_unchanged()
 
     def check_started_resolver(self, bob):
         """
         Check the set of processes needed to run resolver only is started.
         """
         self.check_started(bob, True, False, True)
+        self.check_environment_unchanged()
 
     def check_started_dhcp(self, bob, v4, v6):
         """
@@ -426,6 +436,7 @@ class TestStartStopProcessesBob(unittest.TestCase):
         # there should be exactly one DHCPv6 daemon (if v6==True)
         self.assertEqual(v4==True, v4found==1)
         self.assertEqual(v6==True, v6found==1)
+        self.check_environment_unchanged()
 
     # Checks the processes started when starting neither auth nor resolver
     # is specified.
@@ -799,5 +810,7 @@ class TestBrittle(unittest.TestCase):
         self.assertFalse(bob.runnable)
 
 if __name__ == '__main__':
+    # store os.environ for test_unchanged_environment
+    original_os_environ = copy.deepcopy(os.environ)
     isc.log.resetUnitTestRootLogger()
     unittest.main()

src/bin/cmdctl/cmdctl.py.in

@@ -49,17 +49,12 @@ from hashlib import sha1
 from isc.util import socketserver_mixin
 from isc.log_messages.cmdctl_messages import *
 
-# TODO: these debug-levels are hard-coded here; we are planning on
-# creating a general set of debug levels, see ticket #1074. When done,
-# we should remove these values and use the general ones in the
-# logger.debug calls
-
-# Debug level for communication with BIND10
-DBG_CMDCTL_MESSAGING = 30
-
 isc.log.init("b10-cmdctl")
 logger = isc.log.Logger("cmdctl")
 
+# Debug level for communication with BIND10
+DBG_CMDCTL_MESSAGING = logger.DBGLVL_COMMAND
+
 try:
     import threading
 except ImportError:

src/bin/dhcp6/Makefile.am

@@ -19,10 +19,12 @@ CLEANFILES = *.gcno *.gcda spec_config.h
 man_MANS = b10-dhcp6.8
 EXTRA_DIST = $(man_MANS) dhcp6.spec interfaces.txt
 
-#if ENABLE_MAN
-#b10-dhcp6.8: b10-dhcp6.xml
-#	xsltproc --novalid --xinclude --nonet -o $@ http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl $(srcdir)/b10-dhcp6.xml
-#endif
+if ENABLE_MAN
+
+b10-dhcp6.8: b10-dhcp6.xml
+	xsltproc --novalid --xinclude --nonet -o $@ http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl $(srcdir)/b10-dhcp6.xml
+
+endif
 
 spec_config.h: spec_config.h.pre
 	$(SED) -e "s|@@LOCALSTATEDIR@@|$(localstatedir)|" spec_config.h.pre >$@

src/bin/dhcp6/b10-dhcp6.8

@@ -1,13 +1,13 @@
 '\" t
-.\"     Title: b10-dhpc6
+.\"     Title: b10-dhcp6
 .\"    Author: [FIXME: author] [see http://docbook.sf.net/el/author]
 .\" Generator: DocBook XSL Stylesheets v1.75.2 <http://docbook.sf.net/>
-.\"      Date: March 8, 2011
+.\"      Date: October 27, 2011
 .\"    Manual: BIND10
 .\"    Source: BIND10
 .\"  Language: English
 .\"
-.TH "B10\-DHCP6" "8" "March 8, 2011" "BIND10" "BIND10"
+.TH "B10\-DHCP6" "8" "October 27, 2011" "BIND10" "BIND10"
 .\" -----------------------------------------------------------------
 .\" * set default formatting
 .\" -----------------------------------------------------------------
@@ -19,31 +19,32 @@
 .\" * MAIN CONTENT STARTS HERE *
 .\" -----------------------------------------------------------------
 .SH "NAME"
-b10-dhcp6 \- DHCPv6 daemon in BIND10 architecture
+b10-dhcp6 \- DHCPv6 server in BIND 10 architecture
 .SH "SYNOPSIS"
-.HP \w'\fBb10\-dhcp6
+.HP \w'\fBb10\-dhcp6\fR\ 'u
 \fBb10\-dhcp6\fR [\fB\-v\fR]
 .SH "DESCRIPTION"
 .PP
 The
 \fBb10\-dhcp6\fR
-daemon will provide DHCPv6 server implementation when it becomes functional.
+daemon will provide the DHCPv6 server implementation when it becomes functional\&.
+.SH "ARGUMENTS"
 .PP
+The arguments are as follows:
+.PP
+\fB\-v\fR
+.RS 4
+Enable verbose mode\&.
+.RE
 .SH "SEE ALSO"
 .PP
 
-\fBb10-cfgmgr\fR(8),
-\fBb10-loadzone\fR(8),
-\fBb10-msgq\fR(8),
-\fBb10-stats\fR(8),
-\fBb10-zonemgr\fR(8),
-\fBbind10\fR(8),
-BIND 10 Guide\&.
+\fBbind10\fR(8)\&.
 .SH "HISTORY"
 .PP
 The
 \fBb10\-dhcp6\fR
-daemon was first coded in June 2011\&.
+daemon was first coded in June 2011 by Tomek Mrugalski\&.
 .SH "COPYRIGHT"
 .br
 Copyright \(co 2011 Internet Systems Consortium, Inc. ("ISC")

src/bin/dhcp6/b10-dhcp6.xml

@@ -0,0 +1,98 @@
+<!DOCTYPE book PUBLIC "-//OASIS//DTD DocBook XML V4.2//EN"
+               "http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd"
+	       [<!ENTITY mdash "&#8212;">]>
+<!--
+ - 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.
+-->
+
+<refentry>
+
+  <refentryinfo>
+    <date>October 27, 2011</date>
+  </refentryinfo>
+
+  <refmeta>
+    <refentrytitle>b10-dhcp6</refentrytitle>
+    <manvolnum>8</manvolnum>
+    <refmiscinfo>BIND10</refmiscinfo>
+  </refmeta>
+
+  <refnamediv>
+    <refname>b10-dhcp6</refname>
+    <refpurpose>DHCPv6 server in BIND 10 architecture</refpurpose>
+  </refnamediv>
+
+  <docinfo>
+    <copyright>
+      <year>2011</year>
+      <holder>Internet Systems Consortium, Inc. ("ISC")</holder>
+    </copyright>
+  </docinfo>
+
+  <refsynopsisdiv>
+    <cmdsynopsis>
+      <command>b10-dhcp6</command>
+      <arg><option>-v</option></arg>
+    </cmdsynopsis>
+  </refsynopsisdiv>
+
+  <refsect1>
+    <title>DESCRIPTION</title>
+    <para>
+      The <command>b10-dhcp6</command> daemon will provide the
+       DHCPv6 server implementation when it becomes functional.
+    </para>
+
+  </refsect1>
+
+  <refsect1>
+    <title>ARGUMENTS</title>
+
+    <para>The arguments are as follows:</para>
+
+    <variablelist>
+
+      <varlistentry>
+        <term><option>-v</option></term>
+        <listitem><para>
+          Enable verbose mode.
+<!-- TODO: what does this do? -->
+        </para></listitem>
+      </varlistentry>
+
+    </variablelist>
+  </refsect1>
+
+  <refsect1>
+    <title>SEE ALSO</title>
+    <para>
+      <citerefentry>
+        <refentrytitle>bind10</refentrytitle><manvolnum>8</manvolnum>
+      </citerefentry>.
+    </para>
+  </refsect1>
+
+  <refsect1>
+    <title>HISTORY</title>
+    <para>
+      The <command>b10-dhcp6</command> daemon was first coded in
+      June 2011 by Tomek Mrugalski.
+    </para>
+  </refsect1>
+</refentry><!--
+ - Local variables:
+ - mode: sgml
+ - End:
+-->

src/bin/msgq/msgq.py.in

@@ -28,7 +28,6 @@ import struct
 import errno
 import time
 import select
-import pprint
 import random
 from optparse import OptionParser, OptionValueError
 import isc.util.process
@@ -96,10 +95,10 @@ class MsgQ:
                                "@PACKAGE_NAME@",
                                "msgq_socket").replace("${prefix}",
                                                       "@prefix@")
-    
+
     def __init__(self, socket_file=None, verbose=False):
         """Initialize the MsgQ master.
-        
+
         The socket_file specifies the path to the UNIX domain socket
         that the msgq process listens on. If it is None, the
         environment variable BIND10_MSGQ_SOCKET_FILE is used. If that
@@ -135,7 +134,7 @@ class MsgQ:
             self.poller = select.poll()
         except AttributeError:
             self.kqueue = select.kqueue()
-    
+
     def add_kqueue_socket(self, socket, write_filter=False):
         """Add a kquque filter for a socket.  By default the read
         filter is used; if write_filter is set to True, the write
@@ -167,7 +166,7 @@ class MsgQ:
                              self.socket_file)
 
         self.listen_socket = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
-        
+
         if os.path.exists(self.socket_file):
             os.remove(self.socket_file)
         try:
@@ -196,7 +195,7 @@ class MsgQ:
 
         if self.verbose:
             sys.stdout.write("[b10-msgq] Listening\n")
-        
+
         self.runnable = True
 
     def process_accept(self):
@@ -293,9 +292,6 @@ class MsgQ:
             sys.stderr.write("[b10-msgq] Routing decode error: %s\n" % err)
             return
 
-#        sys.stdout.write("\t" + pprint.pformat(routingmsg) + "\n")
-#        sys.stdout.write("\t" + pprint.pformat(data) + "\n")
-
         self.process_command(fd, sock, routingmsg, data)
 
     def process_command(self, fd, sock, routing, data):
@@ -357,7 +353,18 @@ class MsgQ:
         if fileno in self.sendbuffs:
             amount_sent = 0
         else:
-            amount_sent = self.__send_data(sock, msg)
+            try:
+                amount_sent = self.__send_data(sock, msg)
+            except socket.error as sockerr:
+                # in the case the other side seems gone, kill the socket
+                # and drop the send action
+                if sockerr.errno == errno.EPIPE:
+                    print("[b10-msgq] SIGPIPE on send, dropping message " +
+                          "and closing connection")
+                    self.kill_socket(fileno, sock)
+                    return
+                else:
+                    raise
 
         # Still something to send
         if amount_sent < len(msg):
@@ -448,12 +455,12 @@ class MsgQ:
 
     def run(self):
         """Process messages.  Forever.  Mostly."""
-        
+
         if self.poller:
             self.run_poller()
         else:
             self.run_kqueue()
-    
+
     def run_poller(self):
         while True:
             try:
@@ -511,7 +518,7 @@ def signal_handler(signal, frame):
 
 if __name__ == "__main__":
     def check_port(option, opt_str, value, parser):
-        """Function to insure that the port we are passed is actually 
+        """Function to insure that the port we are passed is actually
         a valid port number. Used by OptionParser() on startup."""
         intval = int(value)
         if (intval < 0) or (intval > 65535):

src/bin/resolver/resolver_log.h

@@ -23,20 +23,20 @@
 /// Defines the levels used to output debug messages in the resolver.  Note that
 /// higher numbers equate to more verbose (and detailed) output.
 
-// Initialization
-const int RESOLVER_DBG_INIT = 10;
+// Initialization and shutdown of the resolver.
+const int RESOLVER_DBG_INIT = DBGLVL_START_SHUT;
 
 // Configuration messages
-const int RESOLVER_DBG_CONFIG = 30;
+const int RESOLVER_DBG_CONFIG = DBGLVL_COMMAND;
 
 // Trace sending and receiving of messages
-const int RESOLVER_DBG_IO = 50;
+const int RESOLVER_DBG_IO = DBGLVL_TRACE_BASIC;
 
 // Trace processing of messages
-const int RESOLVER_DBG_PROCESS = 70;
+const int RESOLVER_DBG_PROCESS = DBGLVL_TRACE_DETAIL;
 
 // Detailed message information
-const int RESOLVER_DBG_DETAIL = 90;
+const int RESOLVER_DBG_DETAIL = DBGLVL_TRACE_DETAIL_DATA;
 
 
 /// \brief Resolver Logger

src/bin/stats/stats.py.in

@@ -32,9 +32,8 @@ from isc.log_messages.stats_messages import *
 isc.log.init("b10-stats")
 logger = isc.log.Logger("stats")
 
-# Some constants for debug levels, these should be removed when we
-# have #1074
-DBG_STATS_MESSAGING = 30
+# Some constants for debug levels.
+DBG_STATS_MESSAGING = logger.DBGLVL_COMMAND
 
 # This is for boot_time of Stats
 _BASETIME = gmtime()

src/bin/stats/stats_httpd.py.in

@@ -40,10 +40,9 @@ from isc.log_messages.stats_httpd_messages import *
 isc.log.init("b10-stats-httpd")
 logger = isc.log.Logger("stats-httpd")
 
-# Some constants for debug levels, these should be removed when we
-# have #1074
-DBG_STATHTTPD_INIT = 10
-DBG_STATHTTPD_MESSAGING = 30
+# Some constants for debug levels.
+DBG_STATHTTPD_INIT = logger.DBGLVL_START_SHUT
+DBG_STATHTTPD_MESSAGING = logger.DBGLVL_COMMAND
 
 # If B10_FROM_SOURCE is set in the environment, we use data files
 # from a directory relative to that, otherwise we use the ones

src/bin/xfrin/tests/xfrin_test.py

@@ -16,6 +16,7 @@
 import unittest
 import shutil
 import socket
+import sys
 import io
 from isc.testutils.tsigctx_mock import MockTSIGContext
 from xfrin import *
@@ -216,8 +217,8 @@ class MockXfrin(Xfrin):
                                  request_type, check_soa)
 
 class MockXfrinConnection(XfrinConnection):
-    def __init__(self, sock_map, zone_name, rrclass, shutdown_event,
-                 master_addr):
+    def __init__(self, sock_map, zone_name, rrclass, datasrc_client,
+                 shutdown_event, master_addr, tsig_key=None):
         super().__init__(sock_map, zone_name, rrclass, MockDataSourceClient(),
                          shutdown_event, master_addr)
         self.query_data = b''
@@ -300,8 +301,9 @@ class TestXfrinState(unittest.TestCase):
     def setUp(self):
         self.sock_map = {}
         self.conn = MockXfrinConnection(self.sock_map, TEST_ZONE_NAME,
-                                        TEST_RRCLASS, threading.Event(),
+                                        TEST_RRCLASS, None, threading.Event(),
                                         TEST_MASTER_IPV4_ADDRINFO)
+        self.conn.init_socket()
         self.begin_soa = RRset(TEST_ZONE_NAME, TEST_RRCLASS, RRType.SOA(),
                                RRTTL(3600))
         self.begin_soa.add_rdata(Rdata(RRType.SOA(), TEST_RRCLASS,
@@ -585,8 +587,9 @@ class TestXfrinConnection(unittest.TestCase):
             os.remove(TEST_DB_FILE)
         self.sock_map = {}
         self.conn = MockXfrinConnection(self.sock_map, TEST_ZONE_NAME,
-                                        TEST_RRCLASS, threading.Event(),
+                                        TEST_RRCLASS, None, threading.Event(),
                                         TEST_MASTER_IPV4_ADDRINFO)
+        self.conn.init_socket()
         self.soa_response_params = {
             'questions': [example_soa_question],
             'bad_qid': False,
@@ -720,14 +723,16 @@ class TestAXFR(TestXfrinConnection):
         # to confirm an AF_INET6 socket has been created.  A naive application
         # tends to assume it's IPv4 only and hardcode AF_INET.  This test
         # uncovers such a bug.
-        c = MockXfrinConnection({}, TEST_ZONE_NAME, TEST_RRCLASS,
+        c = MockXfrinConnection({}, TEST_ZONE_NAME, TEST_RRCLASS, None,
                                 threading.Event(), TEST_MASTER_IPV6_ADDRINFO)
+        c.init_socket()
         c.bind(('::', 0))
         c.close()
 
     def test_init_chclass(self):
-        c = MockXfrinConnection({}, TEST_ZONE_NAME, RRClass.CH(),
+        c = MockXfrinConnection({}, TEST_ZONE_NAME, RRClass.CH(), None,
                                 threading.Event(), TEST_MASTER_IPV4_ADDRINFO)
+        c.init_socket()
         axfrmsg = c._create_query(RRType.AXFR())
         self.assertEqual(axfrmsg.get_question()[0].get_class(),
                          RRClass.CH())
@@ -1679,6 +1684,110 @@ class TestXfrinRecorder(unittest.TestCase):
         self.recorder.decrement(TEST_ZONE_NAME)
         self.assertEqual(self.recorder.xfrin_in_progress(TEST_ZONE_NAME), False)
 
+class TestXfrinProcess(unittest.TestCase):
+    def setUp(self):
+        self.unlocked = False
+        self.conn_closed = False
+        self.do_raise_on_close = False
+        self.do_raise_on_connect = False
+        self.do_raise_on_publish = False
+        self.master = (socket.AF_INET, socket.SOCK_STREAM,
+                       (TEST_MASTER_IPV4_ADDRESS, TEST_MASTER_PORT))
+
+    def tearDown(self):
+        # whatever happens the lock acquired in xfrin_recorder.increment
+        # must always be released.  We checked the condition for all test
+        # cases.
+        self.assertTrue(self.unlocked)
+
+        # Same for the connection
+        self.assertTrue(self.conn_closed)
+
+    def increment(self, zone_name):
+        '''Fake method of xfrin_recorder.increment.
+
+        '''
+        self.unlocked = False
+
+    def decrement(self, zone_name):
+        '''Fake method of xfrin_recorder.decrement.
+
+        '''
+        self.unlocked = True
+
+    def publish_xfrin_news(self, zone_name, rrclass, ret):
+        '''Fake method of serve.publish_xfrin_news
+
+        '''
+        if self.do_raise_on_publish:
+            raise XfrinTestException('Emulated exception in publish')
+
+    def connect_to_master(self, conn):
+        self.sock_fd = conn.fileno()
+        if self.do_raise_on_connect:
+            raise XfrinTestException('Emulated exception in connect')
+        return True
+
+    def conn_close(self, conn):
+        self.conn_closed = True
+        XfrinConnection.close(conn)
+        if self.do_raise_on_close:
+            raise XfrinTestException('Emulated exception in connect')
+
+    def create_xfrinconn(self, sock_map, zone_name, rrclass, datasrc_client,
+                         shutdown_event, master_addrinfo, tsig_key):
+        conn = MockXfrinConnection(sock_map, zone_name, rrclass,
+                                   datasrc_client, shutdown_event,
+                                   master_addrinfo, tsig_key)
+
+        # An awkward check that would specifically identify an old bug
+        # where initialziation of XfrinConnection._tsig_ctx_creator caused
+        # self reference and subsequently led to reference leak.
+        orig_ref = sys.getrefcount(conn)
+        conn._tsig_ctx_creator = None
+        self.assertEqual(orig_ref, sys.getrefcount(conn))
+
+        # Replace some methods for connect with our internal ones for the
+        # convenience of tests
+        conn.connect_to_master = lambda : self.connect_to_master(conn)
+        conn.do_xfrin = lambda x, y : XFRIN_OK
+        conn.close = lambda : self.conn_close(conn)
+
+        return conn
+
+    def test_process_xfrin_normal(self):
+        # Normal, successful case.  We only check that things are cleaned up
+        # at the tearDown time.
+        process_xfrin(self, self, TEST_ZONE_NAME, TEST_RRCLASS, None, None,
+                      self.master,  False, None, RRType.AXFR(),
+                      self.create_xfrinconn)
+
+    def test_process_xfrin_exception_on_connect(self):
+        # connect_to_master() will raise an exception.  Things must still be
+        # cleaned up.
+        self.do_raise_on_connect = True
+        process_xfrin(self, self, TEST_ZONE_NAME, TEST_RRCLASS, None, None,
+                      self.master,  False, None, RRType.AXFR(),
+                      self.create_xfrinconn)
+
+    def test_process_xfrin_exception_on_close(self):
+        # connect() will result in exception, and even the cleanup close()
+        # will fail with an exception.  This should be quite likely a bug,
+        # but we deal with that case.
+        self.do_raise_on_connect = True
+        self.do_raise_on_close = True
+        process_xfrin(self, self, TEST_ZONE_NAME, TEST_RRCLASS, None, None,
+                      self.master,  False, None, RRType.AXFR(),
+                      self.create_xfrinconn)
+
+    def test_process_xfrin_exception_on_publish(self):
+        # xfr succeeds but notifying the zonemgr fails with exception.
+        # everything must still be cleaned up.
+        self.do_raise_on_publish = True
+        process_xfrin(self, self, TEST_ZONE_NAME, TEST_RRCLASS, None, None,
+                      self.master,  False, None, RRType.AXFR(),
+                      self.create_xfrinconn)
+
 class TestXfrin(unittest.TestCase):
     def setUp(self):
         # redirect output

src/bin/xfrin/xfrin.py.in

@@ -64,8 +64,8 @@ ZONE_MANAGER_MODULE_NAME = 'Zonemgr'
 REFRESH_FROM_ZONEMGR = 'refresh_from_zonemgr'
 ZONE_XFRIN_FAILED = 'zone_xfrin_failed'
 
-# Constants for debug levels, to be removed when we have #1074.
-DBG_XFRIN_TRACE = 3
+# Constants for debug levels.
+DBG_XFRIN_TRACE = logger.DBGLVL_TRACE_BASIC
 
 # These two default are currently hard-coded. For config this isn't
 # necessary, but we need these defaults for optional command arguments
@@ -323,6 +323,7 @@ class XfrinFirstData(XfrinState):
                  conn.zone_str())
             # We are now going to add RRs to the new zone.  We need create
             # a Diff object.  It will be used throughtout the XFR session.
+            # DISABLE FOR DEBUG
             conn._diff = Diff(conn._datasrc_client, conn._zone_name, True)
             self.set_xfrstate(conn, XfrinAXFR())
         return False
@@ -468,21 +469,27 @@ class XfrinConnection(asyncore.dispatcher):
         # Data source handler
         self._datasrc_client = datasrc_client
 
-        self.create_socket(master_addrinfo[0], master_addrinfo[1])
         self._sock_map = sock_map
         self._soa_rr_count = 0
         self._idle_timeout = idle_timeout
-        self.setblocking(1)
         self._shutdown_event = shutdown_event
-        self._master_address = master_addrinfo[2]
+        self._master_addrinfo = master_addrinfo
         self._tsig_key = tsig_key
         self._tsig_ctx = None
         # tsig_ctx_creator is introduced to allow tests to use a mock class for
         # easier tests (in normal case we always use the default)
-        self._tsig_ctx_creator = self.__create_tsig_ctx
+        self._tsig_ctx_creator = lambda key : TSIGContext(key)
 
-    def __create_tsig_ctx(self, key):
-        return TSIGContext(key)
+    def init_socket(self):
+        '''Initialize the underlyig socket.
+
+        This is essentially a part of __init__() and is expected to be
+        called immediately after the constructor.  It's separated from
+        the constructor because otherwise we might not be able to close
+        it if the constructor raises an exception after opening the socket.
+        '''
+        self.create_socket(self._master_addrinfo[0], self._master_addrinfo[1])
+        self.setblocking(1)
 
     def __set_xfrstate(self, new_state):
         self.__state = new_state
@@ -498,10 +505,11 @@ class XfrinConnection(asyncore.dispatcher):
         '''Connect to master in TCP.'''
 
         try:
-            self.connect(self._master_address)
+            self.connect(self._master_addrinfo[2])
             return True
         except socket.error as e:
-            logger.error(XFRIN_CONNECT_MASTER, self._master_address, str(e))
+            logger.error(XFRIN_CONNECT_MASTER, self._master_addrinfo[2],
+                         str(e))
             return False
 
     def _get_zone_soa(self):
@@ -697,7 +705,6 @@ class XfrinConnection(asyncore.dispatcher):
             # (if not yet - possible in case of xfr-level exception) as soon
             # as possible
             self._diff = None
-            self.close()
 
         return ret
 
@@ -730,33 +737,6 @@ class XfrinConnection(asyncore.dispatcher):
         if msg.get_rr_count(Message.SECTION_QUESTION) > 1:
             raise XfrinException('query section count greater than 1')
 
-    def _handle_answer_section(self, answer_section):
-        '''Return a generator for the reponse in one tcp package to a zone transfer.'''
-
-        for rrset in answer_section:
-            rrset_name = rrset.get_name().to_text()
-            rrset_ttl = int(rrset.get_ttl().to_text())
-            rrset_class = rrset.get_class().to_text()
-            rrset_type = rrset.get_type().to_text()
-
-            for rdata in rrset.get_rdata():
-                # Count the soa record count
-                if rrset.get_type() == RRType.SOA():
-                    self._soa_rr_count += 1
-
-                    # XXX: the current DNS message parser can't preserve the
-                    # RR order or separete the beginning and ending SOA RRs.
-                    # As a short term workaround, we simply ignore the second
-                    # SOA, and ignore the erroneous case where the transfer
-                    # session doesn't end with an SOA.
-                    if (self._soa_rr_count == 2):
-                        # Avoid inserting soa record twice
-                        break
-
-                rdata_text = rdata.to_text()
-                yield (rrset_name, rrset_ttl, rrset_class, rrset_type,
-                       rdata_text)
-
     def _handle_xfrin_responses(self):
         read_next_msg = True
         while read_next_msg:
@@ -794,47 +774,82 @@ class XfrinConnection(asyncore.dispatcher):
 
         return False
 
-    def log_info(self, msg, type='info'):
-        # Overwrite the log function, log nothing
-        pass
-
-def process_xfrin(server, xfrin_recorder, zone_name, rrclass, db_file,
-                  shutdown_event, master_addrinfo, check_soa, tsig_key,
-                  request_type):
-    xfrin_recorder.increment(zone_name)
-
-    # Create a data source client used in this XFR session.  Right now we
-    # still assume an sqlite3-based data source, and use both the old and new
-    # data source APIs.  We also need to use a mock client for tests.
-    # For a temporary workaround to deal with these situations, we skip the
-    # creation when the given file is none (the test case).  Eventually
-    # this code will be much cleaner.
-    datasrc_client = None
-    if db_file is not None:
-        # temporary hardcoded sqlite initialization. Once we decide on
-        # the config specification, we need to update this (TODO)
-        # this may depend on #1207, or any followup ticket created for #1207
-        datasrc_type = "sqlite3"
-        datasrc_config = "{ \"database_file\": \"" + db_file + "\"}"
-        datasrc_client = DataSourceClient(datasrc_type, datasrc_config)
-
-    # Create a TCP connection for the XFR session and perform the operation.
-    sock_map = {}
-    conn = XfrinConnection(sock_map, zone_name, rrclass, datasrc_client,
-                           shutdown_event, master_addrinfo, tsig_key)
-    # XXX: We still need _db_file for temporary workaround in _create_query().
-    # This should be removed when we eliminate the need for the workaround.
-    conn._db_file = db_file
+def __process_xfrin(server, zone_name, rrclass, db_file,
+                    shutdown_event, master_addrinfo, check_soa, tsig_key,
+                    request_type, conn_class=XfrinConnection):
+    conn = None
+    exception = None
     ret = XFRIN_FAIL
-    if conn.connect_to_master():
-        ret = conn.do_xfrin(check_soa, request_type)
+    try:
+        # Create a data source client used in this XFR session.  Right now we
+        # still assume an sqlite3-based data source, and use both the old and
+        # new data source APIs.  We also need to use a mock client for tests.
+        # For a temporary workaround to deal with these situations, we skip the
+        # creation when the given file is none (the test case).  Eventually
+        # this code will be much cleaner.
+        datasrc_client = None
+        if db_file is not None:
+            # temporary hardcoded sqlite initialization. Once we decide on
+            # the config specification, we need to update this (TODO)
+            # this may depend on #1207, or any followup ticket created for #1207
+            datasrc_type = "sqlite3"
+            datasrc_config = "{ \"database_file\": \"" + db_file + "\"}"
+            datasrc_client = DataSourceClient(datasrc_type, datasrc_config)
+
+        # Create a TCP connection for the XFR session and perform the operation
+        sock_map = {}
+        conn = conn_class(sock_map, zone_name, rrclass, datasrc_client,
+                          shutdown_event, master_addrinfo, tsig_key)
+        conn.init_socket()
+        # XXX: We still need _db_file for temporary workaround in _create_query().
+        # This should be removed when we eliminate the need for the workaround.
+        conn._db_file = db_file
+        if conn.connect_to_master():
+            ret = conn.do_xfrin(check_soa, request_type)
+    except Exception as ex:
+        # If exception happens, just remember it here so that we can re-raise
+        # after cleaning up things.  We don't log it here because we want
+        # eliminate smallest possibility of having an exception in logging
+        # itself.
+        exception = ex
+
+    # asyncore.dispatcher requires explicit close() unless its lifetime
+    # from born to destruction is closed within asyncore.loop, which is not
+    # the case for us.  We always close() here, whether or not do_xfrin
+    # succeeds, and even when we see an unexpected exception.
+    if conn is not None:
+        conn.close()
 
     # Publish the zone transfer result news, so zonemgr can reset the
     # zone timer, and xfrout can notify the zone's slaves if the result
     # is success.
     server.publish_xfrin_news(zone_name, rrclass, ret)
+
+    if exception is not None:
+        raise exception
+
+def process_xfrin(server, xfrin_recorder, zone_name, rrclass, db_file,
+                  shutdown_event, master_addrinfo, check_soa, tsig_key,
+                  request_type, conn_class=XfrinConnection):
+    # Even if it should be rare, the main process of xfrin session can
+    # raise an exception.  In order to make sure the lock in xfrin_recorder
+    # is released in any cases, we delegate the main part to the helper
+    # function in the try block, catch any exceptions, then release the lock.
+    xfrin_recorder.increment(zone_name)
+    exception = None
+    try:
+        __process_xfrin(server, zone_name, rrclass, db_file,
+                        shutdown_event, master_addrinfo, check_soa, tsig_key,
+                        request_type, conn_class)
+    except Exception as ex:
+        # don't log it until we complete decrement().
+        exception = ex
     xfrin_recorder.decrement(zone_name)
 
+    if exception is not None:
+        typestr = "AXFR" if request_type == RRType.AXFR() else "IXFR"
+        logger.error(XFRIN_XFR_PROCESS_FAILURE, typestr, zone_name.to_text(),
+                     str(rrclass), str(exception))
 
 class XfrinRecorder:
     def __init__(self):

src/bin/xfrin/xfrin_messages.mes

@@ -29,6 +29,21 @@ this can only happen for AXFR.
 The XFR transfer for the given zone has failed due to a protocol error.
 The error is shown in the log message.
 
+% XFRIN_XFR_PROCESS_FAILURE %1 transfer of zone %2/%3 failed: %4
+An XFR session failed outside the main protocol handling.  This
+includes an error at the data source level at the initialization
+phase, unexpected failure in the network connection setup to the
+master server, or even more unexpected failure due to unlikely events
+such as memory allocation failure.  Details of the error are shown in
+the log message.  In general, these errors are not really expected
+ones, and indicate an installation error or a program bug.  The
+session handler thread tries to clean up all intermediate resources
+even on these errors, but it may be incomplete.  So, if this log
+message continuously appears, system resource consumption should be
+checked, and you may even want to disable the corresponding transfers.
+You may also want to file a bug report if this message appears so
+often.
+
 % XFRIN_XFR_TRANSFER_STARTED %1 transfer of zone %2 started
 A connection to the master server has been made, the serial value in
 the SOA record has been checked, and a zone transfer has been started.

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

@@ -922,7 +922,7 @@ class TestInitialization(unittest.TestCase):
         self.setEnv("BIND10_XFROUT_SOCKET_FILE", None)
         xfrout.init_paths()
         self.assertEqual(xfrout.UNIX_SOCKET_FILE,
-                         "@@LOCALSTATEDIR@@/auth_xfrout_conn")
+                         "@@LOCALSTATEDIR@@/@PACKAGE_NAME@/auth_xfrout_conn")
 
     def testProvidedSocket(self):
         self.setEnv("B10_FROM_BUILD", None)

src/bin/xfrout/xfrout.py.in

@@ -85,7 +85,7 @@ def init_paths():
         if "BIND10_XFROUT_SOCKET_FILE" in os.environ:
             UNIX_SOCKET_FILE = os.environ["BIND10_XFROUT_SOCKET_FILE"]
         else:
-            UNIX_SOCKET_FILE = "@@LOCALSTATEDIR@@/auth_xfrout_conn"
+            UNIX_SOCKET_FILE = "@@LOCALSTATEDIR@@/@PACKAGE_NAME@/auth_xfrout_conn"
 
 init_paths()
 

src/bin/zonemgr/zonemgr.py.in

@@ -43,10 +43,10 @@ from isc.log_messages.zonemgr_messages import *
 isc.log.init("b10-zonemgr")
 logger = isc.log.Logger("zonemgr")
 
-# Constants for debug levels, to be removed when we have #1074.
-DBG_START_SHUT = 0
-DBG_ZONEMGR_COMMAND = 10
-DBG_ZONEMGR_BASIC = 40
+# Constants for debug levels.
+DBG_START_SHUT = logger.DBGLVL_START_SHUT
+DBG_ZONEMGR_COMMAND = logger.DBGLVL_COMMAND
+DBG_ZONEMGR_BASIC = logger.DBGLVL_TRACE_BASIC
 
 isc.util.process.rename()
 

src/lib/asiodns/io_fetch.cc

@@ -61,17 +61,13 @@ namespace asiodns {
 
 /// Use the ASIO logger
 
-namespace {
-
 isc::log::Logger logger("asiolink");
+
 // Log debug verbosity
-enum {
-    DBG_IMPORTANT = 1,
-    DBG_COMMON = 20,
-    DBG_ALL = 50
-};
 
-}
+const int DBG_IMPORTANT = DBGLVL_TRACE_BASIC;
+const int DBG_COMMON = DBGLVL_TRACE_DETAIL;
+const int DBG_ALL = DBGLVL_TRACE_DETAIL + 20;
 
 /// \brief IOFetch Data
 ///

src/lib/cache/logger.h

@@ -31,14 +31,13 @@ namespace cache {
 /// \brief The logger for this library
 extern isc::log::Logger logger;
 
-enum {
-    /// \brief Trace basic operations
-    DBG_TRACE_BASIC = 10,
-    /// \brief Trace data operations
-    DBG_TRACE_DATA = 40,
-};
-
-}
-}
+/// \brief Trace basic operations
+const int DBG_TRACE_BASIC = DBGLVL_TRACE_BASIC;
+
+/// \brief Trace data operations
+const int DBG_TRACE_DATA = DBGLVL_TRACE_BASIC_DATA;
+
+} // namespace cache
+} // namespace isc
 
 #endif

src/lib/cc/logger.h

@@ -28,20 +28,19 @@
 namespace isc {
 namespace cc {
 
-enum {
-    /// \brief Trace basic operation
-    DBG_TRACE_BASIC = 10,
-    /// \brief Trace even details
-    ///
-    /// This includes messages being sent and received, waiting for messages
-    /// and alike.
-    DBG_TRACE_DETAILED = 80
-};
+/// Trace basic operation
+const int DBG_TRACE_BASIC = DBGLVL_TRACE_BASIC;
 
-/// \brief Logger for this library
+/// This includes messages being sent and received, waiting for messages
+/// and alike.
+const int DBG_TRACE_DETAILED = DBGLVL_TRACE_DETAIL;
+
+// Declaration of the logger.
 extern isc::log::Logger logger;
 
-}
-}
+} // namespace cc
+} // namespace isc
+
+/// \brief Logger for this library
 
 #endif

src/lib/config/config_log.h

@@ -30,15 +30,10 @@ namespace config {
 /// Define the logger used to log messages.  We could define it in multiple
 /// modules, but defining in a single module and linking to it saves time and
 /// space.
-extern isc::log::Logger config_logger;    // isc::config::config_logger is the CONFIG logger
+extern isc::log::Logger config_logger;
 
-/// \brief Debug Levels
-///
-/// Debug levels used in the configuration library
-enum {
-    DBG_CONFIG_PROCESS = 40     // Enumerate configuration elements as they
-                                // ... are processed.
-};
+// Enumerate configuration elements as they are processed.
+const int DBG_CONFIG_PROCESS = DBGLVL_TRACE_BASIC;
 
 } // namespace config
 } // namespace isc

src/lib/datasrc/logger.h

@@ -31,14 +31,14 @@ namespace datasrc {
 /// \brief The logger for this library
 extern isc::log::Logger logger;
 
-enum {
-    /// \brief Trace basic operations
-    DBG_TRACE_BASIC = 10,
-    /// \brief Trace data changes and lookups as well
-    DBG_TRACE_DATA = 20,
-    /// \brief Detailed even about how the lookups happen
-    DBG_TRACE_DETAILED = 50
-};
+/// \brief Trace basic operations
+const int DBG_TRACE_BASIC = DBGLVL_TRACE_BASIC;
+
+/// \brief Trace data changes and lookups as well
+const int DBG_TRACE_DATA = DBGLVL_TRACE_BASIC_DATA;
+
+/// \brief Detailed even about how the lookups happen
+const int DBG_TRACE_DETAILED = DBGLVL_TRACE_DETAIL;
 
 }
 }

src/lib/dhcp/Makefile.am

@@ -14,8 +14,9 @@ libdhcp_la_SOURCES += option.cc option.h
 libdhcp_la_SOURCES += option6_ia.cc option6_ia.h
 libdhcp_la_SOURCES += option6_iaaddr.cc option6_iaaddr.h
 libdhcp_la_SOURCES += option6_addrlst.cc option6_addrlst.h
-libdhcp_la_SOURCES += dhcp6.h
+libdhcp_la_SOURCES += dhcp6.h dhcp4.h
 libdhcp_la_SOURCES += pkt6.cc pkt6.h
+libdhcp_la_SOURCES += pkt4.cc pkt4.h
 
 EXTRA_DIST  = README
 #EXTRA_DIST += log_messages.mes

src/lib/dhcp/dhcp4.h

@@ -0,0 +1,191 @@
+/*
+ * Copyright (c) 2004-2011 by Internet Systems Consortium, Inc. ("ISC")
+ * Copyright (c) 1995-2003 by Internet Software Consortium
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND 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.
+ *
+ *   Internet Systems Consortium, Inc.
+ *   950 Charter Street
+ *   Redwood City, CA 94063
+ *   <info@isc.org>
+ *   https://www.isc.org/
+ *
+ * This software has been written for Internet Systems Consortium
+ * by Ted Lemon in cooperation with Vixie Enterprises.  To learn more
+ * about Internet Systems Consortium, see ``https://www.isc.org''.
+ * To learn more about Vixie Enterprises, see ``http://www.vix.com''.
+ */
+
+/*
+ * NOTE: This files is imported from ISC DHCP. It uses C notation.
+ *       Format kept for easier merge.
+ */
+
+#ifndef DHCP_H
+#define DHCP_H
+
+#include <stdint.h>
+
+namespace isc {
+namespace dhcp {
+
+/* BOOTP (rfc951) message types */
+enum BOOTPTypes {
+    BOOTREQUEST = 1,
+    BOOTREPLY = 2
+};
+
+/* Possible values for flags field... */
+static const uint16_t BOOTP_BROADCAST = 32768L;
+
+/* Possible values for hardware type (htype) field... */
+enum HType {
+    HTYPE_ETHER = 1,   /* Ethernet 10Mbps */
+    HTYPE_IEEE802 = 6, /* IEEE 802.2 Token Ring */
+    HTYPE_FDDI = 8     /* FDDI */
+    /// TODO Add infiniband here
+};
+
+/* DHCP Option codes: */
+enum DHCPOptionType {
+    DHO_PAD                          = 0,
+    DHO_SUBNET_MASK                  = 1,
+    DHO_TIME_OFFSET                  = 2,
+    DHO_ROUTERS                      = 3,
+    DHO_TIME_SERVERS                 = 4,
+    DHO_NAME_SERVERS                 = 5,
+    DHO_DOMAIN_NAME_SERVERS          = 6,
+    DHO_LOG_SERVERS                  = 7,
+    DHO_COOKIE_SERVERS               = 8,
+    DHO_LPR_SERVERS                  = 9,
+    DHO_IMPRESS_SERVERS              = 10,
+    DHO_RESOURCE_LOCATION_SERVERS    = 11,
+    DHO_HOST_NAME                    = 12,
+    DHO_BOOT_SIZE                    = 13,
+    DHO_MERIT_DUMP                   = 14,
+    DHO_DOMAIN_NAME                  = 15,
+    DHO_SWAP_SERVER                  = 16,
+    DHO_ROOT_PATH                    = 17,
+    DHO_EXTENSIONS_PATH              = 18,
+    DHO_IP_FORWARDING                = 19,
+    DHO_NON_LOCAL_SOURCE_ROUTING     = 20,
+    DHO_POLICY_FILTER                = 21,
+    DHO_MAX_DGRAM_REASSEMBLY         = 22,
+    DHO_DEFAULT_IP_TTL               = 23,
+    DHO_PATH_MTU_AGING_TIMEOUT       = 24,
+    DHO_PATH_MTU_PLATEAU_TABLE       = 25,
+    DHO_INTERFACE_MTU                = 26,
+    DHO_ALL_SUBNETS_LOCAL            = 27,
+    DHO_BROADCAST_ADDRESS            = 28,
+    DHO_PERFORM_MASK_DISCOVERY       = 29,
+    DHO_MASK_SUPPLIER                = 30,
+    DHO_ROUTER_DISCOVERY             = 31,
+    DHO_ROUTER_SOLICITATION_ADDRESS  = 32,
+    DHO_STATIC_ROUTES                = 33,
+    DHO_TRAILER_ENCAPSULATION        = 34,
+    DHO_ARP_CACHE_TIMEOUT            = 35,
+    DHO_IEEE802_3_ENCAPSULATION      = 36,
+    DHO_DEFAULT_TCP_TTL              = 37,
+    DHO_TCP_KEEPALIVE_INTERVAL       = 38,
+    DHO_TCP_KEEPALIVE_GARBAGE        = 39,
+    DHO_NIS_DOMAIN                   = 40,
+    DHO_NIS_SERVERS                  = 41,
+    DHO_NTP_SERVERS                  = 42,
+    DHO_VENDOR_ENCAPSULATED_OPTIONS  = 43,
+    DHO_NETBIOS_NAME_SERVERS         = 44,
+    DHO_NETBIOS_DD_SERVER            = 45,
+    DHO_NETBIOS_NODE_TYPE            = 46,
+    DHO_NETBIOS_SCOPE                = 47,
+    DHO_FONT_SERVERS                 = 48,
+    DHO_X_DISPLAY_MANAGER            = 49,
+    DHO_DHCP_REQUESTED_ADDRESS       = 50,
+    DHO_DHCP_LEASE_TIME              = 51,
+    DHO_DHCP_OPTION_OVERLOAD         = 52,
+    DHO_DHCP_MESSAGE_TYPE            = 53,
+    DHO_DHCP_SERVER_IDENTIFIER       = 54,
+    DHO_DHCP_PARAMETER_REQUEST_LIST  = 55,
+    DHO_DHCP_MESSAGE                 = 56,
+    DHO_DHCP_MAX_MESSAGE_SIZE        = 57,
+    DHO_DHCP_RENEWAL_TIME            = 58,
+    DHO_DHCP_REBINDING_TIME          = 59,
+    DHO_VENDOR_CLASS_IDENTIFIER      = 60,
+    DHO_DHCP_CLIENT_IDENTIFIER       = 61,
+    DHO_NWIP_DOMAIN_NAME             = 62,
+    DHO_NWIP_SUBOPTIONS              = 63,
+    DHO_USER_CLASS                   = 77,
+    DHO_FQDN                         = 81,
+    DHO_DHCP_AGENT_OPTIONS           = 82,
+    DHO_AUTHENTICATE                 = 90,  /* RFC3118, was 210 */
+    DHO_CLIENT_LAST_TRANSACTION_TIME = 91,
+    DHO_ASSOCIATED_IP                = 92,
+    DHO_SUBNET_SELECTION             = 118, /* RFC3011! */
+    DHO_DOMAIN_SEARCH                = 119, /* RFC3397 */
+    DHO_VIVCO_SUBOPTIONS             = 124,
+    DHO_VIVSO_SUBOPTIONS             = 125,
+
+    DHO_END                          = 255
+};
+
+/* DHCP message types. */
+enum DHCPMessageType {
+    DHCPDISCOVER        =  1,
+    DHCPOFFER           =  2,
+    DHCPREQUEST         =  3,
+    DHCPDECLINE         =  4,
+    DHCPACK             =  5,
+    DHCPNAK             =  6,
+    DHCPRELEASE         =  7,
+    DHCPINFORM          =  8,
+    DHCPLEASEQUERY      =  10,
+    DHCPLEASEUNASSIGNED =  11,
+    DHCPLEASEUNKNOWN    =  12,
+    DHCPLEASEACTIVE     =  13
+};
+
+static const uint16_t DHCP4_CLIENT_PORT = 68;
+static const uint16_t DHCP4_SERVER_PORT = 67;
+
+/// Magic cookie validating dhcp options field (and bootp vendor
+/// extensions field).
+///static const char* DHCP_OPTIONS_COOKIE = "\143\202\123\143";
+
+// TODO: Following are leftovers from dhcp.h import from ISC DHCP
+// They will be converted to C++-style defines once they will start
+// to be used.
+#if 0
+/* Relay Agent Information option subtypes: */
+#define RAI_CIRCUIT_ID  1
+#define RAI_REMOTE_ID   2
+#define RAI_AGENT_ID    3
+#define RAI_LINK_SELECT 5
+
+/* FQDN suboptions: */
+#define FQDN_NO_CLIENT_UPDATE           1
+#define FQDN_SERVER_UPDATE              2
+#define FQDN_ENCODED                    3
+#define FQDN_RCODE1                     4
+#define FQDN_RCODE2                     5
+#define FQDN_HOSTNAME                   6
+#define FQDN_DOMAINNAME                 7
+#define FQDN_FQDN                       8
+#define FQDN_SUBOPTION_COUNT            8
+
+/* Enterprise Suboptions: */
+#define VENDOR_ISC_SUBOPTIONS           2495
+
+#endif
+
+} // end of isc::dhcp namespace
+} // end of isc namespace
+
+#endif /* DHCP_H */

src/lib/dhcp/pkt4.cc

@@ -0,0 +1,189 @@
+// 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 <dhcp/pkt4.h>
+#include <dhcp/libdhcp.h>
+#include <dhcp/dhcp4.h>
+#include <exceptions/exceptions.h>
+#include <asiolink/io_address.h>
+#include <iostream>
+#include <sstream>
+
+using namespace std;
+using namespace isc::dhcp;
+using namespace isc::asiolink;
+
+namespace isc {
+namespace dhcp {
+
+const IOAddress DEFAULT_ADDRESS("0.0.0.0");
+
+Pkt4::Pkt4(uint8_t msg_type, uint32_t transid)
+     :local_addr_(DEFAULT_ADDRESS),
+      remote_addr_(DEFAULT_ADDRESS),
+      iface_(""),
+      ifindex_(0),
+      local_port_(DHCP4_SERVER_PORT),
+      remote_port_(DHCP4_CLIENT_PORT),
+      op_(DHCPTypeToBootpType(msg_type)),
+      htype_(HTYPE_ETHER),
+      hlen_(0),
+      hops_(0),
+      transid_(transid),
+      secs_(0),
+      flags_(0),
+      ciaddr_(DEFAULT_ADDRESS),
+      yiaddr_(DEFAULT_ADDRESS),
+      siaddr_(DEFAULT_ADDRESS),
+      giaddr_(DEFAULT_ADDRESS),
+      bufferIn_(0), // not used, this is TX packet
+      bufferOut_(DHCPV4_PKT_HDR_LEN),
+      msg_type_(msg_type)
+{
+    /// TODO: fixed fields, uncomment in ticket #1224
+    memset(chaddr_, 0, MAX_CHADDR_LEN);
+    memset(sname_, 0, MAX_SNAME_LEN);
+    memset(file_, 0, MAX_FILE_LEN);
+}
+
+Pkt4::Pkt4(const uint8_t* data, size_t len)
+     :local_addr_(DEFAULT_ADDRESS),
+      remote_addr_(DEFAULT_ADDRESS),
+      iface_(""),
+      ifindex_(-1),
+      local_port_(DHCP4_SERVER_PORT),
+      remote_port_(DHCP4_CLIENT_PORT),
+      /// TODO Fixed fields, uncomment in ticket #1224
+      op_(BOOTREQUEST),
+      transid_(0),
+      secs_(0),
+      flags_(0),
+      ciaddr_(DEFAULT_ADDRESS),
+      yiaddr_(DEFAULT_ADDRESS),
+      siaddr_(DEFAULT_ADDRESS),
+      giaddr_(DEFAULT_ADDRESS),
+      bufferIn_(0), // not used, this is TX packet
+      bufferOut_(DHCPV4_PKT_HDR_LEN),
+      msg_type_(DHCPDISCOVER)
+{
+    if (len < DHCPV4_PKT_HDR_LEN) {
+        isc_throw(OutOfRange, "Truncated DHCPv4 packet (len=" << len
+                  << " received, at least 236 bytes expected.");
+    }
+    bufferIn_.writeData(data, len);
+}
+
+size_t
+Pkt4::len() {
+    size_t length = DHCPV4_PKT_HDR_LEN; // DHCPv4 header
+
+    /// TODO: Include options here (ticket #1228)
+    return (length);
+}
+
+bool
+Pkt4::pack() {
+    /// TODO: Implement this (ticket #1227)
+
+    return (false);
+}
+bool
+Pkt4::unpack() {
+    /// TODO: Implement this (ticket #1226)
+
+    return (false);
+}
+
+std::string
+Pkt4::toText() {
+    stringstream tmp;
+    tmp << "localAddr=[" << local_addr_.toText() << "]:" << local_port_
+        << " remoteAddr=[" << remote_addr_.toText()
+        << "]:" << remote_port_ << endl;
+    tmp << "msgtype=" << msg_type_
+        << ", transid=0x" << hex << transid_ << dec
+        << endl;
+
+    return tmp.str();
+}
+
+void
+Pkt4::setHWAddr(uint8_t hType, uint8_t hlen,
+                const std::vector<uint8_t>& macAddr) {
+    /// TODO Rewrite this once support for client-identifier option
+    /// is implemented (ticket 1228?)
+    if (hlen>MAX_CHADDR_LEN) {
+        isc_throw(OutOfRange, "Hardware address (len=" << hlen
+                  << " too long. Max " << MAX_CHADDR_LEN << " supported.");
+    }
+    if ( (macAddr.size() == 0) && (hlen > 0) ) {
+        isc_throw(OutOfRange, "Invalid HW Address specified");
+    }
+
+    htype_ = hType;
+    hlen_ = hlen;
+    memset(chaddr_, 0, MAX_CHADDR_LEN);
+    memcpy(chaddr_, &macAddr[0], hlen);
+}
+
+void
+Pkt4::setSname(const uint8_t* sname, size_t snameLen /*= MAX_SNAME_LEN*/) {
+    if (snameLen > MAX_SNAME_LEN) {
+        isc_throw(OutOfRange, "sname field (len=" << snameLen
+                  << ") too long, Max " << MAX_SNAME_LEN << " supported.");
+    }
+    memset(sname_, 0, MAX_SNAME_LEN);
+    memcpy(sname_, sname, snameLen);
+
+    // no need to store snameLen as any empty space is filled with 0s
+}
+
+void
+Pkt4::setFile(const uint8_t* file, size_t fileLen /*= MAX_FILE_LEN*/) {
+    if (fileLen > MAX_FILE_LEN) {
+        isc_throw(OutOfRange, "file field (len=" << fileLen
+                  << ") too long, Max " << MAX_FILE_LEN << " supported.");
+    }
+    memset(file_, 0, MAX_FILE_LEN);
+    memcpy(file_, file, fileLen);
+
+    // no need to store fileLen as any empty space is filled with 0s
+}
+
+uint8_t
+Pkt4::DHCPTypeToBootpType(uint8_t dhcpType) {
+    switch (dhcpType) {
+    case DHCPDISCOVER:
+    case DHCPREQUEST:
+    case DHCPDECLINE:
+    case DHCPRELEASE:
+    case DHCPINFORM:
+    case DHCPLEASEQUERY:
+        return (BOOTREQUEST);
+    case DHCPACK:
+    case DHCPNAK:
+    case DHCPOFFER:
+    case DHCPLEASEUNASSIGNED:
+    case DHCPLEASEUNKNOWN:
+    case DHCPLEASEACTIVE:
+        return (BOOTREPLY);
+    default:
+        isc_throw(OutOfRange, "Invalid message type: "
+                  << static_cast<int>(dhcpType) );
+    }
+}
+
+} // end of namespace isc::dhcp
+
+} // end of namespace isc

src/lib/dhcp/pkt4.h

@@ -0,0 +1,380 @@
+// 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 PKT4_H
+#define PKT4_H
+
+#include <iostream>
+#include <vector>
+#include <boost/shared_ptr.hpp>
+#include <boost/shared_array.hpp>
+#include "asiolink/io_address.h"
+#include "util/buffer.h"
+#include "dhcp/option.h"
+
+namespace isc {
+
+namespace dhcp {
+
+class Pkt4 {
+public:
+
+    /// length of the CHADDR field in DHCPv4 message
+    const static size_t MAX_CHADDR_LEN = 16;
+
+    /// length of the SNAME field in DHCPv4 message
+    const static size_t MAX_SNAME_LEN = 64;
+
+    /// length of the FILE field in DHCPv4 message
+    const static size_t MAX_FILE_LEN = 128;
+
+    /// specifies DHCPv4 packet header length (fixed part)
+    const static size_t DHCPV4_PKT_HDR_LEN = 236;
+
+    /// Constructor, used in replying to a message.
+    ///
+    /// @param msg_type type of message (e.g. DHCPDISOVER=1)
+    /// @param transid transaction-id
+    Pkt4(uint8_t msg_type, uint32_t transid);
+
+    /// @brief Constructor, used in message reception.
+    ///
+    /// Creates new message. Pkt4 will copy data to bufferIn_
+    /// buffer on creation.
+    ///
+    /// @param data pointer to received data
+    /// @param len size of buffer to be allocated for this packet.
+    Pkt4(const uint8_t* data, size_t len);
+
+    /// @brief Prepares on-wire format of DHCPv4 packet.
+    ///
+    /// Prepares on-wire format of message and all its options.
+    /// Options must be stored in options_ field.
+    /// Output buffer will be stored in bufferOut_.
+    ///
+    /// @return true if packing procedure was successful
+    bool
+    pack();
+
+    /// @brief Parses on-wire form of DHCPv4 packet.
+    ///
+    /// Parses received packet, stored in on-wire format in bufferIn_.
+    ///
+    /// Will create a collection of option objects that will
+    /// be stored in options_ container.
+    ///
+    /// @return true, if parsing was successful
+    bool
+    unpack();
+
+    /// @brief Returns text representation of the packet.
+    ///
+    /// This function is useful mainly for debugging.
+    ///
+    /// @return string with text representation
+    std::string
+    toText();
+
+    /// @brief Returns the size of the required buffer to build the packet.
+    ///
+    /// Returns the size of the required buffer to build the packet with
+    /// the current set of packet options.
+    ///
+    /// @return number of bytes required to build this packet
+    size_t
+    len();
+
+    /// Sets hops field
+    ///
+    /// @param hops value to be set
+    void
+    setHops(uint8_t hops) { hops_ = hops; };
+
+    /// Returns hops field
+    ///
+    /// @return hops field
+    uint8_t
+    getHops() { return (hops_); };
+
+    // Note: There's no need to manipulate OP field directly,
+    // thus no setOp() method. See op_ comment.
+
+    /// Returns op field
+    ///
+    /// @return op field
+    uint8_t
+    getOp() { return (op_); };
+
+    /// Sets secs field
+    ///
+    /// @param secs value to be set
+    void
+    setSecs(uint16_t secs) { secs_ = secs; };
+
+    /// Returns secs field
+    ///
+    /// @return secs field
+    uint16_t
+    getSecs() { return (secs_); };
+
+    /// Sets flags field
+    ///
+    /// @param flags value to be set
+    void
+    setFlags(uint16_t flags) { flags_ = flags; };
+
+    /// Returns flags field
+    ///
+    /// @return flags field
+    uint16_t
+    getFlags() { return (flags_); };
+
+
+    /// Returns ciaddr field
+    ///
+    /// @return ciaddr field
+    isc::asiolink::IOAddress&
+    getCiaddr() { return (ciaddr_); };
+
+    /// Sets ciaddr field
+    ///
+    /// @param ciaddr value to be set
+    void
+    setCiaddr(const isc::asiolink::IOAddress& ciaddr) { ciaddr_ = ciaddr; };
+
+
+    /// Returns siaddr field
+    ///
+    /// @return siaddr field
+    isc::asiolink::IOAddress&
+    getSiaddr() { return (siaddr_); };
+
+    /// Sets siaddr field
+    ///
+    /// @param siaddr value to be set
+    void
+    setSiaddr(const isc::asiolink::IOAddress& siaddr) { siaddr_ = siaddr; };
+
+
+    /// Returns yiaddr field
+    ///
+    /// @return yiaddr field
+    isc::asiolink::IOAddress&
+    getYiaddr() { return (yiaddr_); };
+
+    /// Sets yiaddr field
+    ///
+    /// @param yiaddr value to be set
+    void
+    setYiaddr(const isc::asiolink::IOAddress& yiaddr) { yiaddr_ = yiaddr; };
+
+
+    /// Returns giaddr field
+    ///
+    /// @return giaddr field
+    isc::asiolink::IOAddress&
+    getGiaddr() { return (giaddr_); };
+
+    /// Sets giaddr field
+    ///
+    /// @param giaddr value to be set
+    void
+    setGiaddr(const isc::asiolink::IOAddress& giaddr) { giaddr_ = giaddr; };
+
+    /// Returns value of transaction-id field
+    ///
+    /// @return transaction-id
+    uint32_t getTransid() { return (transid_); };
+
+    /// Returns message type (e.g. 1 = DHCPDISCOVER)
+    ///
+    /// @return message type
+    uint8_t
+    getType() { return (msg_type_); }
+
+    /// Sets message type (e.g. 1 = DHCPDISCOVER)
+    ///
+    /// @param type message type to be set
+    void setType(uint8_t type) { msg_type_=type; };
+
+    /// @brief Returns sname field
+    ///
+    /// Note: This is 64 bytes long field. It doesn't have to be
+    /// null-terminated. Do not use strlen() or similar on it.
+    ///
+    /// @return sname field
+    const std::vector<uint8_t>
+    getSname() { return (std::vector<uint8_t>(sname_, &sname_[MAX_SNAME_LEN])); };
+
+    /// Sets sname field
+    ///
+    /// @param sname value to be set
+    void
+    setSname(const uint8_t* sname, size_t snameLen = MAX_SNAME_LEN);
+
+    /// @brief Returns file field
+    ///
+    /// Note: This is 128 bytes long field. It doesn't have to be
+    /// null-terminated. Do not use strlen() or similar on it.
+    ///
+    /// @return pointer to file field
+    const std::vector<uint8_t>
+    getFile() { return (std::vector<uint8_t>(file_, &file_[MAX_FILE_LEN])); };
+
+    /// Sets file field
+    ///
+    /// @param file value to be set
+    void
+    setFile(const uint8_t* file, size_t fileLen = MAX_FILE_LEN);
+
+    /// @brief Sets hardware address.
+    ///
+    /// Sets parameters of hardware address. hlen specifies
+    /// length of macAddr buffer. Content of macAddr buffer
+    /// will be copied to appropriate field.
+    ///
+    /// Note: macAddr must be a buffer of at least hlen bytes.
+    ///
+    /// @param hwType hardware type (will be sent in htype field)
+    /// @param hlen hardware length (will be sent in hlen field)
+    /// @param macAddr pointer to hardware address
+    void setHWAddr(uint8_t hType, uint8_t hlen,
+                   const std::vector<uint8_t>& macAddr);
+
+    /// Returns htype field
+    ///
+    /// @return hardware type
+    uint8_t
+    getHtype() { return (htype_); };
+
+    /// Returns hlen field
+    ///
+    /// @return hardware address length
+    uint8_t
+    getHlen() { return (hlen_); };
+
+    /// @brief Returns chaddr field
+    ///
+    /// Note: This is 16 bytes long field. It doesn't have to be
+    /// null-terminated. Do no use strlen() or similar on it.
+    ///
+    /// @return pointer to hardware address
+    const uint8_t*
+    getChaddr() { return (chaddr_); };
+
+
+protected:
+
+    /// converts DHCP message type to BOOTP op type
+    ///
+    /// @param dhcpType DHCP message type (e.g. DHCPDISCOVER)
+    ///
+    /// @return BOOTP type (BOOTREQUEST or BOOTREPLY)
+    uint8_t
+    DHCPTypeToBootpType(uint8_t dhcpType);
+
+    /// local address (dst if receiving packet, src if sending packet)
+    isc::asiolink::IOAddress local_addr_;
+
+    /// remote address (src if receiving packet, dst if sending packet)
+    isc::asiolink::IOAddress remote_addr_;
+
+    /// name of the network interface the packet was received/to be sent over
+    std::string iface_;
+
+    /// @brief interface index
+    ///
+    /// Each network interface has assigned unique ifindex. It is functional
+    /// equvalent of name, but sometimes more useful, e.g. when using crazy
+    /// systems that allow spaces in interface names e.g. MS Windows)
+    int ifindex_;
+
+    /// local UDP port
+    int local_port_;
+
+    /// remote UDP port
+    int remote_port_;
+
+    /// @brief message operation code
+    ///
+    /// Note: This is legacy BOOTP field. There's no need to manipulate it
+    /// directly. Its value is set based on DHCP message type. Note that
+    /// DHCPv4 protocol reuses BOOTP message format, so this field is
+    /// kept due to BOOTP format. This is NOT DHCPv4 type (DHCPv4 message
+    /// type is kept in message type option).
+    uint8_t op_;
+
+    /// link-layer address type
+    uint8_t htype_;
+
+    /// link-layer address length
+    uint8_t hlen_;
+
+    /// Number of relay agents traversed
+    uint8_t hops_;
+
+    /// DHCPv4 transaction-id (32 bits, not 24 bits as in DHCPv6)
+    uint32_t transid_;
+
+    /// elapsed (number of seconds since beginning of transmission)
+    uint16_t secs_;
+
+    /// flags
+    uint16_t flags_;
+
+    /// ciaddr field (32 bits): Client's IP address
+    isc::asiolink::IOAddress ciaddr_;
+
+    /// yiaddr field (32 bits): Client's IP address ("your"), set by server
+    isc::asiolink::IOAddress yiaddr_;
+
+    /// siaddr field (32 bits): next server IP address in boot process(e.g.TFTP)
+    isc::asiolink::IOAddress siaddr_;
+
+    /// giaddr field (32 bits): Gateway IP address
+    isc::asiolink::IOAddress giaddr_;
+
+    /// Hardware address field (16 bytes)
+    uint8_t chaddr_[MAX_CHADDR_LEN];
+
+    /// sname field (64 bytes)
+    uint8_t sname_[MAX_SNAME_LEN];
+
+    /// file field (128 bytes)
+    uint8_t file_[MAX_FILE_LEN];
+
+    // end of real DHCPv4 fields
+
+    /// input buffer (used during message reception)
+    /// Note that it must be modifiable as hooks can modify incoming buffer),
+    /// thus OutputBuffer, not InputBuffer
+    isc::util::OutputBuffer bufferIn_;
+
+    /// output buffer (used during message
+    isc::util::OutputBuffer bufferOut_;
+
+    /// message type (e.g. 1=DHCPDISCOVER)
+    /// TODO: this will eventually be replaced with DHCP Message Type
+    /// option (option 53)
+    uint8_t msg_type_;
+
+    /// collection of options present in this message
+    isc::dhcp::Option::Option4Collection options_;
+}; // Pkt4 class
+
+} // isc::dhcp namespace
+
+} // isc namespace
+
+#endif

src/lib/dhcp/pkt6.cc

@@ -124,7 +124,8 @@ Pkt6::packUDP() {
         cout << "Packet build failed:" << e.what() << endl;
         return (false);
     }
-    cout << "Packet built, len=" << len() << endl;
+    // Limited verbosity of this method
+    // cout << "Packet built, len=" << len() << endl;
     return (true);
 }
 

src/lib/dhcp/pkt6.h

@@ -19,7 +19,7 @@
 #include <boost/shared_ptr.hpp>
 #include <boost/shared_array.hpp>
 #include "asiolink/io_address.h"
-#include "option.h"
+#include "dhcp/option.h"
 
 namespace isc {
 

src/lib/dhcp/tests/Makefile.am

@@ -22,6 +22,7 @@ libdhcp_unittests_SOURCES += ../option6_ia.h ../option6_ia.cc option6_ia_unittes
 libdhcp_unittests_SOURCES += ../option6_addrlst.h ../option6_addrlst.cc option6_addrlst_unittest.cc
 libdhcp_unittests_SOURCES += ../option.h ../option.cc option_unittest.cc
 libdhcp_unittests_SOURCES += ../pkt6.h ../pkt6.cc pkt6_unittest.cc
+libdhcp_unittests_SOURCES += ../pkt4.h ../pkt4.cc pkt4_unittest.cc
 
 libdhcp_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES) $(LOG4CPLUS_INCLUDES)
 libdhcp_unittests_LDFLAGS  = $(AM_LDFLAGS)  $(GTEST_LDFLAGS)

src/lib/dhcp/tests/option6_ia_unittest.cc

@@ -220,7 +220,9 @@ TEST_F(Option6IATest, suboptions_unpack) {
     Option6IA* ia = 0;
     EXPECT_NO_THROW({
         ia = new Option6IA(D6O_IA_NA, buf, 128, 4, 44);
-        cout << "Parsed option:" << endl << ia->toText() << endl;
+
+        // let's limit verbosity of this test
+        // cout << "Parsed option:" << endl << ia->toText() << endl;
     });
     ASSERT_TRUE(ia);
 

src/lib/dhcp/tests/pkt4_unittest.cc

@@ -0,0 +1,432 @@
+// 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 <config.h>
+#include <iostream>
+#include <sstream>
+#include <arpa/inet.h>
+#include <gtest/gtest.h>
+#include <boost/static_assert.hpp>
+#include <boost/shared_ptr.hpp>
+#include <boost/shared_array.hpp>
+
+#include "io_address.h"
+#include "dhcp/pkt4.h"
+#include "dhcp/dhcp4.h"
+#include "exceptions/exceptions.h"
+
+using namespace std;
+using namespace isc;
+using namespace isc::asiolink;
+using namespace isc::dhcp;
+using namespace boost;
+
+// can't compare const to value directly, as it gives strange
+// linker errors in gtest.h
+
+static size_t DHCPV4_PKT_HDR_LEN = Pkt4::DHCPV4_PKT_HDR_LEN;
+
+namespace {
+
+TEST(Pkt4Test, constructor) {
+
+    ASSERT_EQ(236U, DHCPV4_PKT_HDR_LEN);
+    Pkt4* pkt = 0;
+
+    // minimal
+    uint8_t testData[250];
+    for (int i = 0; i < 250; i++) {
+        testData[i]=i;
+    }
+
+    // positive case1. Normal received packet
+    EXPECT_NO_THROW(
+        pkt = new Pkt4(testData, 236);
+    );
+
+    EXPECT_EQ(236, pkt->len());
+
+    EXPECT_NO_THROW(
+        delete pkt;
+        pkt = 0;
+    );
+
+    // positive case2. Normal outgoing packet
+    EXPECT_NO_THROW(
+        pkt = new Pkt4(DHCPDISCOVER, 0xffffffff);
+    );
+
+    // DHCPv4 packet must be at least 236 bytes long
+    EXPECT_EQ(DHCPV4_PKT_HDR_LEN, pkt->len());
+    EXPECT_EQ(DHCPDISCOVER, pkt->getType());
+    EXPECT_EQ(0xffffffff, pkt->getTransid());
+    EXPECT_NO_THROW(
+        delete pkt;
+        pkt = 0;
+    );
+
+    // negative case. Should drop truncated messages
+    EXPECT_THROW(
+        pkt = new Pkt4(testData, 235),
+        OutOfRange
+    );
+    if (pkt) {
+        // test failed. Exception should have been thrown, but
+        // object was created instead. Let's clean this up
+        delete pkt;
+    }
+}
+
+// a sample transaction-id
+const static uint32_t dummyTransid = 0x12345678;
+
+// a dummy MAC address
+const uint8_t dummyMacAddr[] = {0, 1, 2, 3, 4, 5};
+
+// a dummy MAC address, padded with 0s
+const uint8_t dummyChaddr[16] = {0, 1, 2, 3, 4, 5, 0, 0,
+                                 0, 0, 0, 0, 0, 0, 0, 0 };
+
+// let's use some creative test content here (128 chars + \0)
+const uint8_t dummyFile[] = "Lorem ipsum dolor sit amet, consectetur "
+    "adipiscing elit. Proin mollis placerat metus, at "
+    "lacinia orci ornare vitae. Mauris amet.";
+
+// yet another type of test content (64 chars + \0)
+const uint8_t dummySname[] = "Lorem ipsum dolor sit amet, consectetur "
+    "adipiscing elit posuere.";
+
+BOOST_STATIC_ASSERT(sizeof(dummyFile)  == Pkt4::MAX_FILE_LEN + 1);
+BOOST_STATIC_ASSERT(sizeof(dummySname) == Pkt4::MAX_SNAME_LEN + 1);
+
+/// Generates test packet
+///
+/// Allocates and generates test packet, with all fixed
+/// fields set to non-zero values. Content is not always
+/// reasonable.
+///
+/// See generateTestPacket2() function that returns
+/// exactly the same packet in on-wire format.
+///
+/// @return pointer to allocated Pkt4 object.
+boost::shared_ptr<Pkt4>
+generateTestPacket1() {
+
+    boost::shared_ptr<Pkt4> pkt(new Pkt4(DHCPDISCOVER, dummyTransid));
+
+    vector<uint8_t> vectorMacAddr(dummyMacAddr, dummyMacAddr
+                                  +sizeof(dummyMacAddr));
+
+    // hwType = 6(ETHERNET), hlen = 6(MAC address len)
+    pkt->setHWAddr(6, 6, vectorMacAddr);
+    pkt->setHops(13); // 13 relays. Wow!
+    // transaction-id is already set
+    pkt->setSecs(42);
+    pkt->setFlags(0xffffU); // all flags set
+    pkt->setCiaddr(IOAddress("192.0.2.1"));
+    pkt->setYiaddr(IOAddress("1.2.3.4"));
+    pkt->setSiaddr(IOAddress("192.0.2.255"));
+    pkt->setGiaddr(IOAddress("255.255.255.255"));
+    // chaddr already set with setHWAddr()
+    pkt->setSname(dummySname, 64);
+    pkt->setFile(dummyFile, 128);
+
+    return (pkt);
+}
+
+/// Generates test packet
+///
+/// Allocates and generates on-wire buffer that represents
+/// test packet, with all fixed fields set to non-zero values.
+/// Content is not always reasonable.
+///
+/// See generateTestPacket1() function that returns
+/// exactly the same packet as Pkt4 object.
+///
+/// @return pointer to allocated Pkt4 object
+// Returns a vector containing a DHCPv4 packet header.
+#if 0
+vector<uint8_t>
+generateTestPacket2() {
+
+    // That is only part of the header. It contains all "short" fields,
+    // larger fields are constructed separately.
+    uint8_t hdr[] = {
+        1, 6, 6, 13,            // op, htype, hlen, hops,
+        0x12, 0x34, 0x56, 0x78, // transaction-id
+        0, 42, 0xff, 0xff,      // 42 secs, 0xffff flags
+        192, 0, 2, 1,           // ciaddr
+        1, 2, 3, 4,             // yiaddr
+        192, 0, 2, 255,         // siaddr
+        255, 255, 255, 255,     // giaddr
+    };
+
+    // Initialize the vector with the header fields defined above.
+    vector<uint8_t> buf(hdr, hdr + sizeof(hdr));
+
+    // Append the large header fields.
+    copy(dummyMacAddr, dummyMacAddr + Pkt4::MAX_CHADDR_LEN, back_inserter(buf));
+    copy(dummySname, dummySname + Pkt4::MAX_SNAME_LEN, back_inserter(buf));
+    copy(dummyFile, dummyFile + Pkt4::MAX_FILE_LEN, back_inserter(buf));
+
+    // Should now have all the header, so check.  The "static_cast" is used
+    // to get round an odd bug whereby the linker appears not to find the
+    // definition of DHCPV4_PKT_HDR_LEN if it appears within an EXPECT_EQ().
+    EXPECT_EQ(static_cast<size_t>(Pkt4::DHCPV4_PKT_HDR_LEN), buf.size());
+
+    return (buf);
+}
+#endif
+
+TEST(Pkt4Test, fixedFields) {
+
+    shared_ptr<Pkt4> pkt = generateTestPacket1();
+
+    // ok, let's check packet values
+    EXPECT_EQ(1, pkt->getOp());
+    EXPECT_EQ(6, pkt->getHtype());
+    EXPECT_EQ(6, pkt->getHlen());
+    EXPECT_EQ(13, pkt->getHops());
+    EXPECT_EQ(dummyTransid, pkt->getTransid());
+    EXPECT_EQ(42, pkt->getSecs());
+    EXPECT_EQ(0xffff, pkt->getFlags());
+
+    EXPECT_EQ(string("192.0.2.1"), pkt->getCiaddr().toText());
+    EXPECT_EQ(string("1.2.3.4"), pkt->getYiaddr().toText());
+    EXPECT_EQ(string("192.0.2.255"), pkt->getSiaddr().toText());
+    EXPECT_EQ(string("255.255.255.255"), pkt->getGiaddr().toText());
+
+    // chaddr is always 16 bytes long and contains link-layer addr (MAC)
+    EXPECT_EQ(0, memcmp(dummyChaddr, pkt->getChaddr(), 16));
+
+    EXPECT_EQ(0, memcmp(dummySname, &pkt->getSname()[0], 64));
+
+    EXPECT_EQ(0, memcmp(dummyFile, &pkt->getFile()[0], 128));
+
+    EXPECT_EQ(DHCPDISCOVER, pkt->getType());
+}
+
+#if 0
+/// TODO Uncomment when ticket #1227 is implemented
+TEST(Pkt4Test, fixedFieldsPack) {
+    shared_ptr<Pkt4> pkt = generateTestPacket1();
+    shared_array<uint8_t> expectedFormat = generateTestPacket2();
+
+    EXPECT_NO_THROW(
+        pkt->pack();
+    );
+
+    ASSERT_EQ(Pkt4::DHCPV4_PKT_HDR_LEN, pkt->len());
+
+    EXPECT_EQ(0, memcmp(&expectedFormat[0], pkt->getData(), pkt->len()));
+}
+
+/// TODO Uncomment when ticket #1226 is implemented
+TEST(Pkt4Test, fixedFieldsUnpack) {
+    shared_array<uint8_t> expectedFormat = generateTestPkt2();
+
+    shared_ptr<Pkt4> pkt(new Pkt4(&expectedFormat[0],
+                                  Pkt4::DHCPV4_PKT_HDR_LEN));
+
+    // ok, let's check packet values
+    EXPECT_EQ(1, pkt->getOp());
+    EXPECT_EQ(6, pkt->getHtype());
+    EXPECT_EQ(6, pkt->getHlen());
+    EXPECT_EQ(13, pkt->getHops());
+    EXPECT_EQ(transid, pkt->getTransid());
+    EXPECT_EQ(42, pkt->getSecs());
+    EXPECT_EQ(0xffff, pkt->getFlags());
+
+    EXPECT_EQ(string("192.0.2.1"), pkt->getCiaddr.toText());
+    EXPECT_EQ(string("1.2.3.4"), pkt->getYiaddr.toText());
+    EXPECT_EQ(string("192.0.2.255"), pkt->getSiaddr.toText());
+    EXPECT_EQ(string("255.255.255.255"), pkt->getGiaddr.toText());
+
+    // chaddr is always 16 bytes long and contains link-layer addr (MAC)
+    EXPECT_EQ(0, memcmp(expectedChaddr, pkt->getChaddr(), 16));
+
+    EXPECT_EQ(0, memcmp(expectedSname, pkt->getSname(), 64));
+
+    EXPECT_EQ(0, memcmp(expectedFile, pkt->getFile(), 128));
+
+    EXPECT_EQ(DHCPSOLICIT, pkt->getType());
+}
+#endif
+
+// this test is for hardware addresses (htype, hlen and chaddr fields)
+TEST(Pkt4Test, hwAddr) {
+
+    vector<uint8_t> mac;
+    uint8_t expectedChaddr[Pkt4::MAX_CHADDR_LEN];
+
+    mac.resize(Pkt4::MAX_CHADDR_LEN);
+
+    Pkt4* pkt = 0;
+    // let's test each hlen, from 0 till 16
+    for (int macLen=0; macLen < Pkt4::MAX_CHADDR_LEN; macLen++) {
+        for (int i=0; i < Pkt4::MAX_CHADDR_LEN; i++) {
+            mac[i] = 0;
+            expectedChaddr[i] = 0;
+        }
+        for (int i=0; i < macLen; i++) {
+            mac[i] = 128+i;
+            expectedChaddr[i] = 128+i;
+        }
+
+        // type and transaction doesn't matter in this test
+        pkt = new Pkt4(DHCPOFFER, 1234);
+        pkt->setHWAddr(255-macLen*10, // just weird htype
+                       macLen,
+                       mac);
+        EXPECT_EQ(0, memcmp(expectedChaddr, pkt->getChaddr(),
+                            Pkt4::MAX_CHADDR_LEN));
+
+#if 0
+        /// TODO Uncomment when ticket #1227 is implemented)
+        EXPECT_NO_THROW(
+            pkt->pack();
+        );
+
+        // CHADDR starts at offset 28 in DHCP packet
+        EXPECT_EQ(0, memcmp(pkt->getData()+28, expectedChaddr,
+                            Pkt4::MAX_CHADDR_LEN));
+#endif
+
+        delete pkt;
+    }
+
+    /// TODO: extend this test once options support is implemented. HW address
+    /// longer than 16 bytes should be stored in client-identifier option
+}
+
+TEST(Pkt4Test, msgTypes) {
+
+    struct msgType {
+        uint8_t dhcp;
+        uint8_t bootp;
+    };
+
+    msgType types[] = {
+        {DHCPDISCOVER, BOOTREQUEST},
+        {DHCPOFFER, BOOTREPLY},
+        {DHCPREQUEST, BOOTREQUEST},
+        {DHCPDECLINE, BOOTREQUEST},
+        {DHCPACK, BOOTREPLY},
+        {DHCPNAK, BOOTREPLY},
+        {DHCPRELEASE, BOOTREQUEST},
+        {DHCPINFORM, BOOTREQUEST},
+        {DHCPLEASEQUERY, BOOTREQUEST},
+        {DHCPLEASEUNASSIGNED, BOOTREPLY},
+        {DHCPLEASEUNKNOWN, BOOTREPLY},
+        {DHCPLEASEACTIVE, BOOTREPLY}
+    };
+
+    Pkt4* pkt = 0;
+    for (int i=0; i < sizeof(types)/sizeof(msgType); i++) {
+
+        pkt = new Pkt4(types[i].dhcp, 0);
+        EXPECT_EQ(types[i].dhcp, pkt->getType());
+
+        EXPECT_EQ(types[i].bootp, pkt->getOp());
+
+        delete pkt;
+        pkt = 0;
+    }
+
+    EXPECT_THROW(
+        pkt = new Pkt4(100, 0), // there's no message type 100
+        OutOfRange
+    );
+    if (pkt) {
+        delete pkt;
+    }
+}
+
+// this test verifies handling of sname field
+TEST(Pkt4Test, sname) {
+
+    uint8_t sname[Pkt4::MAX_SNAME_LEN];
+    uint8_t expectedSname[Pkt4::MAX_SNAME_LEN];
+
+    Pkt4* pkt = 0;
+    // let's test each sname length, from 0 till 64
+    for (int snameLen=0; snameLen < Pkt4::MAX_SNAME_LEN; snameLen++) {
+        for (int i=0; i < Pkt4::MAX_SNAME_LEN; i++) {
+            sname[i] = 0;
+            expectedSname[i] = 0;
+        }
+        for (int i=0; i < snameLen; i++) {
+            sname[i] = i;
+            expectedSname[i] = i;
+        }
+
+        // type and transaction doesn't matter in this test
+        pkt = new Pkt4(DHCPOFFER, 1234);
+        pkt->setSname(sname, snameLen);
+
+        EXPECT_EQ(0, memcmp(expectedSname, &pkt->getSname()[0], Pkt4::MAX_SNAME_LEN));
+
+#if 0
+        /// TODO Uncomment when ticket #1227 is implemented)
+        EXPECT_NO_THROW(
+            pkt->pack();
+        );
+
+        // SNAME starts at offset 44 in DHCP packet
+        EXPECT_EQ(0, memcmp(pkt->getData()+44, expectedChaddr, Pkt4::MAX_SNAME_LEN));
+#endif
+
+        delete pkt;
+    }
+}
+
+TEST(Pkt4Test, file) {
+
+    uint8_t file[Pkt4::MAX_FILE_LEN];
+    uint8_t expectedFile[Pkt4::MAX_FILE_LEN];
+
+    Pkt4* pkt = 0;
+    // let's test each file length, from 0 till 64
+    for (int fileLen=0; fileLen < Pkt4::MAX_FILE_LEN; fileLen++) {
+        for (int i=0; i < Pkt4::MAX_FILE_LEN; i++) {
+            file[i] = 0;
+            expectedFile[i] = 0;
+        }
+        for (int i=0; i < fileLen; i++) {
+            file[i] = i;
+            expectedFile[i] = i;
+        }
+
+        // type and transaction doesn't matter in this test
+        pkt = new Pkt4(DHCPOFFER, 1234);
+        pkt->setFile(file, fileLen);
+
+        EXPECT_EQ(0, memcmp(expectedFile, &pkt->getFile()[0], Pkt4::MAX_FILE_LEN));
+
+#if 0
+        /// TODO Uncomment when ticket #1227 is implemented)
+        EXPECT_NO_THROW(
+            pkt->pack();
+        );
+
+        // FILE starts at offset 44 in DHCP packet
+        EXPECT_EQ(0, memcmp(pkt->getData()+44, expectedChaddr, Pkt4::MAX_FILE_LEN));
+#endif
+
+        delete pkt;
+    }
+
+}
+
+} // end of anonymous namespace

src/lib/dhcp/tests/pkt6_unittest.cc

@@ -108,7 +108,8 @@ TEST_F(Pkt6Test, unpack_solicit1) {
     EXPECT_FALSE(sol->getOption(D6O_IA_TA));
     EXPECT_FALSE(sol->getOption(D6O_IAADDR));
 
-    std::cout << sol->toText();
+    // let's limit verbosity of this test
+    // std::cout << sol->toText();
 
     delete sol;
 }

src/lib/dns/python/message_python.cc

@@ -16,6 +16,7 @@
 #include <Python.h>
 
 #include <exceptions/exceptions.h>
+#include <util/python/pycppwrapper_util.h>
 #include <dns/message.h>
 #include <dns/rcode.h>
 #include <dns/tsig.h>
@@ -38,6 +39,7 @@ using namespace std;
 using namespace isc::dns;
 using namespace isc::dns::python;
 using namespace isc::util;
+using namespace isc::util::python;
 
 // Import pydoc text
 #include "message_python_inc.cc"
@@ -64,8 +66,8 @@ PyObject* Message_setEDNS(s_Message* self, PyObject* args);
 PyObject* Message_getTSIGRecord(s_Message* self);
 PyObject* Message_getRRCount(s_Message* self, PyObject* args);
 // use direct iterators for these? (or simply lists for now?)
-PyObject* Message_getQuestion(s_Message* self);
-PyObject* Message_getSection(s_Message* self, PyObject* args);
+PyObject* Message_getQuestion(PyObject* self, PyObject*);
+PyObject* Message_getSection(PyObject* self, PyObject* args);
 //static PyObject* Message_beginQuestion(s_Message* self, PyObject* args);
 //static PyObject* Message_endQuestion(s_Message* self, PyObject* args);
 //static PyObject* Message_beginSection(s_Message* self, PyObject* args);
@@ -127,10 +129,10 @@ PyMethodDef Message_methods[] = {
     },
     { "get_rr_count", reinterpret_cast<PyCFunction>(Message_getRRCount), METH_VARARGS,
       "Returns the number of RRs contained in the given section." },
-    { "get_question", reinterpret_cast<PyCFunction>(Message_getQuestion), METH_NOARGS,
+    { "get_question", Message_getQuestion, METH_NOARGS,
       "Returns a list of all Question objects in the message "
       "(should be either 0 or 1)" },
-    { "get_section", reinterpret_cast<PyCFunction>(Message_getSection), METH_VARARGS,
+    { "get_section", Message_getSection, METH_VARARGS,
       "Returns a list of all RRset objects in the given section of the message\n"
       "The argument must be of type Section" },
     { "add_question", reinterpret_cast<PyCFunction>(Message_addQuestion), METH_VARARGS,
@@ -409,50 +411,59 @@ Message_getRRCount(s_Message* self, PyObject* args) {
     }
 }
 
+// This is a helper templated class commonly used for getQuestion and
+// getSection in order to build a list of Message section items.
+template <typename ItemType, typename CreatorParamType>
+class SectionInserter {
+    typedef PyObject* (*creator_t)(const CreatorParamType&);
+public:
+    SectionInserter(PyObject* pylist, creator_t creator) :
+        pylist_(pylist), creator_(creator)
+    {}
+    void operator()(ItemType item) {
+        if (PyList_Append(pylist_, PyObjectContainer(creator_(*item)).get())
+            == -1) {
+            isc_throw(PyCPPWrapperException, "PyList_Append failed, "
+                      "probably due to short memory");
+        }
+    }
+private:
+    PyObject* pylist_;
+    creator_t creator_;
+};
+
+typedef SectionInserter<ConstQuestionPtr, Question> QuestionInserter;
+typedef SectionInserter<ConstRRsetPtr, RRset> RRsetInserter;
+
 // TODO use direct iterators for these? (or simply lists for now?)
 PyObject*
-Message_getQuestion(s_Message* self) {
-    QuestionIterator qi, qi_end;
+Message_getQuestion(PyObject* po_self, PyObject*) {
+    const s_Message* const self = static_cast<s_Message*>(po_self);
+
     try {
-        qi = self->cppobj->beginQuestion();
-        qi_end = self->cppobj->endQuestion();
+        PyObjectContainer list_container(PyList_New(0));
+        for_each(self->cppobj->beginQuestion(),
+                 self->cppobj->endQuestion(),
+                 QuestionInserter(list_container.get(), createQuestionObject));
+        return (list_container.release());
     } catch (const InvalidMessageSection& ex) {
         PyErr_SetString(po_InvalidMessageSection, ex.what());
-        return (NULL);
-    } catch (...) {
-        PyErr_SetString(po_IscException,
-                        "Unexpected exception in getting section iterators");
-        return (NULL);
-    }
-
-    PyObject* list = PyList_New(0);
-    if (list == NULL) {
-        return (NULL);
-    }
-
-    try {
-        for (; qi != qi_end; ++qi) {
-            if (PyList_Append(list, createQuestionObject(**qi)) == -1) {
-                Py_DECREF(list);
-                return (NULL);
-            }
-        }
-        return (list);
     } catch (const exception& ex) {
         const string ex_what =
-            "Unexpected failure getting Question section: " +
+            "Unexpected failure in Message.get_question: " +
             string(ex.what());
         PyErr_SetString(po_IscException, ex_what.c_str());
     } catch (...) {
         PyErr_SetString(PyExc_SystemError,
-                        "Unexpected failure getting Question section");
+                        "Unexpected failure in Message.get_question");
     }
-    Py_DECREF(list);
     return (NULL);
 }
 
 PyObject*
-Message_getSection(s_Message* self, PyObject* args) {
+Message_getSection(PyObject* po_self, PyObject* args) {
+    const s_Message* const self = static_cast<s_Message*>(po_self);
+
     unsigned int section;
     if (!PyArg_ParseTuple(args, "I", &section)) {
         PyErr_Clear();
@@ -460,46 +471,28 @@ Message_getSection(s_Message* self, PyObject* args) {
                         "no valid type in get_section argument");
         return (NULL);
     }
-    RRsetIterator rrsi, rrsi_end;
+
     try {
-        rrsi = self->cppobj->beginSection(
-            static_cast<Message::Section>(section));
-        rrsi_end = self->cppobj->endSection(
-            static_cast<Message::Section>(section));
+        PyObjectContainer list_container(PyList_New(0));
+        const Message::Section msgsection =
+            static_cast<Message::Section>(section);
+        for_each(self->cppobj->beginSection(msgsection),
+                 self->cppobj->endSection(msgsection),
+                 RRsetInserter(list_container.get(), createRRsetObject));
+        return (list_container.release());
     } catch (const isc::OutOfRange& ex) {
         PyErr_SetString(PyExc_OverflowError, ex.what());
-        return (NULL);
     } catch (const InvalidMessageSection& ex) {
         PyErr_SetString(po_InvalidMessageSection, ex.what());
-        return (NULL);
-    } catch (...) {
-        PyErr_SetString(po_IscException,
-                        "Unexpected exception in getting section iterators");
-        return (NULL);
-    }
-
-    PyObject* list = PyList_New(0);
-    if (list == NULL) {
-        return (NULL);
-    }
-    try {
-        for (; rrsi != rrsi_end; ++rrsi) {
-            if (PyList_Append(list, createRRsetObject(**rrsi)) == -1) {
-                    Py_DECREF(list);
-                    return (NULL);
-            }
-        }
-        return (list);
     } catch (const exception& ex) {
         const string ex_what =
-            "Unexpected failure creating Question object: " +
+            "Unexpected failure in Message.get_section: " +
             string(ex.what());
         PyErr_SetString(po_IscException, ex_what.c_str());
     } catch (...) {
         PyErr_SetString(PyExc_SystemError,
-                        "Unexpected failure creating Question object");
+                        "Unexpected failure in Message.get_section");
     }
-    Py_DECREF(list);
     return (NULL);
 }
 

src/lib/dns/python/rrset_python.cc

@@ -63,7 +63,7 @@ PyObject* RRset_toText(s_RRset* self);
 PyObject* RRset_str(PyObject* self);
 PyObject* RRset_toWire(s_RRset* self, PyObject* args);
 PyObject* RRset_addRdata(s_RRset* self, PyObject* args);
-PyObject* RRset_getRdata(s_RRset* self);
+PyObject* RRset_getRdata(PyObject* po_self, PyObject*);
 PyObject* RRset_removeRRsig(s_RRset* self);
 
 // TODO: iterator?
@@ -94,7 +94,7 @@ PyMethodDef RRset_methods[] = {
       "returned" },
     { "add_rdata", reinterpret_cast<PyCFunction>(RRset_addRdata), METH_VARARGS,
       "Adds the rdata for one RR to the RRset.\nTakes an Rdata object as an argument" },
-    { "get_rdata", reinterpret_cast<PyCFunction>(RRset_getRdata), METH_NOARGS,
+    { "get_rdata", RRset_getRdata, METH_NOARGS,
       "Returns a List containing all Rdata elements" },
     { "remove_rrsig", reinterpret_cast<PyCFunction>(RRset_removeRRsig), METH_NOARGS,
       "Clears the list of RRsigs for this RRset" },
@@ -291,22 +291,26 @@ RRset_addRdata(s_RRset* self, PyObject* args) {
 }
 
 PyObject*
-RRset_getRdata(s_RRset* self) {
-    PyObject* list = PyList_New(0);
-
-    RdataIteratorPtr it = self->cppobj->getRdataIterator();
+RRset_getRdata(PyObject* po_self, PyObject*) {
+    const s_RRset* const self = static_cast<s_RRset*>(po_self);
 
     try {
-        for (; !it->isLast(); it->next()) {
-            const rdata::Rdata *rd = &it->getCurrent();
-            if (PyList_Append(list,
-                    createRdataObject(createRdata(self->cppobj->getType(),
-                                      self->cppobj->getClass(), *rd))) == -1) {
-                Py_DECREF(list);
-                return (NULL);
+        PyObjectContainer list_container(PyList_New(0));
+
+        for (RdataIteratorPtr it = self->cppobj->getRdataIterator();
+             !it->isLast(); it->next()) {
+            if (PyList_Append(list_container.get(),
+                              PyObjectContainer(
+                                  createRdataObject(
+                                      createRdata(self->cppobj->getType(),
+                                                  self->cppobj->getClass(),
+                                                  it->getCurrent()))).get())
+                == -1) {
+                isc_throw(PyCPPWrapperException, "PyList_Append failed, "
+                          "probably due to short memory");
             }
         }
-        return (list);
+        return (list_container.release());
     } catch (const exception& ex) {
         const string ex_what =
             "Unexpected failure getting rrset Rdata: " +
@@ -316,7 +320,6 @@ RRset_getRdata(s_RRset* self) {
         PyErr_SetString(PyExc_SystemError,
                         "Unexpected failure getting rrset Rdata");
     }
-    Py_DECREF(list);
     return (NULL);
 }
 

src/lib/dns/python/tests/message_python_test.py

@@ -17,6 +17,7 @@
 # Tests for the message part of the pydnspp module
 #
 
+import sys
 import unittest
 import os
 from pydnspp import *
@@ -230,6 +231,14 @@ class MessageTest(unittest.TestCase):
         self.assertTrue(compare_rrset_list(section_rrset, self.r.get_section(Message.SECTION_ANSWER)))
         self.assertEqual(2, self.r.get_rr_count(Message.SECTION_ANSWER))
 
+        # We always make a new deep copy in get_section(), so the reference
+        # count of the returned list and its each item should be 1; otherwise
+        # they would leak.
+        self.assertEqual(1, sys.getrefcount(self.r.get_section(
+                    Message.SECTION_ANSWER)))
+        self.assertEqual(1, sys.getrefcount(self.r.get_section(
+                    Message.SECTION_ANSWER)[0]))
+
         self.assertFalse(compare_rrset_list(section_rrset, self.r.get_section(Message.SECTION_AUTHORITY)))
         self.assertEqual(0, self.r.get_rr_count(Message.SECTION_AUTHORITY))
         self.r.add_rrset(Message.SECTION_AUTHORITY, self.rrset_a)
@@ -242,7 +251,7 @@ class MessageTest(unittest.TestCase):
         self.assertTrue(compare_rrset_list(section_rrset, self.r.get_section(Message.SECTION_ADDITIONAL)))
         self.assertEqual(2, self.r.get_rr_count(Message.SECTION_ADDITIONAL))
 
-    def test_add_question(self):
+    def test_add_and_get_question(self):
         self.assertRaises(TypeError, self.r.add_question, "wrong", "wrong")
         q = Question(Name("example.com"), RRClass("IN"), RRType("A"))
         qs = [q]
@@ -252,6 +261,12 @@ class MessageTest(unittest.TestCase):
         self.assertTrue(compare_rrset_list(qs, self.r.get_question()))
         self.assertEqual(1, self.r.get_rr_count(Message.SECTION_QUESTION))
 
+        # We always make a new deep copy in get_section(), so the reference
+        # count of the returned list and its each item should be 1; otherwise
+        # they would leak.
+        self.assertEqual(1, sys.getrefcount(self.r.get_question()))
+        self.assertEqual(1, sys.getrefcount(self.r.get_question()[0]))
+
     def test_add_rrset(self):
         self.assertRaises(TypeError, self.r.add_rrset, "wrong")
         self.assertRaises(TypeError, self.r.add_rrset)

src/lib/dns/python/tests/rrset_python_test.py

@@ -17,6 +17,7 @@
 # Tests for the rrtype part of the pydnspp module
 #
 
+import sys
 import unittest
 import os
 from pydnspp import *
@@ -110,6 +111,12 @@ class TestModuleSpec(unittest.TestCase):
                 ]
         self.assertEqual(rdata, self.rrset_a.get_rdata())
         self.assertEqual([], self.rrset_a_empty.get_rdata())
+
+        # We always make a new deep copy in get_rdata(), so the reference
+        # count of the returned list and its each item should be 1; otherwise
+        # they would leak.
+        self.assertEqual(1, sys.getrefcount(self.rrset_a.get_rdata()))
+        self.assertEqual(1, sys.getrefcount(self.rrset_a.get_rdata()[0]))
         
 if __name__ == '__main__':
     unittest.main()

src/lib/log/Makefile.am

@@ -9,6 +9,7 @@ lib_LTLIBRARIES = liblog.la
 liblog_la_SOURCES  =
 liblog_la_SOURCES += dummylog.h dummylog.cc
 liblog_la_SOURCES += logimpl_messages.cc logimpl_messages.h
+liblog_la_SOURCES += log_dbglevels.h
 liblog_la_SOURCES += log_formatter.h log_formatter.cc
 liblog_la_SOURCES += logger.cc logger.h
 liblog_la_SOURCES += logger_impl.cc logger_impl.h
@@ -21,8 +22,8 @@ liblog_la_SOURCES += logger_name.cc logger_name.h
 liblog_la_SOURCES += logger_specification.h
 liblog_la_SOURCES += logger_support.cc logger_support.h
 liblog_la_SOURCES += logger_unittest_support.cc logger_unittest_support.h
-liblog_la_SOURCES += macros.h
 liblog_la_SOURCES += log_messages.cc log_messages.h
+liblog_la_SOURCES += macros.h
 liblog_la_SOURCES += message_dictionary.cc message_dictionary.h
 liblog_la_SOURCES += message_exception.h
 liblog_la_SOURCES += message_initializer.cc message_initializer.h

src/lib/log/README

@@ -477,6 +477,11 @@ the severity system:
 When a particular severity is set, it - and all severities and/or debug
 levels above it - will be logged.
 
+To try to ensure that the information from different modules is roughly
+comparable for the same debug level, a set of standard debug levels has
+been defined for common type of debug output.  However, modules are free
+to set their own debug levels or define additional ones.
+
 Logging Sources v Logging Severities
 ------------------------------------
 When logging events, make a distinction between events related to the

src/lib/log/log_dbglevels.h

@@ -0,0 +1,93 @@
+// 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 __LOG_DBGLVLS_H
+#define __LOG_DBGLVLS_H
+
+/// \file
+///
+/// When a message is logged with DEBUG severity, the debug level associated
+/// with the message is also specified.  This debug level is a number
+/// ranging from 0 to 99; the idea is that the higher the debug level, the
+/// more detailed the message.
+///
+/// If debug messages are being logged, the logging system allows them to be
+/// filtered by debug level - only messages logged with a level equal to or
+/// less than the set debug level will be output.  (For example, if the
+/// filter is set to 30, only debug messages logged with levels in the range
+/// 0 to 30 will be output; messages logged with levels 31 to 99 will be
+/// suppressed.)
+///
+/// Levels of 30 or below are reserved for debug messages that are most
+/// likely to be useful for an administrator. Levels 31 to 99 are for use by
+/// someone familiar with the code. "Useful for an administrator" is,
+/// admittedly, a subjective term: it is loosely defined as messages helping
+/// someone diagnose a problem that they could solve without needing to dive
+/// into the code.  So it covers things like start-up steps and configuration
+/// messages.
+///
+/// In practice, this means that levels of 30 and below are most-likely to
+/// be used by the top-level programs, and 31 and above by the various
+/// libraries.
+///
+/// This file defines a set of standard debug levels for use across all loggers.
+/// In this way users can have some expection of what will be output when
+/// enabling debugging.  Symbols are prefixed DBGLVL so as not to clash with
+/// DBG_ symbols in the various modules.
+///
+/// \note If the names of debug constants are changed, or if ones are added or
+/// removed, edit the file src/lib/python/isc/log/log.cc to update the log
+/// level definitions available to Python.  The change does not need to be
+/// made if only the numeric values of constants are updated.
+
+namespace {
+
+/// Process startup/shutdown debug messages.  Note that these are _debug_
+/// messages, as other messages related to startup and shutdown may be output
+/// with another severity.  For example, when the authoritative server starts
+/// up, the "server started" message could be output at a severity of INFO.
+/// "Server starting" and messages indicating the stages in startup should be
+/// debug messages output at this severity.
+///
+/// This is given a value of 0 as that is the level selected if debugging is
+/// enabled without giving a level.
+const int DBGLVL_START_SHUT = 0;
+
+/// This debug level is reserved for logging the exchange of messages/commands
+/// between processes, including configuration messages.
+const int DBGLVL_COMMAND = 10;
+
+/// If the commands have associated data, this level is when they are printed.
+/// This includes configuration messages.
+const int DBGLVL_COMMAND_DATA = 20;
+
+// The following constants are suggested values for common operations.
+// Depending on the exact nature of the code, modules may or may not use these
+// levels.
+
+/// Trace basic operations.
+const int DBGLVL_TRACE_BASIC = 40;
+
+/// Trace data associated with the basic operations.
+const int DBGLVL_TRACE_BASIC_DATA = 45;
+
+/// Trace detailed operations.
+const int DBGLVL_TRACE_DETAIL = 50;
+
+/// Trace data associated with detailed operations.
+const int DBGLVL_TRACE_DETAIL_DATA = 55;
+
+}   // Anonymous namespace
+
+#endif // __LOG_DBGLVLS_H

src/lib/log/macros.h

@@ -16,6 +16,7 @@
 #define __LOG_MACROS_H
 
 #include <log/logger.h>
+#include <log/log_dbglevels.h>
 
 /// \brief Macro to conveniently test debug output and log it
 #define LOG_DEBUG(LOGGER, LEVEL, MESSAGE) \

src/lib/nsas/nsas_log.h

@@ -29,15 +29,15 @@ namespace nsas {
 // The first level traces normal operations - asking the NSAS for an address,
 // and cancelling a lookup.  It also records when the NSAS calls back to the
 // resolver to resolve something.
-const int NSAS_DBG_TRACE = 10;
+const int NSAS_DBG_TRACE = DBGLVL_TRACE_BASIC;
 
 // The next level extends the normal operations and records the results of the
 // lookups.
-const int NSAS_DBG_RESULTS = 20;
+const int NSAS_DBG_RESULTS = DBGLVL_TRACE_BASIC_DATA;
 
 // Additional information on the usage of the names - the RTT values obtained
 // when queries were done.
-const int NSAS_DBG_RTT = 30;
+const int NSAS_DBG_RTT = DBGLVL_TRACE_DETAIL_DATA;
 
 
 /// \brief NSAS Logger

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

@@ -16,6 +16,7 @@
 import socket
 import struct
 import os
+import copy
 import subprocess
 from isc.log_messages.bind10_messages import *
 from libutil_io_python import recv_fd
@@ -207,7 +208,7 @@ class Creator(Parser):
         # stdin as well as stdout, so we dup it before passing it there.
         remote2 = socket.fromfd(remote.fileno(), socket.AF_UNIX,
                                 socket.SOCK_STREAM)
-        env = os.environ
+        env = copy.deepcopy(os.environ)
         env['PATH'] = path
         self.__process = subprocess.Popen(['b10-sockcreator'], env=env,
                                           stdin=remote.fileno(),

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

@@ -143,7 +143,9 @@ class ModuleCCSession(ConfigData):
        callbacks are called when 'check_command' is called on the
        ModuleCCSession"""
        
-    def __init__(self, spec_file_name, config_handler, command_handler, cc_session=None, handle_logging_config=True):
+    def __init__(self, spec_file_name, config_handler, command_handler,
+                 cc_session=None, handle_logging_config=True,
+                 socket_file = None):
         """Initialize a ModuleCCSession. This does *NOT* send the
            specification and request the configuration yet. Use start()
            for that once the ModuleCCSession has been initialized.
@@ -165,6 +167,12 @@ class ModuleCCSession(ConfigData):
            logger manager when the logging configuration gets updated.
            The module does not need to do anything except intializing
            its loggers, and provide log messages. Defaults to true.
+
+           socket_file: If cc_session was none, this optional argument
+           specifies which socket file to use to connect to msgq. It
+           will be overridden by the environment variable
+           MSGQ_SOCKET_FILE. If none, and no environment variable is
+           set, it will use the system default.
         """
         module_spec = isc.config.module_spec_from_file(spec_file_name)
         ConfigData.__init__(self, module_spec)
@@ -175,7 +183,7 @@ class ModuleCCSession(ConfigData):
         self.set_command_handler(command_handler)
 
         if not cc_session:
-            self._session = Session()
+            self._session = Session(socket_file)
         else:
             self._session = cc_session
         self._session.group_subscribe(self._module_name, "*")

src/lib/python/isc/datasrc/finder_python.cc

@@ -268,16 +268,16 @@ PyTypeObject zonefinder_type = {
 
 PyObject*
 createZoneFinderObject(isc::datasrc::ZoneFinderPtr source, PyObject* base_obj) {
-    s_ZoneFinder* py_zi = static_cast<s_ZoneFinder*>(
+    s_ZoneFinder* py_zf = static_cast<s_ZoneFinder*>(
         zonefinder_type.tp_alloc(&zonefinder_type, 0));
-    if (py_zi != NULL) {
-        py_zi->cppobj = source;
-        py_zi->base_obj = base_obj;
-    }
-    if (base_obj != NULL) {
-        Py_INCREF(base_obj);
+    if (py_zf != NULL) {
+        py_zf->cppobj = source;
+        py_zf->base_obj = base_obj;
+        if (base_obj != NULL) {
+            Py_INCREF(base_obj);
+        }
     }
-    return (py_zi);
+    return (py_zf);
 }
 
 } // namespace python

src/lib/python/isc/datasrc/iterator_python.cc

@@ -204,9 +204,9 @@ createZoneIteratorObject(isc::datasrc::ZoneIteratorPtr source,
     if (py_zi != NULL) {
         py_zi->cppobj = source;
         py_zi->base_obj = base_obj;
-    }
-    if (base_obj != NULL) {
-        Py_INCREF(base_obj);
+        if (base_obj != NULL) {
+            Py_INCREF(base_obj);
+        }
     }
     return (py_zi);
 }

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

@@ -20,6 +20,7 @@ import isc.dns
 import unittest
 import os
 import shutil
+import sys
 import json
 
 TESTDATA_PATH = os.environ['TESTDATA_PATH'] + os.sep
@@ -494,6 +495,23 @@ class DataSrcUpdater(unittest.TestCase):
                          dsc.get_updater(isc.dns.Name("notexistent.example"),
                                          True))
 
+    def test_client_reference(self):
+        # Temporarily create various objects using factory methods of the
+        # client.  The created objects won't be stored anywhere and
+        # immediately released.  The creation shouldn't affect the reference
+        # to the base client.
+        dsc = isc.datasrc.DataSourceClient("sqlite3", WRITE_ZONE_DB_CONFIG)
+        orig_ref = sys.getrefcount(dsc)
+
+        dsc.find_zone(isc.dns.Name("example.com"))
+        self.assertEqual(orig_ref, sys.getrefcount(dsc))
+
+        dsc.get_iterator(isc.dns.Name("example.com."))
+        self.assertEqual(orig_ref, sys.getrefcount(dsc))
+
+        dsc.get_updater(isc.dns.Name("example.com"), True)
+        self.assertEqual(orig_ref, sys.getrefcount(dsc))
+
 if __name__ == "__main__":
     isc.log.init("bind10")
     unittest.main()

src/lib/python/isc/datasrc/updater_python.cc

@@ -270,15 +270,16 @@ PyObject*
 createZoneUpdaterObject(isc::datasrc::ZoneUpdaterPtr source,
                         PyObject* base_obj)
 {
-    s_ZoneUpdater* py_zi = static_cast<s_ZoneUpdater*>(
+    s_ZoneUpdater* py_zu = static_cast<s_ZoneUpdater*>(
         zoneupdater_type.tp_alloc(&zoneupdater_type, 0));
-    if (py_zi != NULL) {
-        py_zi->cppobj = source;
-    }
-    if (base_obj != NULL) {
-        Py_INCREF(base_obj);
+    if (py_zu != NULL) {
+        py_zu->cppobj = source;
+        py_zu->base_obj = base_obj;
+        if (base_obj != NULL) {
+            Py_INCREF(base_obj);
+        }
     }
-    return (py_zi);
+    return (py_zu);
 }
 
 } // namespace python

src/lib/python/isc/log/log.cc

@@ -28,7 +28,11 @@
 #include <string>
 #include <boost/bind.hpp>
 
+#include <util/python/pycppwrapper_util.h>
+#include <log/log_dbglevels.h>
+
 using namespace isc::log;
+using namespace isc::util::python;
 using std::string;
 using boost::bind;
 
@@ -723,7 +727,38 @@ PyInit_log(void) {
                                &logger_type))) < 0) {
         return (NULL);
     }
-    Py_INCREF(&logger_type);
 
+    // Add in the definitions of the standard debug levels.  These can then
+    // be referred to in Python through the constants log.DBGLVL_XXX.
+    // N.B. These should be kept in sync with the constants defined in
+    // log_dbglevels.h.
+    try {
+        installClassVariable(logger_type, "DBGLVL_START_SHUT",
+                             Py_BuildValue("I", DBGLVL_START_SHUT));
+        installClassVariable(logger_type, "DBGLVL_COMMAND",
+                             Py_BuildValue("I", DBGLVL_COMMAND));
+        installClassVariable(logger_type, "DBGLVL_COMMAND_DATA",
+                             Py_BuildValue("I", DBGLVL_COMMAND_DATA));
+        installClassVariable(logger_type, "DBGLVL_TRACE_BASIC",
+                             Py_BuildValue("I", DBGLVL_TRACE_BASIC));
+        installClassVariable(logger_type, "DBGLVL_TRACE_BASIC_DATA",
+                             Py_BuildValue("I", DBGLVL_TRACE_BASIC_DATA));
+        installClassVariable(logger_type, "DBGLVL_TRACE_DETAIL",
+                             Py_BuildValue("I", DBGLVL_TRACE_DETAIL));
+        installClassVariable(logger_type, "DBGLVL_TRACE_DETAIL_DATA",
+                             Py_BuildValue("I", DBGLVL_TRACE_DETAIL_DATA));
+    } catch (const std::exception& ex) {
+        const std::string ex_what =
+            "Unexpected failure in Log initialization: " +
+            std::string(ex.what());
+        PyErr_SetString(PyExc_SystemError, ex_what.c_str());
+        return (NULL);
+    } catch (...) {
+        PyErr_SetString(PyExc_SystemError,
+                        "Unexpected failure in Log initialization");
+        return (NULL);
+    }
+
+    Py_INCREF(&logger_type);
     return (mod);
 }

src/lib/python/isc/log/tests/log_test.py

@@ -159,5 +159,15 @@ class Logger(unittest.TestCase):
         # Bad type
         self.assertRaises(TypeError, logger.debug, "42", "hello")
 
+    def test_dbglevel_constants(self):
+        """
+            Just check a constant to make sure it is defined and is the
+            correct value.  (The constant chosen has a non-zero value to
+            ensure that the code has both define the constant and set its
+            value correctly.)
+        """
+        logger = isc.log.Logger("child")
+        self.assertEqual(logger.DBGLVL_COMMAND, 10)
+
 if __name__ == '__main__':
     unittest.main()

src/lib/resolve/resolve_log.h

@@ -27,17 +27,17 @@ namespace resolve {
 /// Note that higher numbers equate to more verbose (and detailed) output.
 
 // The first level traces normal operations
-const int RESLIB_DBG_TRACE = 10;
+const int RESLIB_DBG_TRACE = DBGLVL_TRACE_BASIC;
 
 // The next level extends the normal operations and records the results of the
 // lookups.
-const int RESLIB_DBG_RESULTS = 20;
+const int RESLIB_DBG_RESULTS = DBGLVL_TRACE_BASIC_DATA;
 
 // Report cache lookups and results
-const int RESLIB_DBG_CACHE = 40;
+const int RESLIB_DBG_CACHE = DBGLVL_TRACE_DETAIL_DATA;
 
 // Indicate when callbacks are called
-const int RESLIB_DBG_CB = 50;
+const int RESLIB_DBG_CB = DBGLVL_TRACE_DETAIL_DATA + 10;
 
 
 /// \brief Resolver Library Logger

src/lib/server_common/logger.h

@@ -31,12 +31,11 @@ namespace server_common {
 /// \brief The logger for this library
 extern isc::log::Logger logger;
 
-enum {
-    /// \brief Trace basic operations
-    DBG_TRACE_BASIC = 10,
-    /// \brief Print also values used
-    DBG_TRACE_VALUES = 40
-};
+/// \brief Trace basic operations
+const int DBG_TRACE_BASIC = DBGLVL_TRACE_BASIC;
+
+/// \brief Print also values used
+const int DBG_TRACE_VALUES = DBGLVL_TRACE_BASIC_DATA;
 
 }
 }