[trac1644]Merge branch 'master' into trac1644

.gitignore

@@ -3,6 +3,7 @@
 *.o
 .deps/
 .libs/
+__pycache__/
 Makefile
 Makefile.in
 TAGS

ChangeLog

@@ -1,3 +1,28 @@
+402.	[func]		jelte
+	b10-xfrout now has a visible command to send out notifies for
+	a given zone, callable from bindctl. Xfrout notify <zone> [class]
+	(Trac #1321, git 0bb258f8610620191d75cfd5d2308b6fc558c280)
+
+401.	[func]*		jinmei
+	libdns++: updated the internal implementation of the
+	MessageRenderer class.  This is mostly a transparent change, but
+	the new version now doesn't allow changing compression mode in the
+	middle of rendering (which shouldn't be an issue in practice).
+	On the other hand, name compression performance was significantly
+	improved: depending on the number of names, micro benchmark tests
+	showed the new version is several times faster than the previous
+	version .
+	(Trac #1603, git 9a2a86f3f47b60ff017ce1a040941d0c145cfe16)
+
+400.	[bug]		stephen
+	Fix crash on Max OS X 10.7 by altering logging so as not to allocate
+	heap storage in the static initialization of logging objects.
+	(Trac #1698, git a8e53be7039ad50d8587c0972244029ff3533b6e)
+
+399.	[func]		muks
+	Add support for the SSHFP RR type (RFC 4255).
+	(Trac #1136, git ea5ac57d508a17611cfae9d9ea1c238f59d52c51)
+
 398.	[func]		jelte
 	The b10-xfrin module now logs more information on successful
 	incoming transfers. In the case of IXFR, it logs the number of

src/bin/auth/auth_srv.cc

@@ -100,6 +100,7 @@ public:
 
     IOService io_service_;
 
+    MessageRenderer renderer_;
     /// Currently non-configurable, but will be.
     static const uint16_t DEFAULT_LOCAL_UDPSIZE = 4096;
 
@@ -305,8 +306,9 @@ makeErrorMessage(Message& message, OutputBuffer& buffer,
         message.setHeaderFlag(Message::HEADERFLAG_CD);
     }
     for_each(questions.begin(), questions.end(), QuestionInserter(message));
-    message.setRcode(rcode);
 
+    message.setRcode(rcode);
+    
     MessageRenderer renderer;
     renderer.setBuffer(&buffer);
     if (tsig_context.get() != NULL) {
@@ -564,20 +566,19 @@ AuthSrvImpl::processNormalQuery(const IOMessage& io_message, Message& message,
         return (true);
     }
 
-    MessageRenderer renderer;
-    renderer.setBuffer(&buffer);
+    renderer_.clear();
+    renderer_.setBuffer(&buffer);
+    
     const bool udp_buffer =
         (io_message.getSocket().getProtocol() == IPPROTO_UDP);
-    renderer.setLengthLimit(udp_buffer ? remote_bufsize : 65535);
+    renderer_.setLengthLimit(udp_buffer ? remote_bufsize : 65535);
     if (tsig_context.get() != NULL) {
-        message.toWire(renderer, *tsig_context);
+        message.toWire(renderer_, *tsig_context);
     } else {
-        message.toWire(renderer);
+        message.toWire(renderer_);
     }
-    renderer.setBuffer(NULL);
     LOG_DEBUG(auth_logger, DBG_AUTH_MESSAGES, AUTH_SEND_NORMAL_RESPONSE)
-              .arg(renderer.getLength()).arg(message);
-
+              .arg(renderer_.getLength()).arg(message.toText());
     return (true);
 }
 
@@ -695,14 +696,13 @@ AuthSrvImpl::processNotify(const IOMessage& io_message, Message& message,
     message.setHeaderFlag(Message::HEADERFLAG_AA);
     message.setRcode(Rcode::NOERROR());
 
-    MessageRenderer renderer;
-    renderer.setBuffer(&buffer);
+    renderer_.clear();
+    renderer_.setBuffer(&buffer);
     if (tsig_context.get() != NULL) {
-        message.toWire(renderer, *tsig_context);
+        message.toWire(renderer_, *tsig_context);
     } else {
-        message.toWire(renderer);
+        message.toWire(renderer_);
     }
-    renderer.setBuffer(NULL);
     return (true);
 }
 

src/bin/auth/tests/auth_srv_unittest.cc

@@ -1075,6 +1075,28 @@ TEST_F(AuthSrvTest, listenAddresses) {
                                 "Released tokens");
 }
 
+TEST_F(AuthSrvTest, processNormalQuery_reuseRenderer1) {
+    UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(),
+                                       default_qid, Name("example.com"),
+                                       RRClass::IN(), RRType::NS());
+    
+    request_message.setHeaderFlag(Message::HEADERFLAG_AA);
+    createRequestPacket(request_message, IPPROTO_UDP);
+    server.processMessage(*io_message, *parse_message, *response_obuffer, &dnsserv);
+    EXPECT_NE(request_message.getRcode(), parse_message->getRcode());
+}
+
+TEST_F(AuthSrvTest, processNormalQuery_reuseRenderer2) {
+    UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(),
+                                       default_qid, Name("example.com"),
+                                       RRClass::IN(), RRType::SOA());
+    
+    request_message.setHeaderFlag(Message::HEADERFLAG_AA);
+    createRequestPacket(request_message, IPPROTO_UDP);
+    server.processMessage(*io_message, *parse_message, *response_obuffer, &dnsserv);
+    ConstQuestionPtr question = *parse_message->beginQuestion();
+    EXPECT_STRNE(question->getType().toText().c_str(),RRType::NS().toText().c_str());
+}
 //
 // Tests for catching exceptions in various stages of the query processing
 //

src/bin/xfrout/xfrout.py.in

@@ -926,7 +926,7 @@ class XfroutServer:
         self._notifier.dispatcher()
 
     def send_notify(self, zone_name, zone_class):
-        self._notifier.send_notify(zone_name, zone_class)
+        return self._notifier.send_notify(zone_name, zone_class)
 
     def config_handler(self, new_config):
         '''Update config data. TODO. Do error check'''
@@ -982,10 +982,16 @@ class XfroutServer:
         elif cmd == notify_out.ZONE_NEW_DATA_READY_CMD:
             zone_name = args.get('zone_name')
             zone_class = args.get('zone_class')
-            if zone_name and zone_class:
+            if not zone_class:
+                zone_class = str(RRClass.IN())
+            if zone_name:
                 logger.info(XFROUT_NOTIFY_COMMAND, zone_name, zone_class)
-                self.send_notify(zone_name, zone_class)
-                answer = create_answer(0)
+                if self.send_notify(zone_name, zone_class):
+                    answer = create_answer(0)
+                else:
+                    zonestr = notify_out.format_zone_str(Name(zone_name),
+                                                         zone_class)
+                    answer = create_answer(1, "Unknown zone: " + zonestr)
             else:
                 answer = create_answer(1, "Bad command parameter:" + str(args))
 

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

@@ -71,6 +71,19 @@
             "item_optional": true
           }
         ]
+        },
+        { "command_name": "notify",
+          "command_description": "Send notifies for zone",
+          "command_args": [
+          { "item_name": "zone_name",
+            "item_type": "string",
+            "item_optional": false,
+            "item_default": "" },
+          { "item_name": "zone_class",
+            "item_type": "string",
+            "item_optional": true,
+            "item_default": "IN"
+          } ]
         }
       ]
   }

src/lib/asiodns/dns_service.cc

@@ -25,9 +25,9 @@
 #include <asio.hpp>
 #include <dns_service.h>
 #include <asiolink/io_service.h>
-#include <asiolink/io_service.h>
 #include <tcp_server.h>
 #include <udp_server.h>
+#include <sync_udp_server.h>
 
 #include <log/dummylog.h>
 
@@ -66,11 +66,13 @@ public:
                   const asio::ip::address* v4addr,
                   const asio::ip::address* v6addr,
                   SimpleCallback* checkin, DNSLookup* lookup,
-                  DNSAnswer* answer);
+                  DNSAnswer* answer,
+                  const UDPVersion param_flags);
 
     IOService& io_service_;
 
     typedef boost::shared_ptr<UDPServer> UDPServerPtr;
+    typedef boost::shared_ptr<SyncUDPServer> SyncUDPServerPtr;
     typedef boost::shared_ptr<TCPServer> TCPServerPtr;
     typedef boost::shared_ptr<DNSServer> DNSServerPtr;
     std::vector<DNSServerPtr> servers_;
@@ -85,7 +87,8 @@ public:
         servers_.push_back(server);
     }
 
-    void addServer(uint16_t port, const asio::ip::address& address) {
+    void addServer(uint16_t port, const asio::ip::address& address,
+                   const UDPVersion param_flags) {
         try {
             dlog(std::string("Initialize TCP server at ") + address.to_string() + ":" + boost::lexical_cast<std::string>(port));
             TCPServerPtr tcpServer(new TCPServer(io_service_.get_io_service(),
@@ -93,10 +96,27 @@ public:
             (*tcpServer)();
             servers_.push_back(tcpServer);
             dlog(std::string("Initialize UDP server at ") + address.to_string() + ":" + boost::lexical_cast<std::string>(port));
-            UDPServerPtr udpServer(new UDPServer(io_service_.get_io_service(),
-                address, port, checkin_, lookup_, answer_));
-            (*udpServer)();
-            servers_.push_back(udpServer);
+            // Use param_flags to generate diff UDPServers.    
+            switch(param_flags) {
+                case SYNC_: {
+                    SyncUDPServerPtr syncUdpServer(new SyncUDPServer(io_service_.get_io_service(),
+                                                   address, port, checkin_, lookup_, answer_));
+                    (*syncUdpServer)();
+                    servers_.push_back(syncUdpServer);
+                    break;
+                }
+                case ASYNC_: {
+                    UDPServerPtr udpServer(new UDPServer(io_service_.get_io_service(),
+                                           address, port, checkin_, lookup_, answer_));
+                    (*udpServer)();
+                    servers_.push_back(udpServer);
+                    break;
+                }
+                default:
+                    // If nerther asyn UDPServer nor sync UDNServer, it throws.
+                    isc_throw(IOError, "Bad UDPServer Version!");
+                    break;
+            }
         }
         catch (const asio::system_error& err) {
             // We need to catch and convert any ASIO level exceptions.
@@ -106,7 +126,8 @@ public:
                       err.what());
         }
     }
-    void addServer(const char& port, const asio::ip::address& address) {
+    void addServer(const char& port, const asio::ip::address& address,
+                   const UDPVersion param_flags) {
         uint16_t portnum;
         try {
             // XXX: SunStudio with stlport4 doesn't reject some invalid
@@ -122,7 +143,7 @@ public:
             isc_throw(IOError, "Invalid port number '" << &port << "': " <<
                       ex.what());
         }
-        addServer(portnum, address);
+        addServer(portnum, address,param_flags);
     }
 };
 
@@ -132,7 +153,8 @@ DNSServiceImpl::DNSServiceImpl(IOService& io_service,
                                const asio::ip::address* const v6addr,
                                SimpleCallback* checkin,
                                DNSLookup* lookup,
-                               DNSAnswer* answer) :
+                               DNSAnswer* answer,
+                               const UDPVersion param_flags):
     io_service_(io_service),
     checkin_(checkin),
     lookup_(lookup),
@@ -140,10 +162,10 @@ DNSServiceImpl::DNSServiceImpl(IOService& io_service,
 {
 
     if (v4addr) {
-        addServer(port, *v4addr);
+        addServer(port, *v4addr,param_flags);
     }
     if (v6addr) {
-        addServer(port, *v6addr);
+        addServer(port, *v6addr,param_flags);
     }
 }
 
@@ -151,11 +173,12 @@ DNSService::DNSService(IOService& io_service,
                        const char& port, const char& address,
                        SimpleCallback* checkin,
                        DNSLookup* lookup,
-                       DNSAnswer* answer) :
+                       DNSAnswer* answer,
+                       const UDPVersion param_flags) :
     impl_(new DNSServiceImpl(io_service, port, NULL, NULL, checkin, lookup,
-        answer)), io_service_(io_service)
+        answer,param_flags)), io_service_(io_service)
 {
-    addServer(port, &address);
+    addServer(port, &address,param_flags);
 }
 
 DNSService::DNSService(IOService& io_service,
@@ -163,7 +186,8 @@ DNSService::DNSService(IOService& io_service,
                        const bool use_ipv4, const bool use_ipv6,
                        SimpleCallback* checkin,
                        DNSLookup* lookup,
-                       DNSAnswer* answer) :
+                       DNSAnswer* answer,
+                       const UDPVersion param_flags) :
     impl_(NULL), io_service_(io_service)
 {
     const asio::ip::address v4addr_any =
@@ -172,13 +196,13 @@ DNSService::DNSService(IOService& io_service,
     const asio::ip::address v6addr_any =
         asio::ip::address(asio::ip::address_v6::any());
     const asio::ip::address* const v6addrp = use_ipv6 ? &v6addr_any : NULL;
-    impl_ = new DNSServiceImpl(io_service, port, v4addrp, v6addrp, checkin, lookup, answer);
+    impl_ = new DNSServiceImpl(io_service, port, v4addrp, v6addrp, checkin, lookup, answer,param_flags);
 }
 
 DNSService::DNSService(IOService& io_service, SimpleCallback* checkin,
-    DNSLookup* lookup, DNSAnswer *answer) :
+    DNSLookup* lookup, DNSAnswer *answer,const UDPVersion param_flags) :
     impl_(new DNSServiceImpl(io_service, *"0", NULL, NULL, checkin, lookup,
-        answer)), io_service_(io_service)
+        answer,param_flags)), io_service_(io_service)
 {
 }
 
@@ -187,21 +211,25 @@ DNSService::~DNSService() {
 }
 
 void
-DNSService::addServer(const char& port, const std::string& address) {
-    impl_->addServer(port, convertAddr(address));
+DNSService::addServer(const char& port, const std::string& address,UDPVersion param_flags) {
+    impl_->addServer(port, convertAddr(address),param_flags);
 }
 
 void
-DNSService::addServer(uint16_t port, const std::string& address) {
-    impl_->addServer(port, convertAddr(address));
+DNSService::addServer(uint16_t port, const std::string& address,UDPVersion param_flags) {
+    impl_->addServer(port, convertAddr(address),param_flags);
 }
 
 void DNSService::addServerTCPFromFD(int fd, int af) {
     impl_->addServerFromFD<DNSServiceImpl::TCPServerPtr, TCPServer>(fd, af);
 }
 
-void DNSService::addServerUDPFromFD(int fd, int af) {
-    impl_->addServerFromFD<DNSServiceImpl::UDPServerPtr, UDPServer>(fd, af);
+void DNSService::addServerUDPFromFD(int fd, int af,const UDPVersion param_flags) {
+    if(SYNC_ == param_flags) { 
+        impl_->addServerFromFD<DNSServiceImpl::SyncUDPServerPtr, SyncUDPServer>(fd, af);
+    } else if(ASYNC_ == param_flags) {
+        impl_->addServerFromFD<DNSServiceImpl::UDPServerPtr, UDPServer>(fd, af);
+    }
 }
 
 void

src/lib/asiodns/dns_service.h

@@ -27,6 +27,15 @@ class DNSLookup;
 class DNSAnswer;
 class DNSServiceImpl;
 
+
+/// Codes for UDPServers used in addServer()method.
+///
+/// Note: the codes only used in how to create the UDPServers.
+enum UDPVersion {
+      SYNC_  = 1,     ///< used synchronous UDPServer
+      ASYNC_ = 2     ///< used asynchronous UDPServer
+};
+
 /// \brief Handle DNS Queries
 ///
 /// DNSService is the service that handles DNS queries and answers with
@@ -57,7 +66,8 @@ public:
     /// \param answer The answer provider (see \c DNSAnswer)
     DNSService(asiolink::IOService& io_service, const char& port,
                const char& address, isc::asiolink::SimpleCallback* checkin,
-               DNSLookup* lookup, DNSAnswer* answer);
+               DNSLookup* lookup, DNSAnswer* answer,
+               const UDPVersion param_flags = SYNC_);
     /// \brief The constructor with a specific port on which the services
     /// listen on.
     ///
@@ -75,19 +85,23 @@ public:
     DNSService(asiolink::IOService& io_service, const char& port,
                const bool use_ipv4, const bool use_ipv6,
                isc::asiolink::SimpleCallback* checkin, DNSLookup* lookup,
-               DNSAnswer* answer);
+               DNSAnswer* answer,
+               const UDPVersion param_flags = SYNC_);
     /// \brief The constructor without any servers.
     ///
     /// Use addServer() to add some servers.
     DNSService(asiolink::IOService& io_service, isc::asiolink::SimpleCallback* checkin,
-               DNSLookup* lookup, DNSAnswer* answer);
+               DNSLookup* lookup, DNSAnswer* answer,
+               const UDPVersion param_flags = SYNC_);
     /// \brief The destructor.
     ~DNSService();
     //@}
 
     /// \brief Add another server to the service
-    void addServer(uint16_t port, const std::string &address);
-    void addServer(const char &port, const std::string &address);
+    void addServer(uint16_t port, const std::string &address,
+                   const UDPVersion param_flags = SYNC_);
+    void addServer(const char &port, const std::string &address,
+                   const UDPVersion param_flags = SYNC_);
 
     /// \brief Add another TCP server/listener to the service from already
     /// opened file descriptor
@@ -122,7 +136,7 @@ public:
     /// \throw isc::InvalidParameter if af is neither AF_INET nor AF_INET6.
     /// \throw isc::asiolink::IOError when a low-level error happens, like the
     ///     fd is not a valid descriptor or it can't be listened on.
-    void addServerUDPFromFD(int fd, int af);
+    void addServerUDPFromFD(int fd, int af,const UDPVersion param_flags = SYNC_);
 
     /// \brief Remove all servers from the service
     void clearServers();

src/lib/asiodns/tests/io_service_unittest.cc

@@ -116,3 +116,8 @@ TEST(IOServiceTest, DISABLED_IPv4MappedDuplicateBind) {
     delete dns_service;
 }
 
+TEST(IOServiceTest, BadUdpServerVersion) {
+    IOService io_service;
+    DNSService* dns_service = new DNSService(io_service, NULL, NULL, NULL);
+    EXPECT_THROW(dns_service->addServer(*TEST_SERVER_PORT, "127.0.0.1", UDPVersion(3)), IOError);
+}

src/lib/cache/tests/negative_cache_unittest.cc

@@ -52,14 +52,17 @@ TEST_F(NegativeCacheTest, testNXDOMAIN){
     msg_nxdomain.makeResponse();
 
     Name non_exist_qname("nonexist.example.com.");
-    EXPECT_TRUE(cache->lookup(non_exist_qname, RRType::A(), RRClass::IN(), msg_nxdomain));
+    EXPECT_TRUE(cache->lookup(non_exist_qname, RRType::A(), RRClass::IN(),
+                msg_nxdomain));
 
     RRsetIterator iter = msg_nxdomain.beginSection(Message::SECTION_AUTHORITY);
     RRsetPtr rrset_ptr = *iter;
 
     // The TTL should equal to the TTL of SOA record
     const RRTTL& nxdomain_ttl1 = rrset_ptr->getTTL();
-    EXPECT_EQ(nxdomain_ttl1.getValue(), 86400);
+    // May have already crossed seconds boundary
+    EXPECT_GE(nxdomain_ttl1.getValue(), 86399);
+    EXPECT_LE(nxdomain_ttl1.getValue(), 86400);
 
     // SOA response for example.com
     Message msg_example_com_soa(Message::PARSE);
@@ -68,16 +71,22 @@ TEST_F(NegativeCacheTest, testNXDOMAIN){
 
     msg_example_com_soa.makeResponse();
     Name soa_qname("example.com.");
-    EXPECT_TRUE(cache->lookup(soa_qname, RRType::SOA(), RRClass::IN(), msg_example_com_soa));
+    EXPECT_TRUE(cache->lookup(soa_qname, RRType::SOA(), RRClass::IN(),
+                msg_example_com_soa));
 
     iter = msg_example_com_soa.beginSection(Message::SECTION_ANSWER);
     rrset_ptr = *iter;
 
     // The TTL should equal to the TTL of SOA record in answer section
     const RRTTL& soa_ttl = rrset_ptr->getTTL();
-    EXPECT_EQ(soa_ttl.getValue(), 172800);
+    // May have already crossed seconds boundary
+    EXPECT_GE(soa_ttl.getValue(), 172799);
+    EXPECT_LE(soa_ttl.getValue(), 172800);
 
-    sleep(1);
+    // Sleep for 2 seconds. 2 seconds to make sure the final range check
+    // does not overlap with the original ones (in which case this test
+    // would erroneously pass if the ttl value is not changed)
+    sleep(2);
 
     // Query nonexist.example.com again
     Message msg_nxdomain2(Message::PARSE);
@@ -90,7 +99,8 @@ TEST_F(NegativeCacheTest, testNXDOMAIN){
 
     // The TTL should equal to the TTL of negative response SOA record
     const RRTTL& nxdomain_ttl2 = rrset_ptr->getTTL();
-    EXPECT_TRUE(86398 <= nxdomain_ttl2.getValue() && nxdomain_ttl2.getValue() <= 86399);
+    EXPECT_GE(nxdomain_ttl2.getValue(), 86397);
+    EXPECT_LE(nxdomain_ttl2.getValue(), 86398);
     // No RRset in ANSWER section
     EXPECT_TRUE(msg_nxdomain2.getRRCount(Message::SECTION_ANSWER) == 0);
     // Check that only one SOA record exist in AUTHORITY section
@@ -103,13 +113,15 @@ TEST_F(NegativeCacheTest, testNXDOMAIN){
     Message msg_example_com_soa2(Message::PARSE);
     messageFromFile(msg_example_com_soa2, "message_example_com_soa.wire");
     msg_example_com_soa2.makeResponse();
-    EXPECT_TRUE(cache->lookup(soa_qname, RRType::SOA(), RRClass::IN(), msg_example_com_soa2));
+    EXPECT_TRUE(cache->lookup(soa_qname, RRType::SOA(), RRClass::IN(),
+                              msg_example_com_soa2));
 
     iter = msg_example_com_soa2.beginSection(Message::SECTION_ANSWER);
     rrset_ptr = *iter;
     const RRTTL& soa_ttl2 = rrset_ptr->getTTL();
     // The TTL should equal to the TTL of SOA record in answer section
-    EXPECT_TRUE(172798 <= soa_ttl2.getValue() && soa_ttl2.getValue() <= 172799);
+    EXPECT_GE(soa_ttl2.getValue(), 172797);
+    EXPECT_LE(soa_ttl2.getValue(), 172798);
 }
 
 TEST_F(NegativeCacheTest, testNXDOMAINWithoutSOA){
@@ -166,15 +178,16 @@ TEST_F(NegativeCacheTest, testNoerrorNodata){
     msg_nodata.makeResponse();
 
     Name example_dot_com("example.com.");
-    EXPECT_TRUE(cache->lookup(example_dot_com, RRType::MX(), RRClass::IN(), msg_nodata));
+    EXPECT_TRUE(cache->lookup(example_dot_com, RRType::MX(), RRClass::IN(),
+                msg_nodata));
 
     RRsetIterator iter = msg_nodata.beginSection(Message::SECTION_AUTHORITY);
     RRsetPtr rrset_ptr = *iter;
 
     // The TTL should equal to the TTL of SOA record
     const RRTTL& nodata_ttl1 = rrset_ptr->getTTL();
-    EXPECT_EQ(nodata_ttl1.getValue(), 86400);
-
+    EXPECT_GE(nodata_ttl1.getValue(), 86399);
+    EXPECT_LE(nodata_ttl1.getValue(), 86400);
 
     // Normal SOA response for example.com
     Message msg_example_com_soa(Message::PARSE);
@@ -183,21 +196,26 @@ TEST_F(NegativeCacheTest, testNoerrorNodata){
 
     msg_example_com_soa.makeResponse();
     Name soa_qname("example.com.");
-    EXPECT_TRUE(cache->lookup(soa_qname, RRType::SOA(), RRClass::IN(), msg_example_com_soa));
+    EXPECT_TRUE(cache->lookup(soa_qname, RRType::SOA(), RRClass::IN(),
+                msg_example_com_soa));
 
     iter = msg_example_com_soa.beginSection(Message::SECTION_ANSWER);
     rrset_ptr = *iter;
 
     // The TTL should equal to the TTL of SOA record in answer section
     const RRTTL& soa_ttl = rrset_ptr->getTTL();
-    EXPECT_EQ(soa_ttl.getValue(), 172800);
+    EXPECT_GE(soa_ttl.getValue(), 172799);
+    EXPECT_LE(soa_ttl.getValue(), 172800);
 
     // Query MX record of example.com again
     Message msg_nodata2(Message::PARSE);
     messageFromFile(msg_nodata2, "message_nodata_with_soa.wire");
     msg_nodata2.makeResponse();
 
-    sleep(1);
+    // Sleep for 2 seconds. 2 seconds to make sure the final range check
+    // does not overlap with the original ones (in which case this test
+    // would erroneously pass if the ttl value is not changed)
+    sleep(2);
 
     EXPECT_TRUE(cache->lookup(example_dot_com, RRType::MX(), RRClass::IN(), msg_nodata2));
 
@@ -209,9 +227,11 @@ TEST_F(NegativeCacheTest, testNoerrorNodata){
     iter = msg_nodata2.beginSection(Message::SECTION_AUTHORITY);
     rrset_ptr = *iter;
 
-    // The TTL should equal to the TTL of negative response SOA record and counted down
+    // The TTL should equal to the TTL of negative response SOA record
+    // and counted down
     const RRTTL& nodata_ttl2 = rrset_ptr->getTTL();
-    EXPECT_TRUE(86398 <= nodata_ttl2.getValue() && nodata_ttl2.getValue() <= 86399);
+    EXPECT_GE(nodata_ttl2.getValue(), 86397);
+    EXPECT_LE(nodata_ttl2.getValue(), 86398);
 }
 
 TEST_F(NegativeCacheTest, testReferralResponse){

src/lib/datasrc/memory_datasrc.cc

@@ -12,16 +12,6 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <algorithm>
-#include <map>
-#include <utility>
-#include <cctype>
-#include <cassert>
-
-#include <boost/shared_ptr.hpp>
-#include <boost/scoped_ptr.hpp>
-#include <boost/bind.hpp>
-
 #include <exceptions/exceptions.h>
 
 #include <dns/name.h>
@@ -39,6 +29,17 @@
 #include <datasrc/data_source.h>
 #include <datasrc/factory.h>
 
+#include <boost/shared_ptr.hpp>
+#include <boost/scoped_ptr.hpp>
+#include <boost/bind.hpp>
+#include <boost/foreach.hpp>
+
+#include <algorithm>
+#include <map>
+#include <utility>
+#include <cctype>
+#include <cassert>
+
 using namespace std;
 using namespace isc::dns;
 using namespace isc::dns::rdata;
@@ -47,8 +48,15 @@ using boost::scoped_ptr;
 namespace isc {
 namespace datasrc {
 
+using namespace internal;
+
 namespace {
 // Some type aliases
+
+// RRset specified for this implementation
+typedef boost::shared_ptr<internal::RBNodeRRset> RBNodeRRsetPtr;
+typedef boost::shared_ptr<const internal::RBNodeRRset> ConstRBNodeRRsetPtr;
+
 /*
  * Each domain consists of some RRsets. They will be looked up by the
  * RRType.
@@ -60,19 +68,44 @@ namespace {
  * critical place and map has better interface for the lookups, so we use
  * that.
  */
-typedef map<RRType, ConstRRsetPtr> Domain;
+typedef map<RRType, ConstRBNodeRRsetPtr> Domain;
 typedef Domain::value_type DomainPair;
 typedef boost::shared_ptr<Domain> DomainPtr;
 // The tree stores domains
 typedef RBTree<Domain> DomainTree;
 typedef RBNode<Domain> DomainNode;
 
+// In the following dedicated namespace we define a few application-specific
+// RBNode flags.  We use a separate namespace so we can consolidate the
+// definition in a single place, which would hopefully reduce the risk of
+// collisions.
+// (Note: it's within an unnamed namespace, so effectively private.)
+namespace domain_flag {
+// This flag indicates the node is at a "wildcard level" (in short, it means
+// one of the node's immediate child is a wildcard).  See addWildcards()
+// for more details.
+const DomainNode::Flags WILD = DomainNode::FLAG_USER1;
+
+// This flag is used for additional record shortcut.  If a node has this
+// flag, it's under a zone cut for a delegation to a child zone.
+// Note: for a statically built zone this information is stable, but if we
+// change the implementation to be dynamically modifiable, it may not be
+// realistic to keep this flag update for all affected nodes, and we may
+// have to reconsider the mechanism.
+const DomainNode::Flags GLUE = DomainNode::FLAG_USER2;
+};
+
 // Separate storage for NSEC3 RRs (and their RRSIGs).  It's an STL map
 // from string to the NSEC3 RRset.  The map key is the first label
 // (upper cased) of the owner name of the corresponding NSEC3 (i.e., map
 // value).  We can use  the standard string comparison (if the comparison
 // target is also upper cased) due to the nature of NSEC3 owner names.
-typedef map<string, ConstRRsetPtr> NSEC3Map;
+//
+// Note: We maintain the RRsets in the form of RBNodeRRset even if they are
+// not stored in the RB tree.  The reason is because comparison can be
+// more efficient if we make sure all RRsets returned from this module are
+// of the same type.
+typedef map<string, ConstRBNodeRRsetPtr> NSEC3Map;
 typedef NSEC3Map::value_type NSEC3Pair;
 
 // Actual zone data: Essentially a set of zone's RRs.  This is defined as
@@ -106,6 +139,275 @@ struct ZoneData {
 };
 }
 
+namespace internal {
+
+/// \brief An encapsulation type for a pointer of an additional node
+/// associated with an \c RBNodeRRset object.
+///
+/// Currently this is defined as a structure only so that it can declared
+/// in rbnode_rrset.h; this is essentially a pointer to \c DomainNode.
+/// In future, however, this structure may have other attributes.
+struct AdditionalNodeInfo {
+    AdditionalNodeInfo(DomainNode* node) : node_(node) {}
+    DomainNode* node_;
+};
+
+//
+// RBNodeRRset details
+//
+struct RBNodeRRsetImpl {
+public:
+    RBNodeRRsetImpl(const ConstRRsetPtr& rrset) : rrset_(rrset)
+    {}
+
+    ConstRRsetPtr rrset_;     ///< Underlying RRset
+    scoped_ptr<vector<AdditionalNodeInfo> > additionals_;
+};
+
+RBNodeRRset::RBNodeRRset(const ConstRRsetPtr& rrset) :
+    impl_(new RBNodeRRsetImpl(rrset))
+{
+}
+
+RBNodeRRset::~RBNodeRRset() {
+    delete impl_;
+}
+
+unsigned int
+RBNodeRRset::getRdataCount() const {
+    return (impl_->rrset_->getRdataCount());
+}
+
+const Name&
+RBNodeRRset::getName() const {
+    return (impl_->rrset_->getName());
+}
+
+const RRClass&
+RBNodeRRset::getClass() const {
+    return (impl_->rrset_->getClass());
+}
+
+const RRType&
+RBNodeRRset::getType() const {
+    return (impl_->rrset_->getType());
+}
+
+const RRTTL&
+RBNodeRRset::getTTL() const {
+    return (impl_->rrset_->getTTL());
+}
+
+void
+RBNodeRRset::setName(const Name&) {
+    isc_throw(isc::NotImplemented, "RBNodeRRset::setName() not supported");
+}
+
+void
+RBNodeRRset::setTTL(const RRTTL&) {
+    isc_throw(isc::NotImplemented, "RBNodeRRset::setTTL() not supported");
+}
+
+string
+RBNodeRRset::toText() const {
+    return (impl_->rrset_->toText());
+}
+
+unsigned int
+RBNodeRRset::toWire(AbstractMessageRenderer& renderer) const {
+    return (impl_->rrset_->toWire(renderer));
+}
+
+unsigned int
+RBNodeRRset::toWire(isc::util::OutputBuffer& buffer) const {
+    return (impl_->rrset_->toWire(buffer));
+}
+
+void
+RBNodeRRset::addRdata(ConstRdataPtr) {
+    isc_throw(isc::NotImplemented, "RBNodeRRset::addRdata() not supported");
+}
+
+void
+RBNodeRRset::addRdata(const Rdata&) {
+    isc_throw(isc::NotImplemented, "RBNodeRRset::addRdata() not supported");
+}
+
+RdataIteratorPtr
+RBNodeRRset::getRdataIterator() const {
+    return (impl_->rrset_->getRdataIterator());
+}
+
+RRsetPtr
+RBNodeRRset::getRRsig() const {
+    return (impl_->rrset_->getRRsig());
+}
+
+void
+RBNodeRRset::addRRsig(const ConstRdataPtr& rdata) {
+    AbstractRRset* p = const_cast<AbstractRRset*>(impl_->rrset_.get());
+    p->addRRsig(rdata);
+}
+
+void
+RBNodeRRset::addRRsig(const RdataPtr& rdata) {
+    AbstractRRset* p = const_cast<AbstractRRset*>(impl_->rrset_.get());
+    p->addRRsig(rdata);
+}
+
+void
+RBNodeRRset::addRRsig(const AbstractRRset& sigs) {
+    AbstractRRset* p = const_cast<AbstractRRset*>(impl_->rrset_.get());
+    p->addRRsig(sigs);
+}
+
+void
+RBNodeRRset::addRRsig(const ConstRRsetPtr& sigs) {
+    AbstractRRset* p = const_cast<AbstractRRset*>(impl_->rrset_.get());
+    p->addRRsig(sigs);
+}
+
+void
+RBNodeRRset::addRRsig(const RRsetPtr& sigs) {
+    AbstractRRset* p = const_cast<AbstractRRset*>(impl_->rrset_.get());
+    p->addRRsig(sigs);
+}
+
+void
+RBNodeRRset::removeRRsig() {
+    AbstractRRset* p = const_cast<AbstractRRset*>(impl_->rrset_.get());
+    p->removeRRsig();
+}
+
+ConstRRsetPtr
+RBNodeRRset::getUnderlyingRRset() const {
+    return (impl_->rrset_);
+}
+
+void
+RBNodeRRset::addAdditionalNode(const AdditionalNodeInfo& additional) {
+    // Lazy initialization
+    if (!impl_->additionals_) {
+        impl_->additionals_.reset(new vector<AdditionalNodeInfo>);
+    }
+    impl_->additionals_->push_back(additional);
+}
+
+const vector<AdditionalNodeInfo>*
+RBNodeRRset::getAdditionalNodes() const {
+    return (impl_->additionals_.get());
+}
+
+void
+RBNodeRRset::copyAdditionalNodes(RBNodeRRset& dst) const {
+    if (impl_->additionals_) {
+        dst.impl_->additionals_.reset(
+            new vector<AdditionalNodeInfo>(impl_->additionals_->begin(),
+                                           impl_->additionals_->end()));
+    }
+}
+
+} // end of internal
+
+namespace {
+// Specialized version of ZoneFinder::ResultContext, which specifically
+// holds rrset in the form of RBNodeRRset.
+struct RBNodeResultContext {
+    /// \brief Constructor
+    ///
+    /// The first three parameters correspond to those of
+    /// ZoneFinder::ResultContext.  If node is non NULL, it specifies the
+    /// found RBNode in the search.
+    RBNodeResultContext(ZoneFinder::Result code_param,
+                        ConstRBNodeRRsetPtr rrset_param,
+                        ZoneFinder::FindResultFlags flags_param,
+                        const DomainNode* node) :
+        code(code_param), rrset(rrset_param), flags(flags_param),
+        found_node(node)
+    {}
+
+    const ZoneFinder::Result code;
+    const ConstRBNodeRRsetPtr rrset;
+    const ZoneFinder::FindResultFlags flags;
+    const DomainNode* const found_node;
+};
+}
+
+class InMemoryZoneFinder::Context : public ZoneFinder::Context {
+public:
+    /// \brief Constructor.
+    ///
+    /// Note that we don't have a specific constructor for the findAll() case.
+    /// For (successful) type ANY query, found_node points to the
+    /// corresponding RB node, which is recorded within this specialized
+    /// context.
+    Context(ZoneFinder& finder, ZoneFinder::FindOptions options,
+            const RBNodeResultContext& result) :
+        ZoneFinder::Context(finder, options,
+                            ResultContext(result.code, result.rrset,
+                                          result.flags)),
+        rrset_(result.rrset), found_node_(result.found_node)
+    {}
+
+protected:
+    virtual void getAdditionalImpl(const vector<RRType>& requested_types,
+                                   vector<ConstRRsetPtr>& result)
+    {
+        if (!rrset_) {
+            // In this case this context should encapsulate the result of
+            // findAll() and found_node_ should point to a valid answer node.
+            if (found_node_ == NULL || found_node_->isEmpty()) {
+                isc_throw(isc::Unexpected,
+                          "Invalid call to in-memory getAdditional: caller's "
+                          "bug or broken zone");
+            }
+            BOOST_FOREACH(const DomainPair& dom_it, *found_node_->getData()) {
+                getAdditionalForRRset(*dom_it.second, requested_types,
+                                      result);
+            }
+        } else {
+            getAdditionalForRRset(*rrset_, requested_types, result);
+        }
+    }
+
+private:
+    // Retrieve additional RRsets for a given RRset associated in the context.
+    // The process is straightforward: it examines the link to
+    // AdditionalNodeInfo vector (if set), and find RRsets of the requested
+    // type for each node.
+    static void getAdditionalForRRset(const RBNodeRRset& rrset,
+                                      const vector<RRType>& requested_types,
+                                      vector<ConstRRsetPtr>& result)
+    {
+        const vector<AdditionalNodeInfo>* additionals_ =
+            rrset.getAdditionalNodes();
+        if (additionals_ == NULL) {
+            return;
+        }
+        const bool glue_ok = (rrset.getType() == RRType::NS());
+        BOOST_FOREACH(const AdditionalNodeInfo& additional, *additionals_) {
+            assert(additional.node_ != NULL);
+            if (additional.node_->isEmpty()) {
+                continue;
+            }
+            if (!glue_ok && additional.node_->getFlag(domain_flag::GLUE)) {
+                continue;
+            }
+            BOOST_FOREACH(const RRType& rrtype, requested_types) {
+                Domain::const_iterator found =
+                    additional.node_->getData()->find(rrtype);
+                if (found != additional.node_->getData()->end()) {
+                    // TODO: wildcard consideration
+                    result.push_back(found->second);
+                }
+            }
+        }
+    }
+
+    const ConstRBNodeRRsetPtr rrset_;
+    const DomainNode* const found_node_;
+};
+
 // Private data and hidden methods of InMemoryZoneFinder
 struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
     // Constructor
@@ -114,8 +416,6 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
         zone_data_(new ZoneData(origin_))
     {}
 
-    static const DomainNode::Flags DOMAINFLAG_WILD = DomainNode::FLAG_USER1;
-
     // Information about the zone
     RRClass zone_class_;
     Name origin_;
@@ -154,7 +454,7 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
                                                          &node));
                 assert(result == DomainTree::SUCCESS ||
                        result == DomainTree::ALREADYEXISTS);
-                node->setFlag(DOMAINFLAG_WILD);
+                node->setFlag(domain_flag::WILD);
 
                 // Ensure a separate level exists for the wildcard name.
                 // Note: for 'name' itself we do this later anyway, but the
@@ -407,7 +707,8 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
             return (result::EXIST);
         }
 
-        zone_data.nsec3_data_->map_.insert(NSEC3Pair(fst_label, rrset));
+        zone_data.nsec3_data_->map_.insert(
+            NSEC3Pair(fst_label, ConstRBNodeRRsetPtr(new RBNodeRRset(rrset))));
         return (result::SUCCESS);
     }
 
@@ -416,7 +717,9 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
      * access is without the impl_-> and it will get inlined anyway.
      */
     // Implementation of InMemoryZoneFinder::add
-    result::Result add(const ConstRRsetPtr& rawrrset, ZoneData& zone_data) {
+    result::Result add(const ConstRRsetPtr& rawrrset, ZoneData& zone_data,
+                       vector<RBNodeRRset*>* need_additionals)
+    {
         // Sanitize input.  This will cause an exception to be thrown
         // if the input RRset is empty.
         addValidation(rawrrset);
@@ -428,7 +731,7 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
         // ... although instead of loading the RRset directly, we encapsulate
         // it within an RBNodeRRset.  This contains additional information that
         // speeds up queries.
-        ConstRRsetPtr rrset(new internal::RBNodeRRset(rawrrset));
+        RBNodeRRsetPtr rrset(new RBNodeRRset(rawrrset));
 
         if (rrset->getType() == RRType::NSEC3()) {
             return (addNSEC3(rrset, zone_data));
@@ -485,6 +788,12 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
                 node->setFlag(DomainNode::FLAG_CALLBACK);
             }
 
+            if (need_additionals != NULL &&
+                (rrset->getType() == RRType::NS() ||
+                 rrset->getType() == RRType::MX())) {
+                need_additionals->push_back(rrset.get());
+            }
+
             // If we've added NSEC3PARAM at zone origin, set up NSEC3 specific
             // data or check consistency with already set up parameters.
             if (rrset->getType() == RRType::NSEC3PARAM() &&
@@ -513,8 +822,10 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
      * Same as above, but it checks the return value and if it already exists,
      * it throws.
      */
-    void addFromLoad(const ConstRRsetPtr& set, ZoneData* zone_data) {
-        switch (add(set, *zone_data)) {
+    void addFromLoad(const ConstRRsetPtr& set, ZoneData* zone_data,
+                     vector<RBNodeRRset*>* need_additionals)
+    {
+        switch (add(set, *zone_data, need_additionals)) {
         case result::EXIST:
             LOG_ERROR(logger, DATASRC_MEM_DUP_RRSET).
                 arg(set->getName()).arg(set->getType());
@@ -540,7 +851,7 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
         {}
         const DomainNode* zonecut_node_;
         const DomainNode* dname_node_;
-        ConstRRsetPtr rrset_;
+        ConstRBNodeRRsetPtr rrset_;
         const FindOptions options_;
     };
 
@@ -613,18 +924,19 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
      * It is designed for wildcard case, where we create the rrsets
      * dynamically.
      */
-    static ConstRRsetPtr prepareRRset(const Name& name,
-                                      const ConstRRsetPtr& rrset,
-                                      bool rename, FindOptions options)
+    static ConstRBNodeRRsetPtr prepareRRset(const Name& name,
+                                            const ConstRBNodeRRsetPtr& rrset,
+                                            bool rename, FindOptions options)
     {
         if (rename) {
             LOG_DEBUG(logger, DBG_TRACE_DETAILED, DATASRC_MEM_RENAME).
                 arg(rrset->getName()).arg(name);
-            RRsetPtr result(new RRset(name, rrset->getClass(),
-                                      rrset->getType(), rrset->getTTL()));
+            RRsetPtr result_base(new RRset(name, rrset->getClass(),
+                                           rrset->getType(),
+                                           rrset->getTTL()));
             for (RdataIteratorPtr i(rrset->getRdataIterator()); !i->isLast();
                  i->next()) {
-                result->addRdata(i->getCurrent());
+                result_base->addRdata(i->getCurrent());
             }
             if ((options & FIND_DNSSEC) != 0) {
                 ConstRRsetPtr sig_rrset = rrset->getRRsig();
@@ -638,9 +950,11 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
                     {
                         result_sig->addRdata(i->getCurrent());
                     }
-                    result->addRRsig(result_sig);
+                    result_base->addRRsig(result_sig);
                 }
             }
+            RBNodeRRsetPtr result(new RBNodeRRset(result_base));
+            rrset->copyAdditionalNodes(*result);
             return (result);
         } else {
             return (rrset);
@@ -651,8 +965,13 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
     // account wildcard matches and DNSSEC information.  We set the NSEC/NSEC3
     // flag when applicable regardless of the find option; the caller would
     // simply ignore these when they didn't request DNSSEC related results.
-    ResultContext createFindResult(Result code, ConstRRsetPtr rrset,
-                                   bool wild = false) const
+    // When the optional parameter 'node' is given (in which case it should be
+    // non NULL), it means it's a result of ANY query and the context should
+    // remember the matched node.
+    RBNodeResultContext createFindResult(Result code,
+                                         ConstRBNodeRRsetPtr rrset,
+                                         bool wild = false,
+                                         const DomainNode* node = NULL) const
     {
         FindResultFlags flags = RESULT_DEFAULT;
         if (wild) {
@@ -662,13 +981,13 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
             zone_data_->nsec3_data_) {
             flags = flags | RESULT_NSEC3_SIGNED;
         }
-        return (ZoneFinder::ResultContext(code, rrset, flags));
+        return (RBNodeResultContext(code, rrset, flags, node));
     }
 
     // Implementation of InMemoryZoneFinder::find
-    ZoneFinder::ResultContext find(const Name& name, RRType type,
-                                   std::vector<ConstRRsetPtr>* target,
-                                   const FindOptions options) const
+    RBNodeResultContext find(const Name& name, RRType type,
+                             std::vector<ConstRRsetPtr>* target,
+                             const FindOptions options) const
     {
         LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEM_FIND).arg(name).
             arg(type);
@@ -722,7 +1041,7 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
                     NameComparisonResult::SUPERDOMAIN) {
                     LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_SUPER_STOP).
                         arg(name);
-                    return (createFindResult(NXRRSET, ConstRRsetPtr()));
+                    return (createFindResult(NXRRSET, ConstRBNodeRRsetPtr()));
                 }
 
                 /*
@@ -733,7 +1052,7 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
                  * not match according to 4.3.3 of RFC 1034 (the query name
                  * is known to exist).
                  */
-                if (node->getFlag(DOMAINFLAG_WILD)) {
+                if (node->getFlag(domain_flag::WILD)) {
                     /* Should we cancel this match?
                      *
                      * If we compare with some node and get a common ancestor,
@@ -761,7 +1080,8 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
                         getLastComparisonResult().getCommonLabels() > 1) {
                         LOG_DEBUG(logger, DBG_TRACE_DATA,
                                      DATASRC_MEM_WILDCARD_CANCEL).arg(name);
-                        return (createFindResult(NXDOMAIN, ConstRRsetPtr(),
+                        return (createFindResult(NXDOMAIN,
+                                                 ConstRBNodeRRsetPtr(),
                                                  false));
                     }
                     const Name wildcard(Name("*").concatenate(
@@ -769,7 +1089,7 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
                     DomainTree::Result result =
                         zone_data_->domains_.find(wildcard, &node);
                     /*
-                     * Otherwise, why would the DOMAINFLAG_WILD be there if
+                     * Otherwise, why would the domain_flag::WILD be there if
                      * there was no wildcard under it?
                      */
                     assert(result == DomainTree::EXACTMATCH);
@@ -787,7 +1107,8 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
             case DomainTree::NOTFOUND:
                 LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_NOT_FOUND).
                     arg(name);
-                return (createFindResult(NXDOMAIN, ConstRRsetPtr(), false));
+                return (createFindResult(NXDOMAIN, ConstRBNodeRRsetPtr(),
+                                         false));
             case DomainTree::EXACTMATCH: // This one is OK, handle it
                 break;
             default:
@@ -800,7 +1121,7 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
         if (node->isEmpty()) {
             LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_DOMAIN_EMPTY).
                 arg(name);
-            return (createFindResult(NXRRSET, ConstRRsetPtr(), rename));
+            return (createFindResult(NXRRSET, ConstRBNodeRRsetPtr(), rename));
         }
 
         Domain::const_iterator found;
@@ -832,7 +1153,8 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
             }
             LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_ANY_SUCCESS).
                 arg(name);
-            return (createFindResult(SUCCESS, ConstRRsetPtr(), rename));
+            return (createFindResult(SUCCESS, ConstRBNodeRRsetPtr(), rename,
+                                     node));
         }
 
         found = node->getData()->find(type);
@@ -858,7 +1180,7 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
         // No exact match or CNAME.  Return NXRRSET.
         LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_NXRRSET).arg(type).
             arg(name);
-        return (createFindResult(NXRRSET, ConstRRsetPtr(), rename));
+        return (createFindResult(NXRRSET, ConstRBNodeRRsetPtr(), rename));
     }
 };
 
@@ -890,8 +1212,8 @@ InMemoryZoneFinder::find(const Name& name, const RRType& type,
                          const FindOptions options)
 {
     return (ZoneFinderContextPtr(
-                new Context(*this, options,
-                            impl_->find(name, type, NULL, options))));
+                new Context(*this, options, impl_->find(name, type, NULL,
+                                                        options))));
 }
 
 ZoneFinderContextPtr
@@ -901,8 +1223,7 @@ InMemoryZoneFinder::findAll(const Name& name,
 {
     return (ZoneFinderContextPtr(
                 new Context(*this, options, impl_->find(name, RRType::ANY(),
-                                                        &target, options),
-                            target)));
+                                                        &target, options))));
 }
 
 ZoneFinder::FindNSEC3Result
@@ -934,7 +1255,7 @@ InMemoryZoneFinder::findNSEC3(const Name& name, bool recursive) {
     const unsigned int olabels = impl_->origin_.getLabelCount();
     const unsigned int qlabels = name.getLabelCount();
 
-    ConstRRsetPtr covering_proof; // placeholder of the next closer proof
+    ConstRBNodeRRsetPtr covering_proof; // placeholder of the next closer proof
     // Examine all names from the query name to the origin name, stripping
     // the deepest label one by one, until we find a name that has a matching
     // NSEC3 hash.
@@ -984,20 +1305,101 @@ InMemoryZoneFinder::findNSEC3(const Name& name, bool recursive) {
 
 result::Result
 InMemoryZoneFinder::add(const ConstRRsetPtr& rrset) {
-    return (impl_->add(rrset, *impl_->zone_data_));
+    return (impl_->add(rrset, *impl_->zone_data_, NULL));
+}
+
+namespace {
+// This should eventually be more generalized.
+const Name
+getAdditionalName(RRType rrtype, const rdata::Rdata& rdata) {
+    if (rrtype == RRType::NS()) {
+        const generic::NS& ns = dynamic_cast<const generic::NS&>(rdata);
+        return (ns.getNSName());
+    } else {
+        // In our usage the only other possible case is MX.
+        assert(rrtype == RRType::MX());
+        const generic::MX& mx = dynamic_cast<const generic::MX&>(rdata);
+        return (mx.getMXName());
+    }
+}
+
+bool
+checkZoneCut(const DomainNode& node, pair<bool, bool>* arg) {
+    // We are only interested in the highest zone cut information.
+    // Ignore others and continue the search.
+    if (arg->first) {
+        return (false);
+    }
+    // Once we encounter a delegation point due to a DNAME, anything under it
+    // should be hidden.
+    if (node.getData()->find(RRType::DNAME()) != node.getData()->end()) {
+        return (true);
+    } else if (node.getData()->find(RRType::NS()) != node.getData()->end()) {
+        arg->first = true;
+        arg->second = true;
+        return (false);
+    }
+    return (false);
 }
 
+void
+addAdditional(RBNodeRRset* rrset, ZoneData* zone_data) {
+    RdataIteratorPtr rdata_iterator = rrset->getRdataIterator();
+    for (; !rdata_iterator->isLast(); rdata_iterator->next()) {
+        // For each domain name that requires additional section processing
+        // in each RDATA, search the tree for the name and remember it if
+        // found.  If the name is under a zone cut (for a delegation to a
+        // child zone), mark the node as "GLUE", so we can selectively
+        // include/exclude them when we use it.
+
+        // TODO: wildcard
+        RBTreeNodeChain<Domain> node_path;
+        DomainNode* node = NULL;
+        // The callback argument is a pair of bools: the first is a flag to
+        // only check the highest cut; the second one records whether the
+        // search goes under a zone cut.
+        pair<bool, bool> callback_arg(false, false);
+        const DomainTree::Result result =
+            zone_data->domains_.find(
+                getAdditionalName(rrset->getType(),
+                                  rdata_iterator->getCurrent()),
+                &node, node_path, checkZoneCut, &callback_arg);
+        if (result == DomainTree::EXACTMATCH) {
+            assert(node != NULL);
+            if (callback_arg.second ||
+                (node->getFlag(DomainNode::FLAG_CALLBACK) &&
+                 node->getData()->find(RRType::NS()) !=
+                 node->getData()->end())) {
+                // The node is under or at a zone cut; mark it as a glue.
+                node->setFlag(domain_flag::GLUE);
+            }
+            // Note that node may be empty.  We should keep it in the list
+            // in case we dynamically update the tree and it becomes non empty
+            // (which is not supported yet)
+            rrset->addAdditionalNode(node);
+        }
+    }
+}
+}
 
 void
 InMemoryZoneFinder::load(const string& filename) {
     LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEM_LOAD).arg(getOrigin()).
         arg(filename);
-    // Load it into temporary zone data
+    // Load it into temporary zone data.  As we build the zone, we record
+    // the (RBNode)RRsets that needs to be associated with additional
+    // information in 'need_additionals'.
+    vector<RBNodeRRset*> need_additionals;
     scoped_ptr<ZoneData> tmp(new ZoneData(getOrigin()));
 
     masterLoad(filename.c_str(), getOrigin(), getClass(),
                boost::bind(&InMemoryZoneFinderImpl::addFromLoad, impl_,
-                           _1, tmp.get()));
+                           _1, tmp.get(), &need_additionals));
+
+    // For each RRset in need_additionals, identify the corresponding
+    // RBnode for additional processing and associate it in the RRset.
+    for_each(need_additionals.begin(), need_additionals.end(),
+             boost::bind(addAdditional, _1, tmp.get()));
 
     // If the zone is NSEC3-signed, check if it has NSEC3PARAM
     if (tmp->nsec3_data_) {

src/lib/datasrc/memory_datasrc.h

@@ -218,6 +218,12 @@ private:
     // extracts the pointer to data and puts it into the iterator.
     // The access is read only.
     friend class InMemoryClient;
+
+    /// \brief In-memory version of finder context.
+    ///
+    /// The implementation (and any specialized interface) is completely local
+    /// to the InMemoryZoneFinder class, so it's defined as private
+    class Context;
 };
 
 /// \brief A data source client that holds all necessary data in memory.

src/lib/datasrc/rbnode_rrset.h

@@ -24,11 +24,27 @@
 #include <util/buffer.h>
 
 #include <string>
+#include <vector>
 
 namespace isc {
 namespace datasrc {
 namespace internal {
 
+/// \brief The actual content of \c RBNodeRRset
+///
+///  This is defined in the namespace-scope (not hidden in the main class)
+/// so that the In-memory data source implementation can refer to it.
+struct RBNodeRRsetImpl;
+
+// Forward declaration of an opaque data type defined and used within the
+// implementation.  This is public only because it needs to be used within
+// the in-memory data source implementation, but conceptually this is a
+// private type for the in-memory data source implementation.
+// Note that the definition of the structure is still hidden within the
+// implementation, so, basically, a normal application should never be able
+// to use it directly even if it peeks into the "internal" namespace.
+struct AdditionalNodeInfo;
+
 /// \brief Special RRset for optimizing memory datasource requirement
 ///
 /// To speed up the performance of the in-memory data source, at load time
@@ -84,11 +100,10 @@ public:
     /// Creates an RBNodeRRset from the pointer to the RRset passed to it.
     ///
     /// \param rrset Pointer to underlying RRset encapsulated by this object.
-    explicit RBNodeRRset(const isc::dns::ConstRRsetPtr& rrset) : rrset_(rrset)
-    {}
+    explicit RBNodeRRset(const isc::dns::ConstRRsetPtr& rrset);
 
     /// \brief Destructor
-    virtual ~RBNodeRRset() {}
+    virtual ~RBNodeRRset();
 
     // Getter and Setter Methods
     //
@@ -96,64 +111,48 @@ public:
     // setter methods thrown an exception - this specialisation of the RRset
     // object does not expect the underlying RRset to be modified.
 
-    virtual unsigned int getRdataCount() const {
-        return (rrset_->getRdataCount());
-    }
+    virtual unsigned int getRdataCount() const;
 
-    virtual const isc::dns::Name& getName() const {
-        return (rrset_->getName());
-    }
+    virtual const isc::dns::Name& getName() const;
 
-    virtual const isc::dns::RRClass& getClass() const {
-        return (rrset_->getClass());
-    }
+    virtual const isc::dns::RRClass& getClass() const;
 
-    virtual const isc::dns::RRType& getType() const {
-        return (rrset_->getType());
-    }
+    virtual const isc::dns::RRType& getType() const;
 
-    virtual const isc::dns::RRTTL& getTTL() const {
-        return (rrset_->getTTL());
-    }
+    virtual const isc::dns::RRTTL& getTTL() const;
 
-    virtual void setName(const isc::dns::Name&) {
-        isc_throw(isc::NotImplemented, "RBNodeRRset::setName() not supported");
-    }
+    virtual void setName(const isc::dns::Name&);
 
-    virtual void setTTL(const isc::dns::RRTTL&) {
-        isc_throw(isc::NotImplemented, "RBNodeRRset::setTTL() not supported");
-    }
+    virtual void setTTL(const isc::dns::RRTTL&);
 
-    virtual std::string toText() const {
-        return (rrset_->toText());
+    virtual std::string toText() const;
+
+    virtual bool isSameKind(const AbstractRRset& other) const {
+        // This code is an optimisation for comparing
+        // RBNodeRRsets. However, in doing this optimisation,
+        // semantically the code is not "is same kind" but is instead
+        // "is identical object" in the case where RBNodeRRsets are compared.
+
+        const RBNodeRRset* rb = dynamic_cast<const RBNodeRRset*>(&other);
+        if (rb != NULL) {
+            return (this == rb);
+        } else {
+            return (AbstractRRset::isSameKind(other));
+        }
     }
 
     virtual unsigned int toWire(
-            isc::dns::AbstractMessageRenderer& renderer) const {
-        return (rrset_->toWire(renderer));
-    }
+        isc::dns::AbstractMessageRenderer& renderer) const;
 
-    virtual unsigned int toWire(isc::util::OutputBuffer& buffer) const {
-        return (rrset_->toWire(buffer));
-    }
+    virtual unsigned int toWire(isc::util::OutputBuffer& buffer) const;
 
-    virtual void addRdata(isc::dns::rdata::ConstRdataPtr) {
-        isc_throw(isc::NotImplemented,
-                  "RBNodeRRset::addRdata() not supported");
-    }
+    virtual void addRdata(isc::dns::rdata::ConstRdataPtr);
 
-    virtual void addRdata(const isc::dns::rdata::Rdata&) {
-        isc_throw(isc::NotImplemented,
-                  "RBNodeRRset::addRdata() not supported");
-    }
+    virtual void addRdata(const isc::dns::rdata::Rdata&);
 
-    virtual isc::dns::RdataIteratorPtr getRdataIterator() const {
-        return (rrset_->getRdataIterator());
-    }
+    virtual isc::dns::RdataIteratorPtr getRdataIterator() const;
 
-    virtual isc::dns::RRsetPtr getRRsig() const {
-        return (rrset_->getRRsig());
-    }
+    virtual isc::dns::RRsetPtr getRRsig() const;
 
     // With all the RRsig methods, we have the problem that we store the
     // underlying RRset using a ConstRRsetPtr - a pointer to a "const" RRset -
@@ -161,45 +160,65 @@ public:
     // this by temporarily violating the "const" nature of the RRset to add the
     // data.
 
-    virtual void addRRsig(const isc::dns::rdata::ConstRdataPtr& rdata) {
-        AbstractRRset* p = const_cast<AbstractRRset*>(rrset_.get());
-        p->addRRsig(rdata);
-    }
+    virtual void addRRsig(const isc::dns::rdata::ConstRdataPtr& rdata);
 
-    virtual void addRRsig(const isc::dns::rdata::RdataPtr& rdata) {
-        AbstractRRset* p = const_cast<AbstractRRset*>(rrset_.get());
-        p->addRRsig(rdata);
-    }
+    virtual void addRRsig(const isc::dns::rdata::RdataPtr& rdata);
 
-    virtual void addRRsig(const AbstractRRset& sigs) {
-        AbstractRRset* p = const_cast<AbstractRRset*>(rrset_.get());
-        p->addRRsig(sigs);
-    }
+    virtual void addRRsig(const AbstractRRset& sigs);
 
-    virtual void addRRsig(const isc::dns::ConstRRsetPtr& sigs) {
-        AbstractRRset* p = const_cast<AbstractRRset*>(rrset_.get());
-        p->addRRsig(sigs);
-    }
+    virtual void addRRsig(const isc::dns::ConstRRsetPtr& sigs);
 
-    virtual void addRRsig(const isc::dns::RRsetPtr& sigs) {
-        AbstractRRset* p = const_cast<AbstractRRset*>(rrset_.get());
-        p->addRRsig(sigs);
-    }
+    virtual void addRRsig(const isc::dns::RRsetPtr& sigs);
 
-    virtual void removeRRsig() {
-        AbstractRRset* p = const_cast<AbstractRRset*>(rrset_.get());
-        p->removeRRsig();
-    }
+    virtual void removeRRsig();
+
+    /// \brief Associate a link to an RB node of the additional record.
+    ///
+    /// This method adds a given opaque object that holds a link to an RB node
+    /// of the underlying in-memory data source that is corresponding to an
+    /// RDATA of this RRset.
+    ///
+    /// This method is exposed as public so it can be used within the in-memory
+    /// data source implementation, and only for that purpose.
+    ///
+    /// \param additional An opaque \c AdditionalNodeInfo object to be
+    /// associated with this RRset.
+    void addAdditionalNode(const AdditionalNodeInfo& additional);
+
+    /// \brief Return a pointer to the list (vector) of additional RB nodes.
+    ///
+    /// This method returns a pointer to a vector storing the opaque
+    /// \c AdditionalNodeInfo object that may be possibly set in this RRset.
+    /// Not all RRsets are associated with additional nodes; if no
+    /// such node is stored, this method returns NULL.
+    ///
+    /// Like \c addAdditionalNode(), this method is exposed as public only for
+    /// the in-memory data source implementation.
+    ///
+    /// \return A pointer to the associated vector of \c AdditionalNodeInfo;
+    /// NULL if no additional nodes are associated to this RRset.
+    const std::vector<AdditionalNodeInfo>* getAdditionalNodes() const;
+
+    /// \brief Copy the list of additional RB nodes to another RRset.
+    ///
+    /// This method copies the internal list (an STL vector in the actual
+    /// implementation) of additional RB nodes for this RRset to another
+    /// \c RBNodeRRset object.  The copy destination is generally expected to
+    /// be newly created and have an empty list, but this method does not
+    /// check the condition.  If the destination already has a non empty list,
+    /// the existing entries will be lost.
+    ///
+    /// \param dst The \c RBNodeRRset object to which the additional
+    /// RB node list is to be copied.
+    void copyAdditionalNodes(RBNodeRRset& dst) const;
 
     /// \brief Return underlying RRset pointer
     ///
     /// ... mainly for testing.
-    isc::dns::ConstRRsetPtr getUnderlyingRRset() const {
-        return (rrset_);
-    }
+    isc::dns::ConstRRsetPtr getUnderlyingRRset() const;
 
 private:
-    isc::dns::ConstRRsetPtr rrset_;     ///< Underlying RRset
+    RBNodeRRsetImpl* impl_;
 };
 
 }   // namespace internal

src/lib/datasrc/rbtree.h

@@ -123,7 +123,8 @@ public:
     /// set to on by the \c setFlag() method.
     enum Flags {
         FLAG_CALLBACK = 1, ///< Callback enabled. See \ref callback
-        FLAG_USER1 = 0x80000000U ///< Application specific flag
+        FLAG_USER1 = 0x80000000U, ///< Application specific flag
+        FLAG_USER2 = 0x40000000U  ///< Application specific flag
     };
 private:
     // Some flag values are expected to be used for internal purposes
@@ -131,7 +132,8 @@ private:
     // limit the settable flags via the \c setFlag() method to those
     // explicitly defined in \c Flags.  This constant represents all
     // such flags.
-    static const uint32_t SETTABLE_FLAGS = (FLAG_CALLBACK | FLAG_USER1);
+    static const uint32_t SETTABLE_FLAGS = (FLAG_CALLBACK | FLAG_USER1 |
+                                            FLAG_USER2);
 
 public:
 

src/lib/datasrc/tests/rbnode_rrset_unittest.cc

@@ -138,6 +138,23 @@ TEST_F(RBNodeRRsetTest, toText) {
     EXPECT_THROW(rrset_a_empty.toText(), EmptyRRset);
 }
 
+TEST_F(RBNodeRRsetTest, isSameKind) {
+    RBNodeRRset rrset_p(ConstRRsetPtr(new RRset(test_name, RRClass::IN(), RRType::A(), RRTTL(3600))));
+    RBNodeRRset rrset_q(ConstRRsetPtr(new RRset(test_name, RRClass::IN(), RRType::A(), RRTTL(3600))));
+    RRset rrset_w(test_name, RRClass::IN(), RRType::A(), RRTTL(3600));
+    RRset rrset_x(test_nsname, RRClass::IN(), RRType::A(), RRTTL(3600));
+    RRset rrset_y(test_name, RRClass::IN(), RRType::NS(), RRTTL(3600));
+    RRset rrset_z(test_name, RRClass::CH(), RRType::A(), RRTTL(3600));
+
+    EXPECT_TRUE(rrset_p.isSameKind(rrset_p));
+    EXPECT_FALSE(rrset_p.isSameKind(rrset_q));
+
+    EXPECT_TRUE(rrset_p.isSameKind(rrset_w));
+    EXPECT_FALSE(rrset_p.isSameKind(rrset_x));
+    EXPECT_FALSE(rrset_p.isSameKind(rrset_y));
+    EXPECT_FALSE(rrset_p.isSameKind(rrset_z));
+}
+
 // Note: although the next two tests are essentially the same and used common
 // test code, they use different test data: the MessageRenderer produces
 // compressed wire data whereas the OutputBuffer does not.

src/lib/datasrc/tests/testdata/.gitignore

@@ -0,0 +1 @@
+/*.sqlite3.copied

src/lib/datasrc/tests/testdata/contexttest.zone

@@ -1,7 +1,7 @@
 ;; test zone file used for ZoneFinderContext tests.
 ;; RRSIGs are (obviouslly) faked ones for testing.
 
-example.org. 3600 IN SOA	ns1.example.org. bugs.x.w.example.org. 22 3600 300 3600000 3600
+example.org. 3600 IN SOA	ns1.example.org. bugs.x.w.example.org. 56 3600 300 3600000 3600
 example.org.			      3600 IN NS	ns1.example.org.
 example.org.			      3600 IN NS	ns2.example.org.
 example.org.			      3600 IN MX	1 mx1.example.org.
@@ -28,8 +28,48 @@ ns2.a.example.org.		      3600 IN A		192.0.2.6
 ns2.a.example.org.		      3600 IN AAAA	2001:db8::6
 mx.a.example.org.		      3600 IN A		192.0.2.7
 
+;; delegation, one of its NS names is at zone cut.
+b.example.org.			      3600 IN NS	ns.b.example.org.
+b.example.org.			      3600 IN NS	b.example.org.
+b.example.org.			      3600 IN AAAA	2001:db8::8
+
+ns.b.example.org.		      3600 IN A		192.0.2.9
+
+;; The MX name is at a zone cut.  shouldn't be included in the
+;; additional section.
+mxatcut.example.org.		      3600 IN MX	1 b.example.org.
+
+;; delegation, one of its NS names is under a DNAME delegation point;
+;; another is at that point; and yet another is under DNAME below a
+;; zone cut.
+c.example.org. 	      	      3600 IN NS	ns.dname.example.org.
+c.example.org. 	      	      3600 IN NS	dname.example.org.
+c.example.org.      	      3600 IN NS	ns.deepdname.example.org.
+ns.dname.example.org.		      3600 IN A		192.0.2.11
+dname.example.org.		      3600 IN A		192.0.2.12
+ns.deepdname.example.org.	      3600 IN AAAA	2001:db8::9
+
+;; delegation, one of its NS name is at an empty non terminal.
+d.example.org. 	      	      3600 IN NS	ns.empty.example.org.
+d.example.org. 	      	      3600 IN NS	ns1.example.org.
+;; by adding these two we can create an empty RB node for
+;; ns.empty.example.org in the in-memory zone
+foo.ns.empty.example.org.     3600 IN A		192.0.2.13
+bar.ns.empty.example.org.     3600 IN A		192.0.2.14
+
+;; delegation; the NS name matches a wildcard (and there's no exact match)
+e.example.org. 	      	      3600 IN NS	ns.wild.example.org.
+*.wild.example.org.	      3600 IN A		192.0.2.15
+
+;; additional for an answer RRset (MX) as a result of wildcard
+;; expansion
+*.wildmx.example.org. 3600 IN MX 1 mx1.example.org.
+
 ;; CNAME
 alias.example.org. 3600 IN CNAME cname.example.org.
 
 ;; DNAME
 dname.example.org. 3600 IN DNAME dname.example.com.
+
+;; DNAME under a NS (strange one)
+deepdname.c.example.org. 3600 IN DNAME deepdname.example.com.

src/lib/datasrc/tests/zone_finder_context_unittest.cc

@@ -214,6 +214,81 @@ TEST_P(ZoneFinderContextTest, getAdditionalDelegation) {
                 result_sets_.begin(), result_sets_.end());
 }
 
+TEST_P(ZoneFinderContextTest, getAdditionalDelegationAtZoneCut) {
+    // Similar to the previous case, but one of the NS addresses is at the
+    // zone cut.
+
+    // XXX: the current database-based data source incorrectly rejects this
+    // setup (see #1771)
+    if (GetParam() == createSQLite3Client) {
+        return;
+    }
+
+    ZoneFinderContextPtr ctx = finder_->find(Name("www.b.example.org"),
+                                             RRType::SOA());
+    EXPECT_EQ(ZoneFinder::DELEGATION, ctx->code);
+
+    ctx->getAdditional(REQUESTED_BOTH, result_sets_);
+    rrsetsCheck("b.example.org. 3600 IN AAAA 2001:db8::8\n"
+                "ns.b.example.org. 3600 IN A 192.0.2.9\n",
+                result_sets_.begin(), result_sets_.end());
+}
+
+TEST_P(ZoneFinderContextTest, getAdditionalDelegationWithDname) {
+    // Delegation: One of the NS names under a DNAME delegation; another
+    // is at the delegation point; yet another is under DNAME below a zone cut.
+    // The first should be hidden.
+    ZoneFinderContextPtr ctx = finder_->find(Name("www.c.example.org"),
+                                             RRType::TXT());
+    EXPECT_EQ(ZoneFinder::DELEGATION, ctx->code);
+
+    ctx->getAdditional(REQUESTED_BOTH, result_sets_);
+    rrsetsCheck("dname.example.org. 3600 IN A 192.0.2.12\n"
+                "ns.deepdname.example.org. 3600 IN AAAA 2001:db8::9\n",
+                result_sets_.begin(), result_sets_.end());
+}
+
+TEST_P(ZoneFinderContextTest, getAdditionalDelegationWithEmptyName) {
+    // One of NS names is at an empty non terminal node.  It shouldn't cause
+    // any disruption.
+    ZoneFinderContextPtr ctx = finder_->find(Name("www.d.example.org"),
+                                             RRType::A());
+    EXPECT_EQ(ZoneFinder::DELEGATION, ctx->code);
+
+    ctx->getAdditional(REQUESTED_BOTH, result_sets_);
+    rrsetsCheck("ns1.example.org. 3600 IN A 192.0.2.1\n"
+                "ns1.example.org. 3600 IN AAAA 2001:db8::1\n",
+                result_sets_.begin(), result_sets_.end());
+}
+
+TEST_P(ZoneFinderContextTest, getAdditionalDelegationWithWild) {
+    // The NS name needs to be expanded by a wildcard.  Currently it doesn't
+    // work for the optimized in-memory version.
+    if (GetParam() == createInMemoryClient) {
+        return;
+    }
+
+    ZoneFinderContextPtr ctx = finder_->find(Name("www.e.example.org"),
+                                             RRType::AAAA());
+    EXPECT_EQ(ZoneFinder::DELEGATION, ctx->code);
+    ctx->getAdditional(REQUESTED_BOTH, result_sets_);
+    rrsetsCheck("ns.wild.example.org. 3600 IN A 192.0.2.15\n",
+                result_sets_.begin(), result_sets_.end());
+}
+
+TEST_P(ZoneFinderContextTest, getAdditionalDelegationForWild) {
+    // additional for an answer RRset (MX) as a result of wildcard expansion.
+    // note the difference from the previous test.  in this case wildcard
+    // applies to the owner name of the answer, not the owner name of the
+    // additional.
+    ZoneFinderContextPtr ctx = finder_->find(Name("mx.wildmx.example.org"),
+                                             RRType::MX());
+    EXPECT_EQ(ZoneFinder::SUCCESS, ctx->code);
+    ctx->getAdditional(REQUESTED_BOTH, result_sets_);
+    rrsetsCheck("mx1.example.org. 3600 IN A 192.0.2.10\n",
+                result_sets_.begin(), result_sets_.end());
+}
+
 TEST_P(ZoneFinderContextTest, getAdditionalMX) {
     // Similar to the previous cases, but for MX addresses.  The test zone
     // contains MX name under a zone cut.  Its address shouldn't be returned.
@@ -239,6 +314,21 @@ TEST_P(ZoneFinderContextTest, getAdditionalMX) {
                 result_sets_.begin(), result_sets_.end());
 }
 
+TEST_P(ZoneFinderContextTest, getAdditionalMXAtZoneCut) {
+    // XXX: the current database-based data source incorrectly rejects this
+    // setup (see #1771)
+    if (GetParam() == createSQLite3Client) {
+        return;
+    }
+
+    ZoneFinderContextPtr ctx = finder_->find(Name("mxatcut.example.org."),
+                                             RRType::MX());
+    EXPECT_EQ(ZoneFinder::SUCCESS, ctx->code);
+
+    ctx->getAdditional(REQUESTED_BOTH, result_sets_);
+    EXPECT_TRUE(result_sets_.empty());
+}
+
 TEST_P(ZoneFinderContextTest, getAdditionalWithSIG) {
     // Similar to the AuthNS test, but the original find() requested DNSSEC
     // RRSIGs.  Then additional records will also have RRSIGs.

src/lib/datasrc/zone.h

@@ -252,6 +252,13 @@ public:
         /// call resulted in SUCCESS or DELEGATION.  Otherwise this method
         /// does nothing.
         ///
+        /// \note The additional RRsets returned via method are limited to
+        /// ones contained in the zone which the corresponding find/findAll
+        /// call searched (possibly including glues under a zone cut where
+        /// they are applicable).  If the caller needs to get out-of-zone
+        /// additional RRsets, it needs to explicitly finds them by
+        /// identifying the corresponding zone and calls \c find() for it.
+        ///
         /// \param requested_types A vector of RR types for desired additional
         ///  RRsets.
         /// \param result A vector to which any found additional RRsets are

src/lib/dns/Makefile.am

@@ -61,6 +61,8 @@ EXTRA_DIST += rdata/generic/soa_6.cc
 EXTRA_DIST += rdata/generic/soa_6.h
 EXTRA_DIST += rdata/generic/spf_99.cc
 EXTRA_DIST += rdata/generic/spf_99.h
+EXTRA_DIST += rdata/generic/sshfp_44.cc
+EXTRA_DIST += rdata/generic/sshfp_44.h
 EXTRA_DIST += rdata/generic/txt_16.cc
 EXTRA_DIST += rdata/generic/txt_16.h
 EXTRA_DIST += rdata/generic/minfo_14.cc
@@ -96,6 +98,7 @@ libdns___la_SOURCES += masterload.h masterload.cc
 libdns___la_SOURCES += message.h message.cc
 libdns___la_SOURCES += messagerenderer.h messagerenderer.cc
 libdns___la_SOURCES += name.h name.cc
+libdns___la_SOURCES += name_internal.h
 libdns___la_SOURCES += nsec3hash.h nsec3hash.cc
 libdns___la_SOURCES += opcode.h opcode.cc
 libdns___la_SOURCES += rcode.h rcode.cc
@@ -154,7 +157,7 @@ libdns___include_HEADERS = \
 	rrttl.h \
 	tsigkey.h
 # Purposely not installing these headers:
-# util/*.h: used only internally, and not actually DNS specific
+# name_internal.h: used only internally, and not actually DNS specific
 # rdata/*/detail/*.h: these are internal use only
 # rrclass-placeholder.h
 # rrtype-placeholder.h

src/lib/dns/benchmarks/.gitignore

@@ -1 +1,2 @@
+/message_renderer_bench
 /rdatarender_bench

src/lib/dns/benchmarks/Makefile.am

@@ -9,10 +9,16 @@ endif
 
 CLEANFILES = *.gcno *.gcda
 
-noinst_PROGRAMS = rdatarender_bench
+noinst_PROGRAMS = rdatarender_bench message_renderer_bench
+
 rdatarender_bench_SOURCES = rdatarender_bench.cc
 
 rdatarender_bench_LDADD = $(top_builddir)/src/lib/dns/libdns++.la
 rdatarender_bench_LDADD += $(top_builddir)/src/lib/util/libutil.la
 rdatarender_bench_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
-rdatarender_bench_LDADD += $(SQLITE_LIBS)
+
+message_renderer_bench_SOURCES = message_renderer_bench.cc
+message_renderer_bench_SOURCES += oldmessagerenderer.h oldmessagerenderer.cc
+message_renderer_bench_LDADD = $(top_builddir)/src/lib/dns/libdns++.la
+message_renderer_bench_LDADD += $(top_builddir)/src/lib/util/libutil.la
+message_renderer_bench_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la

src/lib/dns/benchmarks/message_renderer_bench.cc

@@ -0,0 +1,176 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <bench/benchmark.h>
+
+#include <dns/name.h>
+#include <dns/messagerenderer.h>
+#include <oldmessagerenderer.h>
+
+#include <cassert>
+#include <vector>
+
+using namespace std;
+using namespace isc::util;
+using namespace isc::bench;
+using namespace isc::dns;
+
+namespace {
+// This templated test performs rendering given set of names using
+// a given (templated) MessageRenderer implementation.  We can check the
+// performance when we modify the renderer implementation by comparing the
+// old and new implementation for the same data.
+template <typename T>
+class MessageRendererBenchMark {
+public:
+    MessageRendererBenchMark(const vector<Name>& names) :
+        names_(names)
+    {}
+    unsigned int run() {
+        renderer_.clear();
+        vector<Name>::const_iterator it = names_.begin();
+        const vector<Name>::const_iterator it_end = names_.end();
+        for (; it != it_end; ++it) {
+            renderer_.writeName(*it);
+        }
+        // Make sure truncation didn't accidentally happen.
+        assert(!renderer_.isTruncated());
+        return (names_.size());
+    }
+private:
+    T renderer_;
+    const vector<Name>& names_;
+};
+
+//
+// Builtin benchmark data.
+//
+// This consists of all names contained in a response from a root server for
+// the query for "www.example.com" (as of this implementing).
+const char* const root_to_com_names[] = {
+    // question section
+    "www.example.com",
+    // authority section
+    "com", "a.gtld-servers.net", "com", "b.gtld-servers.net",
+    "com", "c.gtld-servers.net", "com", "d.gtld-servers.net",
+    "com", "e.gtld-servers.net", "com", "f.gtld-servers.net",
+    "com", "g.gtld-servers.net", "com", "h.gtld-servers.net",
+    "com", "i.gtld-servers.net", "com", "j.gtld-servers.net",
+    "com", "k.gtld-servers.net", "com", "l.gtld-servers.net",
+    "com",                      // owner name of DS
+    "com",                      // owner name of RRSIG(DS)
+    // additional section.  a and b has both AAAA and A; others have A only.
+    "a.gtld-servers.net", "a.gtld-servers.net",
+    "b.gtld-servers.net", "b.gtld-servers.net",
+    "c.gtld-servers.net", "d.gtld-servers.net", "e.gtld-servers.net",
+    "f.gtld-servers.net", "g.gtld-servers.net", "h.gtld-servers.net",
+    "i.gtld-servers.net", "j.gtld-servers.net", "k.gtld-servers.net",
+    "l.gtld-servers.net", "m.gtld-servers.net",
+    NULL
+};
+
+// Names contained a typical "NXDOMAIN" response: the question, the owner
+// name of SOA, and its MNAME and RNAME.
+const char* const example_nxdomain_names[] = {
+    "www.example.com", "example.com", "ns.example.com", "root.example.com",
+    NULL
+};
+
+// Names contained a typical "SERVFAIL" response: only the question.
+const char* const example_servfail_names[] = {
+    "www.example.com", NULL
+};
+
+// An experimental "dumb" renderer for comparison.  It doesn't do any name
+// compression.  It simply ignores all setter method, returns a dummy value
+// for getter methods, and write names to the internal buffer as plain binary
+// data.
+class DumbMessageRenderer : public AbstractMessageRenderer {
+public:
+    virtual void clear() {}
+    virtual size_t getLengthLimit() const { return (512); }
+    virtual void setLengthLimit(const size_t) {}
+    virtual bool isTruncated() const { return (false); }
+    virtual void setTruncated() {}
+    virtual CompressMode getCompressMode() const { return (CASE_INSENSITIVE); }
+    virtual void setCompressMode(const CompressMode) {}
+    virtual void writeName(const Name& name, const bool = false) {
+        name.toWire(getBuffer());
+    }
+};
+
+void
+usage() {
+    cerr << "Usage: message_renderer_bench [-n iterations]" << endl;
+    exit (1);
+}
+}
+
+int
+main(int argc, char* argv[]) {
+    int ch;
+    int iteration = 100000;
+    while ((ch = getopt(argc, argv, "n:")) != -1) {
+        switch (ch) {
+        case 'n':
+            iteration = atoi(optarg);
+            break;
+        case '?':
+        default:
+            usage();
+        }
+    }
+    argc -= optind;
+    argv += optind;
+    if (argc != 0) {
+        usage();
+    }
+
+    cout << "Parameters:" << endl;
+    cout << "  Iterations: " << iteration << endl;
+
+    typedef pair<const char* const*, string> DataSpec;
+    vector<DataSpec> spec_list;
+    spec_list.push_back(DataSpec(root_to_com_names, "(positive response)"));
+    spec_list.push_back(DataSpec(example_nxdomain_names,
+                                 "(NXDOMAIN response)"));
+    spec_list.push_back(DataSpec(example_servfail_names,
+                                 "(SERVFAIL response)"));
+    for (vector<DataSpec>::const_iterator it = spec_list.begin();
+         it != spec_list.end();
+         ++it) {
+        vector<Name> names;
+        for (size_t i = 0; it->first[i] != NULL; ++i) {
+            names.push_back(Name(it->first[i]));
+        }
+
+        typedef MessageRendererBenchMark<OldMessageRenderer>
+            OldRendererBenchMark;
+        cout << "Benchmark for old MessageRenderer " << it->second << endl;
+        BenchMark<OldRendererBenchMark>(iteration,
+                                        OldRendererBenchMark(names));
+
+        typedef MessageRendererBenchMark<DumbMessageRenderer>
+            DumbRendererBenchMark;
+        cout << "Benchmark for dumb MessageRenderer " << it->second << endl;
+        BenchMark<DumbRendererBenchMark>(iteration,
+                                         DumbRendererBenchMark(names));
+
+        typedef MessageRendererBenchMark<MessageRenderer> RendererBenchMark;
+        cout << "Benchmark for new MessageRenderer " << it->second << endl;
+        BenchMark<RendererBenchMark>(iteration, RendererBenchMark(names));
+    }
+
+    return (0);
+}

src/lib/dns/benchmarks/oldmessagerenderer.cc

@@ -0,0 +1,278 @@
+// 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.
+
+#include <exceptions/exceptions.h>
+#include <util/buffer.h>
+#include <dns/name.h>
+#include <oldmessagerenderer.h>
+
+#include <cctype>
+#include <cassert>
+#include <set>
+
+using namespace isc::util;
+
+namespace isc {
+namespace dns {
+
+namespace {     // hide internal-only names from the public namespaces
+///
+/// \brief The \c NameCompressNode class represents a pointer to a name
+/// rendered in the internal buffer for the \c MessageRendererImpl object.
+///
+/// A \c MessageRendererImpl object maintains a set of the \c NameCompressNode
+/// objects, and searches the set for the position of the longest match
+/// (ancestor) name against each new name to be rendered into the buffer.
+struct NameCompressNode {
+    NameCompressNode(const OldMessageRenderer& renderer,
+                     const OutputBuffer& buffer, const size_t pos,
+                     const size_t len) :
+        renderer_(renderer), buffer_(buffer), pos_(pos), len_(len) {}
+    /// The renderer that performs name compression using the node.
+    /// This is kept in each node to detect the compression mode
+    /// (case-sensitive or not) in the comparison functor (\c NameCompare).
+    const OldMessageRenderer& renderer_;
+    /// The buffer in which the corresponding name is rendered.
+    const OutputBuffer& buffer_;
+    /// The position (offset from the beginning) in the buffer where the
+    /// name starts.
+    uint16_t pos_;
+    /// The length of the corresponding name.
+    uint16_t len_;
+};
+
+///
+/// \brief The \c NameCompare class is a functor that gives ordering among
+/// \c NameCompressNode objects stored in \c MessageRendererImpl::nodeset_.
+///
+/// Its only public method as a functor, \c operator(), gives the ordering
+/// between two \c NameCompressNode objects in terms of equivalence, that is,
+/// returns whether one is "less than" the other.
+/// For our purpose we only need to distinguish two different names, so the
+/// ordering is different from the canonical DNS name order used in DNSSEC;
+/// basically, it gives the case-insensitive ordering of the two names as their
+/// textual representation.
+struct NameCompare : public std::binary_function<NameCompressNode,
+                                                 NameCompressNode,
+                                                 bool> {
+    ///
+    /// Returns true if n1 < n2 as a result of case-insensitive comparison;
+    /// otherwise return false.
+    ///
+    /// The name corresponding to \c n1 or \c n2 may be compressed, in which
+    /// case we must follow the compression pointer in the associated buffer.
+    /// The helper private method \c nextPosition() gives the position in the
+    /// buffer for the next character, taking into account compression.
+    ///
+    bool operator()(const NameCompressNode& n1,
+                    const NameCompressNode& n2) const
+    {
+        if (n1.len_ < n2.len_) {
+            return (true);
+        } else if (n1.len_ > n2.len_) {
+            return (false);
+        }
+
+        const bool case_sensitive =
+            (n1.renderer_.getCompressMode() == OldMessageRenderer::CASE_SENSITIVE);
+
+        uint16_t pos1 = n1.pos_;
+        uint16_t pos2 = n2.pos_;
+        uint16_t l1 = 0;
+        uint16_t l2 = 0;
+        for (uint16_t i = 0; i < n1.len_; i++, pos1++, pos2++) {
+            pos1 = nextPosition(n1.buffer_, pos1, l1);
+            pos2 = nextPosition(n2.buffer_, pos2, l2);
+            if (case_sensitive) {
+                if (n1.buffer_[pos1] < n2.buffer_[pos2]) {
+                    return (true);
+                } else if (n1.buffer_[pos1] > n2.buffer_[pos2]) {
+                    return (false);
+                }
+            } else {
+                if (tolower(n1.buffer_[pos1]) < tolower(n2.buffer_[pos2])) {
+                    return (true);
+                } else if (tolower(n1.buffer_[pos1]) >
+                           tolower(n2.buffer_[pos2])) {
+                    return (false);
+                }
+            }
+        }
+
+        return (false);
+    }
+
+private:
+    uint16_t nextPosition(const OutputBuffer& buffer,
+                          uint16_t pos, uint16_t& llen) const
+    {
+        if (llen == 0) {
+            size_t i = 0;
+
+            while ((buffer[pos] & Name::COMPRESS_POINTER_MARK8) ==
+                   Name::COMPRESS_POINTER_MARK8) {
+                pos = (buffer[pos] & ~Name::COMPRESS_POINTER_MARK8) *
+                    256 + buffer[pos + 1];
+
+                // This loop should stop as long as the buffer has been
+                // constructed validly and the search/insert argument is based
+                // on a valid name, which is an assumption for this class.
+                // But we'll abort if a bug could cause an infinite loop.
+                i += 2;
+                assert(i < Name::MAX_WIRE);
+            }
+            llen = buffer[pos];
+        } else {
+            --llen;
+        }
+        return (pos);
+    }
+};
+}
+
+///
+/// \brief The \c MessageRendererImpl class is the actual implementation of
+/// \c MessageRenderer.
+///
+/// The implementation is hidden from applications.  We can refer to specific
+/// members of this class only within the implementation source file.
+///
+struct OldMessageRenderer::MessageRendererImpl {
+    /// \brief Constructor from an output buffer.
+    ///
+    MessageRendererImpl() :
+        nbuffer_(Name::MAX_WIRE), msglength_limit_(512),
+        truncated_(false), compress_mode_(OldMessageRenderer::CASE_INSENSITIVE)
+    {}
+    /// A local working buffer to convert each given name into wire format.
+    /// This could be a local variable of the \c writeName() method, but
+    /// we keep it in the class so that we can reuse it and avoid construction
+    /// overhead.
+    OutputBuffer nbuffer_;
+    /// A set of compression pointers.
+    std::set<NameCompressNode, NameCompare> nodeset_;
+    /// The maximum length of rendered data that can fit without
+    /// truncation.
+    uint16_t msglength_limit_;
+    /// A boolean flag that indicates truncation has occurred while rendering
+    /// the data.
+    bool truncated_;
+    /// The name compression mode.
+    CompressMode compress_mode_;
+};
+
+OldMessageRenderer::OldMessageRenderer() :
+    AbstractMessageRenderer(),
+    impl_(new MessageRendererImpl)
+{}
+
+OldMessageRenderer::~OldMessageRenderer() {
+    delete impl_;
+}
+
+void
+OldMessageRenderer::clear() {
+    AbstractMessageRenderer::clear();
+    impl_->nbuffer_.clear();
+    impl_->nodeset_.clear();
+    impl_->msglength_limit_ = 512;
+    impl_->truncated_ = false;
+    impl_->compress_mode_ = CASE_INSENSITIVE;
+}
+
+size_t
+OldMessageRenderer::getLengthLimit() const {
+    return (impl_->msglength_limit_);
+}
+
+void
+OldMessageRenderer::setLengthLimit(const size_t len) {
+    impl_->msglength_limit_ = len;
+}
+
+bool
+OldMessageRenderer::isTruncated() const {
+    return (impl_->truncated_);
+}
+
+void
+OldMessageRenderer::setTruncated() {
+    impl_->truncated_ = true;
+}
+
+OldMessageRenderer::CompressMode
+OldMessageRenderer::getCompressMode() const {
+    return (impl_->compress_mode_);
+}
+
+void
+OldMessageRenderer::setCompressMode(const CompressMode mode) {
+    impl_->compress_mode_ = mode;
+}
+
+void
+OldMessageRenderer::writeName(const Name& name, const bool compress) {
+    impl_->nbuffer_.clear();
+    name.toWire(impl_->nbuffer_);
+
+    unsigned int i;
+    std::set<NameCompressNode, NameCompare>::const_iterator notfound =
+        impl_->nodeset_.end();
+    std::set<NameCompressNode, NameCompare>::const_iterator n = notfound;
+
+    // Find the longest ancestor name in the rendered set that matches the
+    // given name.
+    for (i = 0; i < impl_->nbuffer_.getLength(); i += impl_->nbuffer_[i] + 1) {
+        // skip the trailing null label
+        if (impl_->nbuffer_[i] == 0) {
+            continue;
+        }
+        n = impl_->nodeset_.find(NameCompressNode(*this, impl_->nbuffer_, i,
+                                                  impl_->nbuffer_.getLength() -
+                                                  i));
+        if (n != notfound) {
+            break;
+        }
+    }
+
+    // Record the current offset before extending the buffer.
+    const size_t offset = getLength();
+    // Write uncompress part...
+    writeData(impl_->nbuffer_.getData(),
+              compress ? i : impl_->nbuffer_.getLength());
+    if (compress && n != notfound) {
+        // ...and compression pointer if available.
+        uint16_t pointer = (*n).pos_;
+        pointer |= Name::COMPRESS_POINTER_MARK16;
+        writeUint16(pointer);
+    }
+
+    // Finally, add to the set the newly rendered name and its ancestors that
+    // have not been in the set.
+    for (unsigned int j = 0; j < i; j += impl_->nbuffer_[j] + 1) {
+        if (impl_->nbuffer_[j] == 0) {
+            continue;
+        }
+        if (offset + j > Name::MAX_COMPRESS_POINTER) {
+            break;
+        }
+        impl_->nodeset_.insert(NameCompressNode(*this, getBuffer(),
+                                                offset + j,
+                                                impl_->nbuffer_.getLength() -
+                                                j));
+    }
+}
+
+}
+}

src/lib/dns/benchmarks/oldmessagerenderer.h

@@ -0,0 +1,55 @@
+// 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.
+
+#ifndef __OLDMESSAGERENDERER_H
+#define __OLDMESSAGERENDERER_H 1
+
+//
+// This is a copy of an older version of MessageRenderer class.  It is kept
+// here to provide a benchmark target.
+//
+
+#include <dns/messagerenderer.h>
+
+namespace isc {
+namespace dns {
+
+class OldMessageRenderer : public AbstractMessageRenderer {
+public:
+    using AbstractMessageRenderer::CASE_INSENSITIVE;
+    using AbstractMessageRenderer::CASE_SENSITIVE;
+
+    /// \brief Constructor from an output buffer.
+    OldMessageRenderer();
+
+    virtual ~OldMessageRenderer();
+    virtual bool isTruncated() const;
+    virtual size_t getLengthLimit() const;
+    virtual CompressMode getCompressMode() const;
+    virtual void setTruncated();
+    virtual void setLengthLimit(size_t len);
+    virtual void setCompressMode(CompressMode mode);
+    virtual void clear();
+    virtual void writeName(const Name& name, bool compress = true);
+private:
+    struct MessageRendererImpl;
+    MessageRendererImpl* impl_;
+};
+}
+}
+#endif // __OLDMESSAGERENDERER_H
+
+// Local Variables:
+// mode: c++
+// End:

src/lib/dns/labelsequence.cc

@@ -13,25 +13,33 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <dns/labelsequence.h>
+#include <dns/name_internal.h>
 #include <exceptions/exceptions.h>
 
-#include <iostream>
+#include <boost/functional/hash.hpp>
+
 namespace isc {
 namespace dns {
 
 const char*
 LabelSequence::getData(size_t *len) const {
+    *len = getDataLength();
+    return (&name_.ndata_[name_.offsets_[first_label_]]);
+}
+
+size_t
+LabelSequence::getDataLength() const {
     // If the labelsequence is absolute, the current last_label_ falls
     // out of the vector (since it points to the 'label' after the
     // root label, which doesn't exist; in that case, return
     // the length for the 'previous' label (the root label) plus
     // one (for the root label zero octet)
     if (isAbsolute()) {
-        *len = name_.offsets_[last_label_ - 1] - name_.offsets_[first_label_] + 1;
+        return (name_.offsets_[last_label_ - 1] -
+                name_.offsets_[first_label_] + 1);
     } else {
-        *len = name_.offsets_[last_label_] - name_.offsets_[first_label_];
+        return (name_.offsets_[last_label_] - name_.offsets_[first_label_]);
     }
-    return (&name_.ndata_[name_.offsets_[first_label_]]);
 }
 
 bool
@@ -44,10 +52,21 @@ LabelSequence::equals(const LabelSequence& other, bool case_sensitive) const {
         return (false);
     }
     if (case_sensitive) {
-        return (strncasecmp(data, other_data, len) == 0);
-    } else {
         return (strncmp(data, other_data, len) == 0);
     }
+
+    // As long as the data was originally validated as (part of) a name,
+    // label length must never be a capital ascii character, so we can
+    // simply compare them after converting to lower characters.
+    for (size_t i = 0; i < len; ++i) {
+        const unsigned char ch = data[i];
+        const unsigned char other_ch = other_data[i];
+        if (isc::dns::name::internal::maptolower[ch] !=
+            isc::dns::name::internal::maptolower[other_ch]) {
+            return (false);
+        }
+    }
+    return (true);
 }
 
 void
@@ -73,5 +92,23 @@ LabelSequence::isAbsolute() const {
     return (last_label_ == name_.offsets_.size());
 }
 
+size_t
+LabelSequence::getHash(bool case_sensitive) const {
+    size_t length;
+    const char* s = getData(&length);
+    if (length > 16) {
+        length = 16;
+    }
+
+    size_t hash_val = 0;
+    while (length > 0) {
+        const unsigned char c = *s++;
+        boost::hash_combine(hash_val, case_sensitive ? c :
+                            isc::dns::name::internal::maptolower[c]);
+        --length;
+    }
+    return (hash_val);
+}
+
 } // end namespace dns
 } // end namespace isc

src/lib/dns/labelsequence.h

@@ -69,6 +69,22 @@ public:
     /// \return Pointer to the wire-format data of this label sequence
     const char* getData(size_t* len) const;
 
+    /// \brief Return the length of the wire-format data of this LabelSequence
+    ///
+    /// This method returns the number of octets for the data that would
+    /// be returned by the \c getData() method.
+    ///
+    /// Note that the return value of this method is always positive.
+    /// Note also that if the return value of this method is 1, it means the
+    /// sequence consists of the null label, i.e., a single "dot", and vice
+    /// versa.
+    ///
+    /// \note The data pointed to is only valid if the original Name
+    /// object is still in scope
+    ///
+    /// \return The length of the data of the label sequence in octets.
+    size_t getDataLength() const;
+
     /// \brief Compares two label sequences.
     ///
     /// Performs a (optionally case-insensitive) comparison between this
@@ -105,7 +121,7 @@ public:
     /// \brief Returns the current number of labels for this LabelSequence
     ///
     /// \return The number of labels
-    size_t getLabelCount() const { return last_label_ - first_label_; }
+    size_t getLabelCount() const { return (last_label_ - first_label_); }
 
     /// \brief Returns the original Name object associated with this
     ///        LabelSequence
@@ -116,7 +132,32 @@ public:
     /// LabelSequence itself.
     ///
     /// \return Reference to the original Name object
-    const Name& getName() const { return name_; }
+    const Name& getName() const { return (name_); }
+
+    /// \brief Calculate a simple hash for the label sequence.
+    ///
+    /// This method calculates a hash value for the label sequence as binary
+    /// data.  If \c case_sensitive is false, it ignores the case stored in
+    /// the labels; specifically, it normalizes the labels by converting all
+    /// upper case characters to lower case ones and calculates the hash value
+    /// for the result.
+    ///
+    /// This method is intended to provide a lightweight way to store a
+    /// relatively small number of label sequences in a hash table.
+    /// For this reason it only takes into account data up to 16 octets
+    /// (16 was derived from BIND 9's implementation).  Also, the function does
+    /// not provide any unpredictability; a specific sequence will always have
+    /// the same hash value.  It should therefore not be used in the context
+    /// where an untrusted third party can mount a denial of service attack by
+    /// forcing the application to create a very large number of label
+    /// sequences that have the same hash value and expected to be stored in
+    /// a hash table.
+    ///
+    /// \exception None
+    ///
+    /// \param case_sensitive
+    /// \return A hash value for this label sequence.
+    size_t getHash(bool case_sensitive) const;
 
     /// \brief Checks whether the label sequence is absolute
     ///

src/lib/dns/messagerenderer.cc

@@ -15,102 +15,105 @@
 #include <exceptions/exceptions.h>
 #include <util/buffer.h>
 #include <dns/name.h>
+#include <dns/name_internal.h>
+#include <dns/labelsequence.h>
 #include <dns/messagerenderer.h>
 
-#include <cctype>
+#include <boost/array.hpp>
+#include <boost/static_assert.hpp>
+
+#include <limits>
 #include <cassert>
-#include <set>
+#include <vector>
 
+using namespace std;
 using namespace isc::util;
+using isc::dns::name::internal::maptolower;
 
 namespace isc {
 namespace dns {
 
 namespace {     // hide internal-only names from the public namespaces
 ///
-/// \brief The \c NameCompressNode class represents a pointer to a name
+/// \brief The \c OffsetItem class represents a pointer to a name
 /// rendered in the internal buffer for the \c MessageRendererImpl object.
 ///
-/// A \c MessageRendererImpl object maintains a set of the \c NameCompressNode
-/// objects, and searches the set for the position of the longest match
-/// (ancestor) name against each new name to be rendered into the buffer.
-struct NameCompressNode {
-    NameCompressNode(const MessageRenderer& renderer,
-                     const OutputBuffer& buffer, const size_t pos,
-                     const size_t len) :
-        renderer_(renderer), buffer_(buffer), pos_(pos), len_(len) {}
-    /// The renderer that performs name compression using the node.
-    /// This is kept in each node to detect the compression mode
-    /// (case-sensitive or not) in the comparison functor (\c NameCompare).
-    const MessageRenderer& renderer_;
-    /// The buffer in which the corresponding name is rendered.
-    const OutputBuffer& buffer_;
+/// A \c MessageRendererImpl object maintains a set of \c OffsetItem
+/// objects in a hash table, and searches the table for the position of the
+/// longest match (ancestor) name against each new name to be rendered into
+/// the buffer.
+struct OffsetItem {
+    OffsetItem(size_t hash, size_t pos, size_t len) :
+        hash_(hash), pos_(pos), len_(len)
+    {}
+
+    /// The hash value for the stored name calculated by LabelSequence.getHash.
+    /// This will help make name comparison in \c NameCompare more efficient.
+    size_t hash_;
+
     /// The position (offset from the beginning) in the buffer where the
     /// name starts.
     uint16_t pos_;
-    /// The length of the corresponding name.
+
+    /// The length of the corresponding sequence (which is a domain name).
     uint16_t len_;
 };
 
+/// \brief The \c NameCompare class is a functor that checks equality
+/// between the name corresponding to an \c OffsetItem object and the name
+/// consists of labels represented by a \c LabelSequence object.
 ///
-/// \brief The \c NameCompare class is a functor that gives ordering among
-/// \c NameCompressNode objects stored in \c MessageRendererImpl::nodeset_.
-///
-/// Its only public method as a functor, \c operator(), gives the ordering
-/// between two \c NameCompressNode objects in terms of equivalence, that is,
-/// returns whether one is "less than" the other.
-/// For our purpose we only need to distinguish two different names, so the
-/// ordering is different from the canonical DNS name order used in DNSSEC;
-/// basically, it gives the case-insensitive ordering of the two names as their
-/// textual representation.
-struct NameCompare : public std::binary_function<NameCompressNode,
-                                                 NameCompressNode,
-                                                 bool> {
-    ///
-    /// Returns true if n1 < n2 as a result of case-insensitive comparison;
-    /// otherwise return false.
+/// Template parameter CASE_SENSITIVE determines whether to ignore the case
+/// of the names.  This policy doesn't change throughout the lifetime of
+/// this object, so we separate these using template to avoid unnecessary
+/// condition check.
+template <bool CASE_SENSITIVE>
+struct NameCompare {
+    /// \brief Constructor
     ///
-    /// The name corresponding to \c n1 or \c n2 may be compressed, in which
-    /// case we must follow the compression pointer in the associated buffer.
-    /// The helper private method \c nextPosition() gives the position in the
-    /// buffer for the next character, taking into account compression.
-    ///
-    bool operator()(const NameCompressNode& n1,
-                    const NameCompressNode& n2) const
-    {
-        if (n1.len_ < n2.len_) {
-            return (true);
-        } else if (n1.len_ > n2.len_) {
+    /// \param buffer The buffer for rendering used in the caller renderer
+    /// \param name_buf An input buffer storing the wire-format data of the
+    /// name to be newly rendered (and only that data).
+    /// \param hash The hash value for the name.
+    NameCompare(const OutputBuffer& buffer, InputBuffer& name_buf,
+                size_t hash) :
+        buffer_(&buffer), name_buf_(&name_buf), hash_(hash)
+    {}
+
+    bool operator()(const OffsetItem& item) const {
+        // Trivial inequality check.  If either the hash or the total length
+        // doesn't match, the names are obviously different.
+        if (item.hash_  != hash_ || item.len_ != name_buf_->getLength()) {
             return (false);
         }
 
-        const bool case_sensitive =
-            (n1.renderer_.getCompressMode() == MessageRenderer::CASE_SENSITIVE);
-
-        uint16_t pos1 = n1.pos_;
-        uint16_t pos2 = n2.pos_;
-        uint16_t l1 = 0;
-        uint16_t l2 = 0;
-        for (uint16_t i = 0; i < n1.len_; i++, pos1++, pos2++) {
-            pos1 = nextPosition(n1.buffer_, pos1, l1);
-            pos2 = nextPosition(n2.buffer_, pos2, l2);
-            if (case_sensitive) {
-                if (n1.buffer_[pos1] < n2.buffer_[pos2]) {
-                    return (true);
-                } else if (n1.buffer_[pos1] > n2.buffer_[pos2]) {
+        // Compare the name data, character-by-character.
+        // item_pos keeps track of the position in the buffer corresponding to
+        // the character to compare.  item_label_len is the number of
+        // characters in the labels where the character pointed by item_pos
+        // belongs.  When it reaches zero, nextPosition() identifies the
+        // position for the subsequent label, taking into account name
+        // compression, and resets item_label_len to the length of the new
+        // label.
+        name_buf_->setPosition(0); // buffer can be reused, so reset position
+        uint16_t item_pos = item.pos_;
+        uint16_t item_label_len = 0;
+        for (size_t i = 0; i < item.len_; ++i, ++item_pos) {
+            item_pos = nextPosition(*buffer_, item_pos, item_label_len);
+            const unsigned char ch1 = (*buffer_)[item_pos];
+            const unsigned char ch2 = name_buf_->readUint8();
+            if (CASE_SENSITIVE) {
+                if (ch1 != ch2) {
                     return (false);
                 }
             } else {
-                if (tolower(n1.buffer_[pos1]) < tolower(n2.buffer_[pos2])) {
-                    return (true);
-                } else if (tolower(n1.buffer_[pos1]) >
-                           tolower(n2.buffer_[pos2])) {
+                if (maptolower[ch1] != maptolower[ch2]) {
                     return (false);
                 }
             }
         }
 
-        return (false);
+        return (true);
     }
 
 private:
@@ -138,6 +141,10 @@ private:
         }
         return (pos);
     }
+
+    const OutputBuffer* buffer_;
+    InputBuffer* name_buf_;
+    const size_t hash_;
 };
 }
 
@@ -148,20 +155,60 @@ private:
 /// The implementation is hidden from applications.  We can refer to specific
 /// members of this class only within the implementation source file.
 ///
+/// It internally holds a hash table for OffsetItem objects corresponding
+/// to portions of names rendered in this renderer.  The offset information
+/// is used to compress subsequent names to be rendered.
 struct MessageRenderer::MessageRendererImpl {
-    /// \brief Constructor from an output buffer.
-    ///
+    // The size of hash buckets and number of hash entries per bucket for
+    // which space is preallocated and kept reserved for subsequent rendering
+    // to provide better performance.  These values are derived from the
+    // BIND 9 implementation that uses a similar hash table.
+    static const size_t BUCKETS = 64;
+    static const size_t RESERVED_ITEMS = 16;
+    static const uint16_t NO_OFFSET = 65535; // used as a marker of 'not found'
+
+    /// \brief Constructor
     MessageRendererImpl() :
-        nbuffer_(Name::MAX_WIRE), msglength_limit_(512),
-        truncated_(false), compress_mode_(MessageRenderer::CASE_INSENSITIVE)
-    {}
-    /// A local working buffer to convert each given name into wire format.
-    /// This could be a local variable of the \c writeName() method, but
-    /// we keep it in the class so that we can reuse it and avoid construction
-    /// overhead.
-    OutputBuffer nbuffer_;
-    /// A set of compression pointers.
-    std::set<NameCompressNode, NameCompare> nodeset_;
+        msglength_limit_(512), truncated_(false),
+        compress_mode_(MessageRenderer::CASE_INSENSITIVE)
+    {
+        // Reserve some spaces for hash table items.
+        for (size_t i = 0; i < BUCKETS; ++i) {
+            table_[i].reserve(RESERVED_ITEMS);
+        }
+    }
+
+    uint16_t findOffset(const OutputBuffer& buffer, InputBuffer& name_buf,
+                        size_t hash, bool case_sensitive) const
+    {
+        // Find a matching entry, if any.  We use some heuristics here: often
+        // the same name appers consecutively (like repeating the same owner
+        // name for a single RRset), so in case there's a collision in the
+        // bucket it will be more likely to find it in the tail side of the
+        // bucket.
+        const size_t bucket_id = hash % BUCKETS;
+        vector<OffsetItem>::const_reverse_iterator found;
+        if (case_sensitive) {
+            found = find_if(table_[bucket_id].rbegin(),
+                            table_[bucket_id].rend(),
+                            NameCompare<true>(buffer, name_buf, hash));
+        } else {
+            found = find_if(table_[bucket_id].rbegin(),
+                            table_[bucket_id].rend(),
+                            NameCompare<false>(buffer, name_buf, hash));
+        }
+        if (found != table_[bucket_id].rend()) {
+            return (found->pos_);
+        }
+        return (NO_OFFSET);
+    }
+
+    void addOffset(size_t hash, size_t offset, size_t len) {
+        table_[hash % BUCKETS].push_back(OffsetItem(hash, offset, len));
+    }
+
+    // The hash table for the (offset + position in the buffer) entries
+    vector<OffsetItem> table_[BUCKETS];
     /// The maximum length of rendered data that can fit without
     /// truncation.
     uint16_t msglength_limit_;
@@ -170,6 +217,11 @@ struct MessageRenderer::MessageRendererImpl {
     bool truncated_;
     /// The name compression mode.
     CompressMode compress_mode_;
+
+    // Placeholder for hash values as they are calculated in writeName().
+    // Note: we may want to make it a local variable of writeName() if it
+    // works more efficiently.
+    boost::array<size_t, Name::MAX_LABELS> seq_hashes_;
 };
 
 MessageRenderer::MessageRenderer() :
@@ -184,11 +236,22 @@ MessageRenderer::~MessageRenderer() {
 void
 MessageRenderer::clear() {
     AbstractMessageRenderer::clear();
-    impl_->nbuffer_.clear();
-    impl_->nodeset_.clear();
     impl_->msglength_limit_ = 512;
     impl_->truncated_ = false;
     impl_->compress_mode_ = CASE_INSENSITIVE;
+
+    // Clear the hash table.  We reserve the minimum space for possible
+    // subsequent use of the renderer.
+    for (size_t i = 0; i < MessageRendererImpl::BUCKETS; ++i) {
+        if (impl_->table_[i].size() > MessageRendererImpl::RESERVED_ITEMS) {
+            // Trim excessive capacity: swap ensures the new capacity is only
+            // reasonably large for the reserved space.
+            vector<OffsetItem> new_table;
+            new_table.reserve(MessageRendererImpl::RESERVED_ITEMS);
+            new_table.swap(impl_->table_[i]);
+        }
+        impl_->table_[i].clear();
+    }
 }
 
 size_t
@@ -218,59 +281,81 @@ MessageRenderer::getCompressMode() const {
 
 void
 MessageRenderer::setCompressMode(const CompressMode mode) {
+    if (getLength() != 0) {
+        isc_throw(isc::InvalidParameter,
+                  "compress mode cannot be changed during rendering");
+    }
     impl_->compress_mode_ = mode;
 }
 
 void
 MessageRenderer::writeName(const Name& name, const bool compress) {
-    impl_->nbuffer_.clear();
-    name.toWire(impl_->nbuffer_);
-
-    unsigned int i;
-    std::set<NameCompressNode, NameCompare>::const_iterator notfound =
-        impl_->nodeset_.end();
-    std::set<NameCompressNode, NameCompare>::const_iterator n = notfound;
-
-    // Find the longest ancestor name in the rendered set that matches the
-    // given name.
-    for (i = 0; i < impl_->nbuffer_.getLength(); i += impl_->nbuffer_[i] + 1) {
-        // skip the trailing null label
-        if (impl_->nbuffer_[i] == 0) {
-            continue;
+    LabelSequence sequence(name);
+    const size_t nlabels = sequence.getLabelCount();
+    size_t data_len;
+    const char* data;
+
+    // Find the offset in the offset table whose name gives the longest
+    // match against the name to be rendered.
+    size_t nlabels_uncomp;
+    uint16_t ptr_offset = MessageRendererImpl::NO_OFFSET;
+    const bool case_sensitive = (impl_->compress_mode_ ==
+                                 MessageRenderer::CASE_SENSITIVE);
+    for (nlabels_uncomp = 0; nlabels_uncomp < nlabels; ++nlabels_uncomp) {
+        data = sequence.getData(&data_len);
+        if (data_len == 1) { // trailing dot.
+            ++nlabels_uncomp;
+            break;
         }
-        n = impl_->nodeset_.find(NameCompressNode(*this, impl_->nbuffer_, i,
-                                                  impl_->nbuffer_.getLength() -
-                                                  i));
-        if (n != notfound) {
+        // write with range check for safety
+        impl_->seq_hashes_.at(nlabels_uncomp) =
+            sequence.getHash(impl_->compress_mode_);
+        InputBuffer name_buf(data, data_len);
+        ptr_offset = impl_->findOffset(getBuffer(), name_buf,
+                                       impl_->seq_hashes_[nlabels_uncomp],
+                                       case_sensitive);
+        if (ptr_offset != MessageRendererImpl::NO_OFFSET) {
             break;
         }
+        sequence.stripLeft(1);
     }
 
-    // Record the current offset before extending the buffer.
-    const size_t offset = getLength();
-    // Write uncompress part...
-    writeData(impl_->nbuffer_.getData(),
-              compress ? i : impl_->nbuffer_.getLength());
-    if (compress && n != notfound) {
-        // ...and compression pointer if available.
-        uint16_t pointer = (*n).pos_;
-        pointer |= Name::COMPRESS_POINTER_MARK16;
-        writeUint16(pointer);
+    // Record the current offset before updating the offset table
+    size_t offset = getLength();
+    // Write uncompress part:
+    if (nlabels_uncomp > 0 || !compress) {
+        LabelSequence uncomp_sequence(name);
+        if (compress && nlabels > nlabels_uncomp) {
+            // If there's compressed part, strip off that part.
+            uncomp_sequence.stripRight(nlabels - nlabels_uncomp);
+        }
+        data = uncomp_sequence.getData(&data_len);
+        writeData(data, data_len);
+    }
+    // And write compression pointer if available:
+    if (compress && ptr_offset != MessageRendererImpl::NO_OFFSET) {
+        ptr_offset |= Name::COMPRESS_POINTER_MARK16;
+        writeUint16(ptr_offset);
     }
 
-    // Finally, add to the set the newly rendered name and its ancestors that
-    // have not been in the set.
-    for (unsigned int j = 0; j < i; j += impl_->nbuffer_[j] + 1) {
-        if (impl_->nbuffer_[j] == 0) {
-            continue;
+    // Finally, record the offset and length for each uncompressed sequence
+    // in the hash table.  The renderer's buffer has just stored the
+    // corresponding data, so we use the rendered data to get the length
+    // of each label of the names.
+    size_t seqlen = name.getLength();
+    for (size_t i = 0; i < nlabels_uncomp; ++i) {
+        const uint8_t label_len = getBuffer()[offset];
+        if (label_len == 0) { // offset for root doesn't need to be stored.
+            break;
         }
-        if (offset + j > Name::MAX_COMPRESS_POINTER) {
+        if (offset > Name::MAX_COMPRESS_POINTER) {
             break;
         }
-        impl_->nodeset_.insert(NameCompressNode(*this, getBuffer(),
-                                                offset + j,
-                                                impl_->nbuffer_.getLength() -
-                                                j));
+        // Store the tuple of <hash, offset, len> to the table.  Note that we
+        // already know the hash value for each name.
+        impl_->addOffset(impl_->seq_hashes_[i], offset, seqlen);
+        offset += (label_len + 1);
+        seqlen -= (label_len + 1);
     }
 }
 

src/lib/dns/messagerenderer.h

@@ -359,7 +359,17 @@ public:
     virtual CompressMode getCompressMode() const;
     virtual void setTruncated();
     virtual void setLengthLimit(size_t len);
+
+    /// This implementation does not allow this call in the middle of
+    /// rendering (i.e. after at least one name is rendered) due to
+    /// restriction specific to the internal implementation.  Such attempts
+    /// will result in an \c isc::InvalidParameter exception.
+    ///
+    /// This shouldn't be too restrictive in practice; there's no known
+    /// practical case for such a mixed compression policy in a single
+    /// message.
     virtual void setCompressMode(CompressMode mode);
+
     virtual void clear();
     virtual void writeName(const Name& name, bool compress = true);
 private:

src/lib/dns/name.cc

@@ -23,11 +23,13 @@
 #include <util/buffer.h>
 #include <dns/exceptions.h>
 #include <dns/name.h>
+#include <dns/name_internal.h>
 #include <dns/messagerenderer.h>
 
 using namespace std;
 using namespace isc::util;
 using isc::dns::NameComparisonResult;
+using namespace isc::dns::name::internal;
 
 namespace isc {
 namespace dns {
@@ -46,12 +48,12 @@ namespace {
 /// we chose the naive but simple hardcoding approach.
 ///
 /// These definitions are derived from BIND 9's libdns module.
-/// Note: it was not clear why the maptolower array was needed rather than
-/// using the standard tolower() function.  It was perhaps due performance
-/// concern, but we were not sure.  Even if it was performance reasons, we
-/// should carefully assess the effect via benchmarking to avoid the pitfall of 
-/// premature optimization.  We should revisit this point later.
-static const char digitvalue[256] = {
+/// Note: we could use the standard tolower() function instead of the
+/// maptolower array, but a benchmark indicated that the private array could
+/// improve the performance of message rendering (which internally uses the
+/// array heavily) about 27%.  Since we want to achieve very good performance
+/// for message rendering in some cases, we'll keep using it.
+const char digitvalue[256] = {
     -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, // 16
     -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, // 32
     -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, // 48
@@ -69,8 +71,11 @@ static const char digitvalue[256] = {
     -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
     -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, // 256
 };
+}
 
-static const unsigned char maptolower[] = {
+namespace name {
+namespace internal {
+const unsigned char maptolower[] = {
     0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
     0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f,
     0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17,
@@ -104,7 +109,8 @@ static const unsigned char maptolower[] = {
     0xf0, 0xf1, 0xf2, 0xf3, 0xf4, 0xf5, 0xf6, 0xf7,
     0xf8, 0xf9, 0xfa, 0xfb, 0xfc, 0xfd, 0xfe, 0xff
 };
-}
+} // end of internal
+} // end of name
 
 namespace {
 ///

src/lib/dns/name_internal.h

@@ -0,0 +1,43 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef __NAME_INTERNAL_H
+#define __NAME_INTERNAL_H 1
+
+// This is effectively a "private" namespace for the Name class implementation,
+// but exposed publicly so the definitions in it can be shared with other
+// modules of the library (as of its introduction, used by LabelSequence and
+// MessageRenderer).  It's not expected to be used even by normal applications.
+// This header file is therefore not expected to be installed as part of the
+// library.
+//
+// Note: if it turns out that we need this shortcut for many other places
+// we may even want to make it expose to other BIND 10 modules, but for now
+// we'll keep it semi-private (note also that except for very performance
+// sensitive applications the standard std::tolower() function should be just
+// sufficient).
+namespace isc {
+namespace dns {
+namespace name {
+namespace internal {
+extern const unsigned char maptolower[];
+} // end of internal
+} // end of name
+} // end of dns
+} // end of isc
+#endif // __NAME_INTERNAL_H
+
+// Local Variables:
+// mode: c++
+// End:

src/lib/dns/python/tests/.gitignore

@@ -1 +0,0 @@
-/__pycache__

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

@@ -0,0 +1,164 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <config.h>
+
+#include <string>
+
+#include <boost/lexical_cast.hpp>
+
+#include <exceptions/exceptions.h>
+
+#include <util/buffer.h>
+#include <util/encode/hex.h>
+#include <dns/name.h>
+#include <dns/messagerenderer.h>
+#include <dns/rdata.h>
+#include <dns/rdataclass.h>
+
+using namespace std;
+using namespace boost;
+using namespace isc::util;
+using namespace isc::util::encode;
+
+// BEGIN_ISC_NAMESPACE
+// BEGIN_RDATA_NAMESPACE
+
+SSHFP::SSHFP(InputBuffer& buffer, size_t rdata_len)
+{
+    if (rdata_len < 2) {
+      isc_throw(InvalidRdataLength, "SSHFP record too short");
+    }
+
+    algorithm_ = buffer.readUint8();
+    fingerprint_type_ = buffer.readUint8();
+
+    rdata_len -= 2;
+    fingerprint_.resize(rdata_len);
+    buffer.readData(&fingerprint_[0], rdata_len);
+}
+
+SSHFP::SSHFP(const std::string& sshfp_str)
+{
+    std::istringstream iss(sshfp_str);
+    // peekc should be of iss's char_type for isspace to work
+    std::istringstream::char_type peekc;
+    std::stringbuf fingerprintbuf;
+    uint32_t algorithm, fingerprint_type;
+
+    iss >> algorithm >> fingerprint_type;
+    if (iss.bad() || iss.fail()) {
+      isc_throw(InvalidRdataText, "Invalid SSHFP text");
+    }
+    if ((algorithm < 1) || (algorithm > 2)) {
+      isc_throw(InvalidRdataText, "SSHFP algorithm number out of range");
+    }
+    if (fingerprint_type != 1) {
+      isc_throw(InvalidRdataText, "SSHFP fingerprint type out of range");
+    }
+
+    iss.read(&peekc, 1);
+    if (!iss.good() || !isspace(peekc, iss.getloc())) {
+      isc_throw(InvalidRdataText, "SSHFP presentation format error");
+    }
+
+    iss >> &fingerprintbuf;
+
+    algorithm_ = algorithm;
+    fingerprint_type_ = fingerprint_type;
+    decodeHex(fingerprintbuf.str(), fingerprint_);
+}
+
+SSHFP::SSHFP(uint8_t algorithm, uint8_t fingerprint_type, const string& fingerprint)
+{
+    if ((algorithm < 1) || (algorithm > 2)) {
+      isc_throw(InvalidRdataText, "SSHFP algorithm number out of range");
+    }
+    if (fingerprint_type != 1) {
+      isc_throw(InvalidRdataText, "SSHFP fingerprint type out of range");
+    }
+
+    algorithm_ = algorithm;
+    fingerprint_type_ = fingerprint_type;
+    decodeHex(fingerprint, fingerprint_);
+}
+
+SSHFP::SSHFP(const SSHFP& other) :
+  Rdata(), algorithm_(other.algorithm_), fingerprint_type_(other.fingerprint_type_), fingerprint_(other.fingerprint_)
+{}
+
+void
+SSHFP::toWire(OutputBuffer& buffer) const {
+    buffer.writeUint8(algorithm_);
+    buffer.writeUint8(fingerprint_type_);
+    buffer.writeData(&fingerprint_[0], fingerprint_.size());
+}
+
+void
+SSHFP::toWire(AbstractMessageRenderer& renderer) const {
+    renderer.writeUint8(algorithm_);
+    renderer.writeUint8(fingerprint_type_);
+    renderer.writeData(&fingerprint_[0], fingerprint_.size());
+}
+
+string
+SSHFP::toText() const {
+    return (lexical_cast<string>(static_cast<int>(algorithm_)) +
+            " " + lexical_cast<string>(static_cast<int>(fingerprint_type_)) +
+            " " + encodeHex(fingerprint_));
+}
+
+int
+SSHFP::compare(const Rdata& other) const {
+    const SSHFP& other_sshfp = dynamic_cast<const SSHFP&>(other);
+
+    /* This doesn't really make any sort of sense, but in the name of
+       consistency... */
+
+    if (algorithm_ < other_sshfp.algorithm_) {
+        return (-1);
+    } else if (algorithm_ > other_sshfp.algorithm_) {
+        return (1);
+    }
+
+    if (fingerprint_type_ < other_sshfp.fingerprint_type_) {
+        return (-1);
+    } else if (fingerprint_type_ > other_sshfp.fingerprint_type_) {
+        return (1);
+    }
+
+    size_t this_len = fingerprint_.size();
+    size_t other_len = other_sshfp.fingerprint_.size();
+    size_t cmplen = min(this_len, other_len);
+    int cmp = memcmp(&fingerprint_[0], &other_sshfp.fingerprint_[0], cmplen);
+    if (cmp != 0) {
+      return (cmp);
+    } else {
+      return ((this_len == other_len)
+	      ? 0 : (this_len < other_len) ? -1 : 1);
+    }
+}
+
+uint8_t
+SSHFP::getSSHFPAlgorithmNumber() const {
+    return (algorithm_);
+}
+
+uint8_t
+SSHFP::getSSHFPFingerprintType() const {
+    return (fingerprint_type_);
+}
+
+// END_RDATA_NAMESPACE
+// END_ISC_NAMESPACE

src/lib/dns/rdata/generic/sshfp_44.h

@@ -0,0 +1,58 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+// BEGIN_HEADER_GUARD
+
+#include <stdint.h>
+
+#include <string>
+
+#include <dns/name.h>
+#include <dns/rdata.h>
+
+// BEGIN_ISC_NAMESPACE
+
+// BEGIN_COMMON_DECLARATIONS
+// END_COMMON_DECLARATIONS
+
+// BEGIN_RDATA_NAMESPACE
+
+class SSHFP : public Rdata {
+public:
+    // BEGIN_COMMON_MEMBERS
+    // END_COMMON_MEMBERS
+
+    SSHFP(uint8_t algorithm, uint8_t fingerprint_type, const std::string& fingerprint);
+
+    ///
+    /// Specialized methods
+    ///
+    uint8_t getSSHFPAlgorithmNumber() const;
+    uint8_t getSSHFPFingerprintType() const;
+
+private:
+    /// Note: this is a prototype version; we may reconsider
+    /// this representation later.
+    uint8_t algorithm_;
+    uint8_t fingerprint_type_;
+    std::vector<uint8_t> fingerprint_;
+};
+
+// END_RDATA_NAMESPACE
+// END_ISC_NAMESPACE
+// END_HEADER_GUARD
+
+// Local Variables:
+// mode: c++
+// End:

src/lib/dns/rrset.cc

@@ -113,6 +113,16 @@ AbstractRRset::toWire(AbstractMessageRenderer& renderer) const {
     return (rrs_written);
 }
 
+bool
+AbstractRRset::isSameKind(const AbstractRRset& other) const {
+  // Compare classes last as they're likely to be identical. Compare
+  // names late in the list too, as these are expensive. So we compare
+  // types first, names second and classes last.
+  return (getType() == other.getType() &&
+	  getName() == other.getName() &&
+	  getClass() == other.getClass());
+}
+
 ostream&
 operator<<(ostream& os, const AbstractRRset& rrset) {
     os << rrset.toText();

src/lib/dns/rrset.h

@@ -475,6 +475,14 @@ public:
     /// \brief Clear the RRSIGs for this RRset
     virtual void removeRRsig() = 0;
 
+    /// \brief Check whether two RRsets are of the same kind
+    ///
+    /// Checks if two RRsets have the same name, RR type, and RR class.
+    ///
+    /// \param other Pointer to another AbstractRRset to compare
+    ///              against.
+    virtual bool isSameKind(const AbstractRRset& other) const;
+
     //@}
 };
 

src/lib/dns/tests/Makefile.am

@@ -33,6 +33,7 @@ run_unittests_SOURCES += rdata_in_a_unittest.cc rdata_in_aaaa_unittest.cc
 run_unittests_SOURCES += rdata_ns_unittest.cc rdata_soa_unittest.cc
 run_unittests_SOURCES += rdata_txt_like_unittest.cc
 run_unittests_SOURCES += rdata_mx_unittest.cc
+run_unittests_SOURCES += rdata_sshfp_unittest.cc
 run_unittests_SOURCES += rdata_ptr_unittest.cc rdata_cname_unittest.cc
 run_unittests_SOURCES += rdata_dname_unittest.cc
 run_unittests_SOURCES += rdata_afsdb_unittest.cc

src/lib/dns/tests/labelsequence_unittest.cc

@@ -13,11 +13,20 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <dns/labelsequence.h>
+#include <dns/name.h>
 #include <exceptions/exceptions.h>
 
 #include <gtest/gtest.h>
 
+#include <boost/functional/hash.hpp>
+
+#include <string>
+#include <set>
+
 using namespace isc::dns;
+using namespace std;
+
+namespace {
 
 class LabelSequenceTest : public ::testing::Test {
 public:
@@ -37,51 +46,14 @@ public:
 
 // Basic equality tests
 TEST_F(LabelSequenceTest, equals_sensitive) {
-    EXPECT_TRUE(ls1.equals(ls1));
-    EXPECT_FALSE(ls1.equals(ls2));
-    EXPECT_TRUE(ls1.equals(ls3));
-    EXPECT_FALSE(ls1.equals(ls4));
-    EXPECT_FALSE(ls1.equals(ls5));
-    EXPECT_FALSE(ls1.equals(ls6));
-    EXPECT_FALSE(ls1.equals(ls7));
-    EXPECT_FALSE(ls1.equals(ls8));
-
-    EXPECT_FALSE(ls2.equals(ls1));
-    EXPECT_TRUE(ls2.equals(ls2));
-    EXPECT_FALSE(ls2.equals(ls3));
-    EXPECT_FALSE(ls2.equals(ls4));
-    EXPECT_FALSE(ls2.equals(ls5));
-    EXPECT_FALSE(ls2.equals(ls6));
-    EXPECT_FALSE(ls2.equals(ls7));
-    EXPECT_FALSE(ls2.equals(ls8));
-
-    EXPECT_FALSE(ls4.equals(ls1));
-    EXPECT_FALSE(ls4.equals(ls2));
-    EXPECT_FALSE(ls4.equals(ls3));
-    EXPECT_TRUE(ls4.equals(ls4));
-    EXPECT_FALSE(ls4.equals(ls5));
-    EXPECT_FALSE(ls4.equals(ls6));
-    EXPECT_FALSE(ls4.equals(ls7));
-    EXPECT_FALSE(ls4.equals(ls8));
-
-    EXPECT_FALSE(ls5.equals(ls1));
-    EXPECT_FALSE(ls5.equals(ls2));
-    EXPECT_FALSE(ls5.equals(ls3));
-    EXPECT_FALSE(ls5.equals(ls4));
-    EXPECT_TRUE(ls5.equals(ls5));
-    EXPECT_FALSE(ls5.equals(ls6));
-    EXPECT_FALSE(ls5.equals(ls7));
-    EXPECT_FALSE(ls5.equals(ls8));
-}
-
-TEST_F(LabelSequenceTest, equals_insensitive) {
     EXPECT_TRUE(ls1.equals(ls1, true));
     EXPECT_FALSE(ls1.equals(ls2, true));
     EXPECT_TRUE(ls1.equals(ls3, true));
     EXPECT_FALSE(ls1.equals(ls4, true));
-    EXPECT_TRUE(ls1.equals(ls5, true));
-    EXPECT_TRUE(ls1.equals(ls6, true));
+    EXPECT_FALSE(ls1.equals(ls5, true));
+    EXPECT_FALSE(ls1.equals(ls6, true));
     EXPECT_FALSE(ls1.equals(ls7, true));
+    EXPECT_FALSE(ls1.equals(ls8, true));
 
     EXPECT_FALSE(ls2.equals(ls1, true));
     EXPECT_TRUE(ls2.equals(ls2, true));
@@ -90,14 +62,7 @@ TEST_F(LabelSequenceTest, equals_insensitive) {
     EXPECT_FALSE(ls2.equals(ls5, true));
     EXPECT_FALSE(ls2.equals(ls6, true));
     EXPECT_FALSE(ls2.equals(ls7, true));
-
-    EXPECT_TRUE(ls3.equals(ls1, true));
-    EXPECT_FALSE(ls3.equals(ls2, true));
-    EXPECT_TRUE(ls3.equals(ls3, true));
-    EXPECT_FALSE(ls3.equals(ls4, true));
-    EXPECT_TRUE(ls3.equals(ls5, true));
-    EXPECT_TRUE(ls3.equals(ls6, true));
-    EXPECT_FALSE(ls3.equals(ls7, true));
+    EXPECT_FALSE(ls2.equals(ls8, true));
 
     EXPECT_FALSE(ls4.equals(ls1, true));
     EXPECT_FALSE(ls4.equals(ls2, true));
@@ -106,14 +71,58 @@ TEST_F(LabelSequenceTest, equals_insensitive) {
     EXPECT_FALSE(ls4.equals(ls5, true));
     EXPECT_FALSE(ls4.equals(ls6, true));
     EXPECT_FALSE(ls4.equals(ls7, true));
+    EXPECT_FALSE(ls4.equals(ls8, true));
 
-    EXPECT_TRUE(ls5.equals(ls1, true));
+    EXPECT_FALSE(ls5.equals(ls1, true));
     EXPECT_FALSE(ls5.equals(ls2, true));
-    EXPECT_TRUE(ls5.equals(ls3, true));
+    EXPECT_FALSE(ls5.equals(ls3, true));
     EXPECT_FALSE(ls5.equals(ls4, true));
     EXPECT_TRUE(ls5.equals(ls5, true));
-    EXPECT_TRUE(ls5.equals(ls6, true));
+    EXPECT_FALSE(ls5.equals(ls6, true));
     EXPECT_FALSE(ls5.equals(ls7, true));
+    EXPECT_FALSE(ls5.equals(ls8, true));
+}
+
+TEST_F(LabelSequenceTest, equals_insensitive) {
+    EXPECT_TRUE(ls1.equals(ls1));
+    EXPECT_FALSE(ls1.equals(ls2));
+    EXPECT_TRUE(ls1.equals(ls3));
+    EXPECT_FALSE(ls1.equals(ls4));
+    EXPECT_TRUE(ls1.equals(ls5));
+    EXPECT_TRUE(ls1.equals(ls6));
+    EXPECT_FALSE(ls1.equals(ls7));
+
+    EXPECT_FALSE(ls2.equals(ls1));
+    EXPECT_TRUE(ls2.equals(ls2));
+    EXPECT_FALSE(ls2.equals(ls3));
+    EXPECT_FALSE(ls2.equals(ls4));
+    EXPECT_FALSE(ls2.equals(ls5));
+    EXPECT_FALSE(ls2.equals(ls6));
+    EXPECT_FALSE(ls2.equals(ls7));
+
+    EXPECT_TRUE(ls3.equals(ls1));
+    EXPECT_FALSE(ls3.equals(ls2));
+    EXPECT_TRUE(ls3.equals(ls3));
+    EXPECT_FALSE(ls3.equals(ls4));
+    EXPECT_TRUE(ls3.equals(ls5));
+    EXPECT_TRUE(ls3.equals(ls6));
+    EXPECT_FALSE(ls3.equals(ls7));
+
+    EXPECT_FALSE(ls4.equals(ls1));
+    EXPECT_FALSE(ls4.equals(ls2));
+    EXPECT_FALSE(ls4.equals(ls3));
+    EXPECT_TRUE(ls4.equals(ls4));
+    EXPECT_FALSE(ls4.equals(ls5));
+    EXPECT_FALSE(ls4.equals(ls6));
+    EXPECT_FALSE(ls4.equals(ls7));
+
+    EXPECT_TRUE(ls5.equals(ls1));
+    EXPECT_FALSE(ls5.equals(ls2));
+    EXPECT_TRUE(ls5.equals(ls3));
+    EXPECT_FALSE(ls5.equals(ls4));
+    EXPECT_TRUE(ls5.equals(ls5));
+    EXPECT_TRUE(ls5.equals(ls6));
+    EXPECT_FALSE(ls5.equals(ls7));
 }
 
 void
@@ -124,6 +133,9 @@ getDataCheck(const char* expected_data, size_t expected_len,
     const char* data = ls.getData(&len);
     ASSERT_EQ(expected_len, len) << "Expected data: " << expected_data <<
                                     " name: " << ls.getName().toText();
+    EXPECT_EQ(expected_len, ls.getDataLength()) <<
+        "Expected data: " << expected_data <<
+        " name: " << ls.getName().toText();
     for (size_t i = 0; i < len; ++i) {
         EXPECT_EQ(expected_data[i], data[i]) << "Difference at pos " << i <<
                                                 ": Expected data: " <<
@@ -251,3 +263,86 @@ TEST_F(LabelSequenceTest, isAbsolute) {
     ls3.stripLeft(2);
     ASSERT_TRUE(ls3.isAbsolute());
 }
+
+// The following are test data used in the getHash test below.  Normally
+// we use example/documentation domain names for testing, but in this case
+// we'd specifically like to use more realistic data, and are intentionally
+// using real-world samples: They are the NS names of root and some top level
+// domains as of this test.
+const char* const root_servers[] = {
+    "a.root-servers.net", "b.root-servers.net", "c.root-servers.net",
+    "d.root-servers.net", "e.root-servers.net", "f.root-servers.net",
+    "g.root-servers.net", "h.root-servers.net", "i.root-servers.net",
+    "j.root-servers.net", "k.root-servers.net", "l.root-servers.net",
+    "m.root-servers.net", NULL
+};
+const char* const gtld_servers[] = {
+    "a.gtld-servers.net", "b.gtld-servers.net", "c.gtld-servers.net",
+    "d.gtld-servers.net", "e.gtld-servers.net", "f.gtld-servers.net",
+    "g.gtld-servers.net", "h.gtld-servers.net", "i.gtld-servers.net",
+    "j.gtld-servers.net", "k.gtld-servers.net", "l.gtld-servers.net",
+    "m.gtld-servers.net", NULL
+};
+const char* const jp_servers[] = {
+    "a.dns.jp", "b.dns.jp", "c.dns.jp", "d.dns.jp", "e.dns.jp",
+    "f.dns.jp", "g.dns.jp", NULL
+};
+const char* const cn_servers[] = {
+    "a.dns.cn", "b.dns.cn", "c.dns.cn", "d.dns.cn", "e.dns.cn",
+    "ns.cernet.net", NULL
+};
+const char* const ca_servers[] = {
+    "k.ca-servers.ca", "e.ca-servers.ca", "a.ca-servers.ca", "z.ca-servers.ca",
+    "tld.isc-sns.net", "c.ca-servers.ca", "j.ca-servers.ca", "l.ca-servers.ca",
+    "sns-pb.isc.org", "f.ca-servers.ca", NULL
+};
+
+// A helper function used in the getHash test below.
+void
+hashDistributionCheck(const char* const* servers) {
+    const size_t BUCKETS = 64;  // constant used in the MessageRenderer
+    set<Name> names;
+    vector<size_t> hash_counts(BUCKETS);
+
+    // Store all test names and their super domain names (excluding the
+    // "root" label) in the set, calculates their hash values, and increments
+    // the counter for the corresponding hash "bucket".
+    for (size_t i = 0; servers[i] != NULL; ++i) {
+        const Name name(servers[i]);
+        for (size_t l = 0; l < name.getLabelCount() - 1; ++l) {
+            pair<set<Name>::const_iterator, bool> ret =
+                names.insert(name.split(l));
+            if (ret.second) {
+                hash_counts[LabelSequence((*ret.first)).getHash(false) %
+                            BUCKETS]++;
+            }
+        }
+    }
+
+    // See how many conflicts we have in the buckets.  For the testing purpose
+    // we expect there's at most 2 conflicts in each set, which is an
+    // arbitrary choice (it should happen to succeed with the hash function
+    // and data we are using; if it's not the case, maybe with an update to
+    // the hash implementation, we should revise the test).
+    for (size_t i = 0; i < BUCKETS; ++i) {
+        EXPECT_GE(3, hash_counts[i]);
+    }
+}
+
+TEST_F(LabelSequenceTest, getHash) {
+    // Trivial case.  The same sequence should have the same hash.
+    EXPECT_EQ(ls1.getHash(true), ls1.getHash(true));
+
+    // Check the case-insensitive mode behavior.
+    EXPECT_EQ(ls1.getHash(false), ls5.getHash(false));
+
+    // Check that the distribution of hash values is "not too bad" (such as
+    // everything has the same hash value due to a stupid bug).  It's
+    // difficult to check such things reliably.  We do some ad hoc tests here.
+    hashDistributionCheck(root_servers);
+    hashDistributionCheck(jp_servers);
+    hashDistributionCheck(cn_servers);
+    hashDistributionCheck(ca_servers);
+}
+
+}

src/lib/dns/tests/messagerenderer_unittest.cc

@@ -21,12 +21,16 @@
 
 #include <gtest/gtest.h>
 
+#include <boost/lexical_cast.hpp>
+
+#include <string>
 #include <vector>
 
 using isc::UnitTestUtil;
 using isc::dns::Name;
 using isc::dns::MessageRenderer;
 using isc::util::OutputBuffer;
+using boost::lexical_cast;
 
 namespace {
 class MessageRendererTest : public ::testing::Test {
@@ -142,13 +146,15 @@ TEST_F(MessageRendererTest, writeNameMixedCaseCompress) {
     renderer.writeName(Name("a.example.com."));
     renderer.writeName(Name("b.eXample.com."));
 
-    // Change the compression mode in the middle of rendering.  This is an
-    // unusual operation and is unlikely to happen in practice, but is still
-    // allowed in this API.
-    renderer.setCompressMode(MessageRenderer::CASE_INSENSITIVE);
-    renderer.writeName(Name("c.b.EXAMPLE.com."));
-    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, renderer.getData(),
-                        renderer.getLength(), &data[0], data.size());
+    // Change the compression mode in the middle of rendering.  This is not
+    // allowed in this implementation.
+    EXPECT_THROW(renderer.setCompressMode(MessageRenderer::CASE_INSENSITIVE),
+                 isc::InvalidParameter);
+
+    // Once the renderer is cleared, it's okay again.
+    renderer.clear();
+    EXPECT_NO_THROW(renderer.setCompressMode(
+                        MessageRenderer::CASE_INSENSITIVE));
 }
 
 TEST_F(MessageRendererTest, writeRootName) {
@@ -211,4 +217,20 @@ TEST_F(MessageRendererTest, setBufferErrors) {
     renderer.setBuffer(&new_buffer);
     EXPECT_NO_THROW(renderer.setBuffer(NULL));
 }
+
+TEST_F(MessageRendererTest, manyRRs) {
+    // Render a large number of names, and the confirm the resulting wire
+    // data store the expected names in the correct order (1000 is an
+    // arbitrary choice).
+    for (size_t i = 0; i < 1000; ++i) {
+        renderer.writeName(Name(lexical_cast<std::string>(i) + ".example"));
+    }
+    isc::util::InputBuffer b(renderer.getData(), renderer.getLength());
+    for (size_t i = 0; i < 1000; ++i) {
+        EXPECT_EQ(Name(lexical_cast<std::string>(i) + ".example"), Name(b));
+    }
+    // This will trigger trimming excessive hash items.  It shouldn't cause
+    // any disruption.
+    EXPECT_NO_THROW(renderer.clear());
+}
 }

src/lib/dns/tests/rdata_sshfp_unittest.cc

@@ -0,0 +1,94 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <algorithm>
+#include <string>
+
+#include <util/buffer.h>
+#include <dns/messagerenderer.h>
+#include <dns/rdata.h>
+#include <dns/rdataclass.h>
+#include <dns/rrclass.h>
+#include <dns/rrtype.h>
+
+#include <gtest/gtest.h>
+
+#include <dns/tests/unittest_util.h>
+#include <dns/tests/rdata_unittest.h>
+#include <boost/algorithm/string.hpp>
+
+using isc::UnitTestUtil;
+using namespace std;
+using namespace isc::dns;
+using namespace isc::util;
+using namespace isc::dns::rdata;
+
+namespace {
+class Rdata_SSHFP_Test : public RdataTest {
+    // there's nothing to specialize
+};
+
+const string sshfp_txt("2 1 123456789abcdef67890123456789abcdef67890");
+const generic::SSHFP rdata_sshfp(2, 1, "123456789abcdef67890123456789abcdef67890");
+
+TEST_F(Rdata_SSHFP_Test, createFromText) {
+    // Basic test
+    const generic::SSHFP rdata_sshfp2(sshfp_txt);
+    EXPECT_EQ(0, rdata_sshfp2.compare(rdata_sshfp));
+
+    // With different spacing
+    const generic::SSHFP rdata_sshfp3("2 1   123456789abcdef67890123456789abcdef67890");
+    EXPECT_EQ(0, rdata_sshfp3.compare(rdata_sshfp));
+
+    // Combination of lowercase and uppercase
+    const generic::SSHFP rdata_sshfp4("2 1   123456789ABCDEF67890123456789abcdef67890");
+    EXPECT_EQ(0, rdata_sshfp4.compare(rdata_sshfp));
+}
+
+TEST_F(Rdata_SSHFP_Test, badText) {
+    EXPECT_THROW(const generic::SSHFP rdata_sshfp("1"), InvalidRdataText);
+    EXPECT_THROW(const generic::SSHFP rdata_sshfp("1 2"), InvalidRdataText);
+    EXPECT_THROW(const generic::SSHFP rdata_sshfp("BUCKLE MY SHOES"), InvalidRdataText);
+    EXPECT_THROW(const generic::SSHFP rdata_sshfp("1 2 foo bar"), InvalidRdataText);
+}
+
+TEST_F(Rdata_SSHFP_Test, copy) {
+    const generic::SSHFP rdata_sshfp2(rdata_sshfp);
+    EXPECT_EQ(0, rdata_sshfp.compare(rdata_sshfp2));
+}
+
+TEST_F(Rdata_SSHFP_Test, createFromWire) {
+    // Basic test
+    EXPECT_EQ(0, rdata_sshfp.compare(
+                  *rdataFactoryFromFile(RRType("SSHFP"), RRClass("IN"),
+                                        "rdata_sshfp_fromWire")));
+    // Combination of lowercase and uppercase
+    EXPECT_EQ(0, rdata_sshfp.compare(
+                  *rdataFactoryFromFile(RRType("SSHFP"), RRClass("IN"),
+                                        "rdata_sshfp_fromWire2")));
+    // TBD: more tests
+}
+
+TEST_F(Rdata_SSHFP_Test, toText) {
+    EXPECT_TRUE(boost::iequals(sshfp_txt, rdata_sshfp.toText()));
+}
+
+TEST_F(Rdata_SSHFP_Test, getSSHFPAlgorithmNumber) {
+    EXPECT_EQ(2, rdata_sshfp.getSSHFPAlgorithmNumber());
+}
+
+TEST_F(Rdata_SSHFP_Test, getSSHFPFingerprintType) {
+    EXPECT_EQ(1, rdata_sshfp.getSSHFPFingerprintType());
+}
+}

src/lib/dns/tests/rrset_unittest.cc

@@ -112,6 +112,20 @@ TEST_F(RRsetTest, setName) {
     EXPECT_EQ(test_nsname, rrset_a.getName());
 }
 
+TEST_F(RRsetTest, isSameKind) {
+    RRset rrset_w(test_name, RRClass::IN(), RRType::A(), RRTTL(3600));
+    RRset rrset_x(test_name, RRClass::IN(), RRType::A(), RRTTL(3600));
+    RRset rrset_y(test_name, RRClass::IN(), RRType::NS(), RRTTL(3600));
+    RRset rrset_z(test_name, RRClass::CH(), RRType::A(), RRTTL(3600));
+    RRset rrset_p(test_nsname, RRClass::IN(), RRType::A(), RRTTL(3600));
+
+    EXPECT_TRUE(rrset_w.isSameKind(rrset_w));
+    EXPECT_TRUE(rrset_w.isSameKind(rrset_x));
+    EXPECT_FALSE(rrset_w.isSameKind(rrset_y));
+    EXPECT_FALSE(rrset_w.isSameKind(rrset_z));
+    EXPECT_FALSE(rrset_w.isSameKind(rrset_p));
+}
+
 void
 addRdataTestCommon(const RRset& rrset) {
     EXPECT_EQ(2, rrset.getRdataCount());

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

@@ -44,6 +44,7 @@ BUILT_SOURCES += rdata_rp_fromWire1.wire rdata_rp_fromWire2.wire
 BUILT_SOURCES += rdata_rp_fromWire3.wire rdata_rp_fromWire4.wire
 BUILT_SOURCES += rdata_rp_fromWire5.wire rdata_rp_fromWire6.wire
 BUILT_SOURCES += rdata_rp_toWire1.wire rdata_rp_toWire2.wire
+BUILT_SOURCES += rdata_sshfp_fromWire1.wire rdata_sshfp_fromWire2.wire
 BUILT_SOURCES += rdata_afsdb_fromWire1.wire rdata_afsdb_fromWire2.wire
 BUILT_SOURCES += rdata_afsdb_fromWire3.wire rdata_afsdb_fromWire4.wire
 BUILT_SOURCES += rdata_afsdb_fromWire5.wire
@@ -125,6 +126,8 @@ EXTRA_DIST += rdata_rp_fromWire1.spec rdata_rp_fromWire2.spec
 EXTRA_DIST += rdata_rp_fromWire3.spec rdata_rp_fromWire4.spec
 EXTRA_DIST += rdata_rp_fromWire5.spec rdata_rp_fromWire6.spec
 EXTRA_DIST += rdata_rp_toWire1.spec rdata_rp_toWire2.spec
+EXTRA_DIST += rdata_sshfp_fromWire rdata_sshfp_fromWire2
+EXTRA_DIST += rdata_sshfp_fromWire1.spec rdata_sshfp_fromWire2.spec
 EXTRA_DIST += rdata_afsdb_fromWire1.spec rdata_afsdb_fromWire2.spec
 EXTRA_DIST += rdata_afsdb_fromWire3.spec rdata_afsdb_fromWire4.spec
 EXTRA_DIST += rdata_afsdb_fromWire5.spec

src/lib/dns/tests/testdata/rdata_mx_fromWire

@@ -11,5 +11,5 @@
   00 0a
 # EXCHANGE: non compressed
 # 4  5  6  7  8  9 10  1  2  3  4  5  6  7  8  9 (bytes)
-#(4) m  x (7) e  x  a  m  p  l  e (3) c  o  m  .
+#(2) m  x (7) e  x  a  m  p  l  e (3) c  o  m  .
  02 6d 78 07 65 78 61 6d 70 6c 65 03 63 6f 6d 00

src/lib/dns/tests/testdata/rdata_sshfp_fromWire

@@ -0,0 +1,4 @@
+# SSHFP RDATA, RDLEN=22
+0016
+# ALGORITHM=2 FINGERPRINT_TYPE=1 FINGERPRINT=123456789abcdef67890123456789abcdef67890
+02 01 123456789abcdef67890123456789abcdef67890

src/lib/dns/tests/testdata/rdata_sshfp_fromWire1.spec

@@ -0,0 +1,6 @@
+#
+# A simplest form of SSHFP: all default parameters
+#
+[custom]
+sections: sshfp
+[sshfp]

src/lib/dns/tests/testdata/rdata_sshfp_fromWire2

@@ -0,0 +1,4 @@
+# SSHFP RDATA, RDLEN=22
+0016
+# ALGORITHM=2 FINGERPRINT_TYPE=1 FINGERPRINT=123456789ABCDEF67890123456789abcdef67890
+02 01 123456789ABCDEF67890123456789abcdef67890

src/lib/dns/tests/testdata/rdata_sshfp_fromWire2.spec

@@ -0,0 +1,7 @@
+#
+# SSHFP RDATA
+#
+[custom]
+sections: sshfp
+[sshfp]
+fingerprint: 123456789abcdef67890123456789abcdef67890

src/lib/log/logger.h

@@ -15,14 +15,17 @@
 #ifndef __LOGGER_H
 #define __LOGGER_H
 
+#include <cassert>
 #include <cstdlib>
 #include <string>
+#include <cstring>
 
 #include <exceptions/exceptions.h>
 #include <log/logger_level.h>
 #include <log/message_types.h>
 #include <log/log_formatter.h>
 
+
 namespace isc {
 namespace log {
 
@@ -68,9 +71,19 @@ namespace log {
 /// is referred to using a symbolic name.  The logging code uses this name as
 /// a key in a dictionary from which the message text is obtained.  Such a
 /// system allows for the optional replacement of message text at run time.
-/// More details about the message disction (and the compiler used to create
+/// More details about the message dictionary (and the compiler used to create
 /// the symbol definitions) can be found in other modules in the src/lib/log
 /// directory.
+///
+/// \section LoggingApiImplementationIssues Implementation Issues
+/// Owing to the way that the logging is implemented, notably that loggers can
+/// be declared as static external objects, there is a restriction on the
+/// length of the name of a logger component (i.e. the length of
+/// the string passed to the Logger constructor) to a maximum of 31 characters.
+/// There is no reason for this particular value other than limiting the amount
+/// of memory used.  It is defined by the constant Logger::MAX_LOGGER_NAME_SIZE,
+/// and can be made larger (or smaller) if so desired.  Note however, using a
+/// logger name larger than this limit will cause an assertion failure.
 
 class LoggerImpl;   // Forward declaration of the implementation class
 
@@ -99,6 +112,8 @@ public:
 
 class Logger {
 public:
+    /// Maximum size of a logger name
+    static const size_t MAX_LOGGER_NAME_SIZE = 31;
 
     /// \brief Constructor
     ///
@@ -107,8 +122,26 @@ public:
     /// \param name Name of the logger.  If the name is that of the root name,
     /// this creates an instance of the root logger; otherwise it creates a
     /// child of the root logger.
-    Logger(const std::string& name) : loggerptr_(NULL), name_(name)
-    {}
+    ///
+    /// \note The name of the logger may be no longer than MAX_LOGGER_NAME_SIZE
+    /// else the program will halt with an assertion failure.  This restriction
+    /// allows loggers to be declared statically: the name is stored in a
+    /// fixed-size array to avoid the need to allocate heap storage during
+    /// program initialization (which causes problems on some operating
+    /// systems).
+    ///
+    /// \note Note also that there is no constructor taking a std::string. This
+    /// minimises the possibility of initializing a static logger with a
+    /// string, so leading to problems mentioned above.
+    Logger(const char* name) : loggerptr_(NULL) {
+        assert(std::strlen(name) < sizeof(name_));
+        // Do the copy.  Note that the assertion above has checked that the
+        // contents of "name" and a trailing null will fit within the space
+        // allocated for name_, so we could use strcpy here and be safe.
+        // However, a bit of extra paranoia doesn't hurt.
+        std::strncpy(name_, name, sizeof(name_));
+        assert(name_[sizeof(name_) - 1] == '\0');
+    }
 
     /// \brief Destructor
     virtual ~Logger();
@@ -256,8 +289,8 @@ private:
     /// \brief Initialize Underlying Implementation and Set loggerptr_
     void initLoggerImpl();
 
-    LoggerImpl*     loggerptr_;     ///< Pointer to the underlying logger
-    std::string     name_;          ///< Copy of the logger name
+    LoggerImpl* loggerptr_;                  ///< Pointer to underlying logger
+    char        name_[MAX_LOGGER_NAME_SIZE + 1]; ///< Copy of the logger name
 };
 
 } // namespace log

src/lib/log/logger_manager.cc

@@ -95,6 +95,10 @@ void
 LoggerManager::init(const std::string& root, isc::log::Severity severity,
                     int dbglevel, const char* file)
 {
+    // Load in the messages declared in the program and registered by
+    // statically-declared MessageInitializer objects.
+    MessageInitializer::loadDictionary();
+
     // Save name, severity and debug level for later.  No need to save the
     // file name as once the local message file is read the messages will
     // not be lost.

src/lib/log/message_initializer.cc

@@ -12,25 +12,80 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <cassert>
+#include <cstdlib>
 #include <log/message_dictionary.h>
 #include <log/message_initializer.h>
 
+
+// As explained in the header file, initialization of the message dictionary
+// is a two-stage process:
+// 1) In the MessageInitializer constructor, a pointer to the array of
+//    messages is stored in a pre-defined array.  Since the MessageInitializers
+//    are declared statically outside a program unit, this takes place before
+//    main() is called.  As no heap storage is allocated in this process, we
+//    should avoid the static initialization fiasco in cases where
+//    initialization of system libraries is also carried out at the same time.
+// 2) After main() starts executing, loadDictionary() is called.
+//
+//
+
+namespace {
+
+// Declare the array of pointers to value arrays.
+const char** logger_values[isc::log::MessageInitializer::MAX_MESSAGE_ARRAYS];
+
+// Declare the index used to access the array.  As this needs to be initialized
+// at first used, it is accessed it via a function.
+size_t& getIndex() {
+    static size_t index = 0;
+    return (index);
+}
+
+}
+
+
 namespace isc {
 namespace log {
 
-// Constructor.  Just retrieve the global dictionary and load the IDs and
-// associated text into it.
+// Constructor.  Add the pointer to the message array to the global array.
+// This method will trigger an assertion failure if the array overflows.
 
 MessageInitializer::MessageInitializer(const char* values[]) {
+    assert(getIndex() < MAX_MESSAGE_ARRAYS);
+    logger_values[getIndex()++] = values;
+}
+
+// Return the number of arrays registered but not yet loaded.
+
+size_t
+MessageInitializer::getPendingCount() {
+    return (getIndex());
+}
+
+// Load the messages in the arrays registered in the logger_values array
+// into the global dictionary.
+
+void
+MessageInitializer::loadDictionary() {
     MessageDictionary& global = MessageDictionary::globalDictionary();
-    std::vector<std::string> repeats = global.load(values);
 
-    // Append the IDs in the list just loaded (the "repeats") to the global list
-    // of duplicate IDs.
-    if (!repeats.empty()) {
-        std::vector<std::string>& duplicates = getDuplicates();
-        duplicates.insert(duplicates.end(), repeats.begin(), repeats.end());
+    for (size_t i = 0; i < getIndex(); ++i) {
+        std::vector<std::string> repeats = global.load(logger_values[i]);
+
+        // Append the IDs in the list just loaded (the "repeats") to the
+        // global list of duplicate IDs.
+        if (!repeats.empty()) {
+            std::vector<std::string>& duplicates = getDuplicates();
+            duplicates.insert(duplicates.end(), repeats.begin(),
+                              repeats.end());
+        }
     }
+
+    // ... and mark that the messages have been loaded.  (This avoids a lot
+    // of "duplicate message ID" messages in some of the unit tests where the
+    // logging initialization code may be called multiple times.)
+    getIndex() = 0;
 }
 
 // Return reference to duplicate array

src/lib/log/message_initializer.h

@@ -15,6 +15,7 @@
 #ifndef __MESSAGEINITIALIZER_H
 #define __MESSAGEINITIALIZER_H
 
+#include <cstdlib>
 #include <string>
 #include <vector>
 #include <log/message_dictionary.h>
@@ -37,28 +38,63 @@ namespace log {
 ///             :
 ///         NULL
 ///     };
-///     MessageDictionaryHelper xyz(values);
+///     MessageInitializer xyz(values);
 ///
-/// This will automatically add the message ID/text pairs to the global
-/// dictionary during initialization - all that is required is that the module
-/// containing the definition is included into the final executable.
+/// All that needed is for the module containing the definitions to be
+/// included in the execution unit.
 ///
-/// Messages are added via the MessageDictionary::add() method, so any
-/// duplicates are stored in the the global dictionary's overflow vector whence
-/// they can be retrieved at run-time.
+/// To avoid static initialization fiasco problems, the initialization is
+/// carried out in two stages:
+/// - The constructor adds a pointer to the values array to a pre-defined array
+///   of pointers.
+/// - During the run-time initialization of the logging system, the static
+///   method loadDictionary() is called to load the message dictionary.
+/// This way, no heap storage is allocated during the static initialization,
+/// something that may give problems on some operating systems.
+///
+/// \note The maximum number of message arrays that can be added to the
+/// dictionary in this way is defined by the constant
+/// MessageInitializer::MAX_MESSAGE_ARRAYS.  This is set to 256 as a compromise
+/// between wasted space and allowing for future expansion, but can be
+/// changed (by editing the source file) to any value desired.
+///
+/// When messages are added to the dictionary, the are added via the
+/// MessageDictionary::add() method, so any duplicates are stored in the
+/// global dictionary's overflow vector whence they can be retrieved at
+/// run-time.
 
 class MessageInitializer {
 public:
+    /// Maximum number of message arrays that can be initialized in this way
+    static const size_t MAX_MESSAGE_ARRAYS = 256;
 
     /// \brief Constructor
     ///
-    /// Adds the array of values to the global dictionary, and notes any
-    /// duplicates.
+    /// Adds a pointer to the array of messages to the global array of
+    /// pointers to message arrays.
     ///
     /// \param values NULL-terminated array of alternating identifier strings
-    /// and associated message text.
+    /// and associated message text. N.B. This object stores a pointer to the
+    /// passed array; the array MUST remain valid at least until
+    /// loadDictionary() has been called.
     MessageInitializer(const char* values[]);
 
+    /// \brief Obtain pending load count
+    ///
+    /// Returns the number of message arrays that will be loaded by the next
+    /// call to loadDictionary().
+    ///
+    /// \return Number of registered message arrays.  This is reset to zero
+    ///         when loadDictionary() is called.
+    static size_t getPendingCount();
+
+    /// \brief Run-Time Initialization
+    ///
+    /// Loops through the internal array of pointers to message arrays
+    /// and adds the messages to the internal dictionary.  This is called
+    /// during run-time initialization.
+    static void loadDictionary();
+
     /// \brief Return Duplicates
     ///
     /// When messages are added to the global dictionary, any duplicates are

src/lib/log/tests/.gitignore

@@ -2,6 +2,8 @@
 /destination_test.sh
 /init_logger_test
 /init_logger_test.sh
+/initializer_unittests_1
+/initializer_unittests_2
 /local_file_test.sh
 /logger_example
 /run_unittests

src/lib/log/tests/Makefile.am

@@ -3,15 +3,52 @@ SUBDIRS = .
 AM_CPPFLAGS = -I$(top_builddir)/src/lib -I$(top_srcdir)/src/lib
 AM_CPPFLAGS += $(BOOST_INCLUDES)
 AM_CXXFLAGS = $(B10_CXXFLAGS)
+AM_LDADD    =
+AM_LDFLAGS  =
 
 if USE_STATIC_LINK
-AM_LDFLAGS = -static
+AM_LDFLAGS += -static
 endif
 
 CLEANFILES = *.gcno *.gcda
 
-TESTS =
+noinst_PROGRAMS = logger_example
+logger_example_SOURCES = logger_example.cc
+logger_example_CPPFLAGS = $(AM_CPPFLAGS)
+logger_example_LDFLAGS = $(AM_LDFLAGS)
+logger_example_LDADD  = $(AM_LDADD) $(LOG4CPLUS_LIBS)
+logger_example_LDADD += $(top_builddir)/src/lib/log/liblog.la
+logger_example_LDADD += $(top_builddir)/src/lib/util/libutil.la
+logger_example_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
+
+noinst_PROGRAMS += init_logger_test
+init_logger_test_SOURCES = init_logger_test.cc
+init_logger_test_CPPFLAGS = $(AM_CPPFLAGS)
+init_logger_test_LDFLAGS = $(AM_LDFLAGS)
+init_logger_test_LDADD  = $(AM_LDADD) $(LOG4CPLUS_LIBS)
+init_logger_test_LDADD += $(top_builddir)/src/lib/log/liblog.la
+init_logger_test_LDADD += $(top_builddir)/src/lib/util/libutil.la
+init_logger_test_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
+
 if HAVE_GTEST
+TESTS =
+
+# Define the flags used in each set of tests
+if USE_CLANGPP
+# Workaround unused variables tcout and tcerr in log4cplus's streams.h.
+AM_CXXFLAGS += -Wno-unused-variable
+endif
+AM_CPPFLAGS += $(GTEST_INCLUDES) $(LOG4CPLUS_INCLUDES)
+AM_LDFLAGS  += $(GTEST_LDFLAGS)
+
+AM_LDADD += $(GTEST_LDADD)
+AM_LDADD += $(top_builddir)/src/lib/log/liblog.la
+AM_LDADD += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la
+AM_LDADD += $(top_builddir)/src/lib/util/libutil.la
+AM_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
+AM_LDADD += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la
+
+# Set of unit tests for the general logging classes
 TESTS += run_unittests
 run_unittests_SOURCES  = run_unittests.cc
 run_unittests_SOURCES += log_formatter_unittest.cc
@@ -23,47 +60,39 @@ run_unittests_SOURCES += logger_support_unittest.cc
 run_unittests_SOURCES += logger_unittest.cc
 run_unittests_SOURCES += logger_specification_unittest.cc
 run_unittests_SOURCES += message_dictionary_unittest.cc
-run_unittests_SOURCES += message_initializer_unittest_2.cc
-run_unittests_SOURCES += message_initializer_unittest.cc
 run_unittests_SOURCES += message_reader_unittest.cc
 run_unittests_SOURCES += output_option_unittest.cc
 
-run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES) $(LOG4CPLUS_INCLUDES)
-run_unittests_LDFLAGS  = $(AM_LDFLAGS)  $(GTEST_LDFLAGS)
-
+run_unittests_CPPFLAGS = $(AM_CPPFLAGS)
 run_unittests_CXXFLAGS = $(AM_CXXFLAGS)
-if USE_CLANGPP
-# This is to workaround unused variables tcout and tcerr in
-# log4cplus's streams.h.
-run_unittests_CXXFLAGS += -Wno-unused-variable
-endif
-run_unittests_LDADD  = $(GTEST_LDADD)
-run_unittests_LDADD += $(top_builddir)/src/lib/log/liblog.la
-run_unittests_LDADD += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la
-run_unittests_LDADD += $(top_builddir)/src/lib/util/libutil.la
-run_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
-run_unittests_LDADD += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la
-endif
+run_unittests_LDADD    = $(AM_LDADD)
+run_unittests_LDFLAGS  = $(AM_LDFLAGS)
 
-noinst_PROGRAMS = logger_example
-logger_example_SOURCES = logger_example.cc
-logger_example_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
-logger_example_LDFLAGS = $(AM_LDFLAGS)
-logger_example_LDADD  = $(LOG4CPLUS_LIBS)
-logger_example_LDADD += $(top_builddir)/src/lib/log/liblog.la
-logger_example_LDADD += $(top_builddir)/src/lib/util/libutil.la
-logger_example_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
+# logging initialization tests.  These are put in separate programs to
+# ensure that the initialization status at the start of each test is known,
+# and to prevent circumstances where the execution of one test affects the
+# starting conditions of the next.
+TESTS += initializer_unittests_1
+initializer_unittests_1_SOURCES  = run_initializer_unittests.cc
+initializer_unittests_1_SOURCES += message_initializer_1_unittest.cc
+initializer_unittests_1_SOURCES += message_initializer_1a_unittest.cc
 
-noinst_PROGRAMS += init_logger_test
-init_logger_test_SOURCES = init_logger_test.cc
-init_logger_test_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
-init_logger_test_LDFLAGS = $(AM_LDFLAGS)
-init_logger_test_LDADD  = $(LOG4CPLUS_LIBS)
-init_logger_test_LDADD += $(top_builddir)/src/lib/log/liblog.la
-init_logger_test_LDADD += $(top_builddir)/src/lib/util/libutil.la
-init_logger_test_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
+initializer_unittests_1_CPPFLAGS = $(AM_CPPFLAGS)
+initializer_unittests_1_CXXFLAGS = $(AM_CXXFLAGS)
+initializer_unittests_1_LDADD    = $(AM_LDADD)
+initializer_unittests_1_LDFLAGS  = $(AM_LDFLAGS)
+
+TESTS += initializer_unittests_2
+initializer_unittests_2_SOURCES  = run_initializer_unittests.cc
+initializer_unittests_2_SOURCES += message_initializer_2_unittest.cc
+
+initializer_unittests_2_CPPFLAGS = $(AM_CPPFLAGS)
+initializer_unittests_2_CXXFLAGS = $(AM_CXXFLAGS)
+initializer_unittests_2_LDADD    = $(AM_LDADD)
+initializer_unittests_2_LDFLAGS  = $(AM_LDFLAGS)
 
 noinst_PROGRAMS += $(TESTS)
+endif
 
 # Additional test using the shell.  These are principally tests
 # where the global logging environment is affected, and where the

src/lib/log/tests/logger_example.cc

@@ -105,7 +105,7 @@ void usage() {
 // messages.  Looking at the output determines whether the program worked.
 
 int main(int argc, char** argv) {
-    const string ROOT_NAME = "example";
+    const char* ROOT_NAME = "example";
 
     bool                sw_found = false;   // Set true if switch found
     bool                c_found = false;    // Set true if "-c" found

src/lib/log/tests/logger_manager_unittest.cc

@@ -201,7 +201,7 @@ TEST_F(LoggerManagerTest, FileLogger) {
         // Scope-limit the logger to ensure it is destroyed after the brief
         // check.  This adds weight to the idea that the logger will not
         // keep the file open.
-        Logger logger(file_spec.getLoggerName());
+        Logger logger(file_spec.getLoggerName().c_str());
 
         LOG_FATAL(logger, LOG_DUPLICATE_MESSAGE_ID).arg("test");
         ids.push_back(LOG_DUPLICATE_MESSAGE_ID);
@@ -223,7 +223,7 @@ TEST_F(LoggerManagerTest, FileLogger) {
     manager.process(spec.begin(), spec.end());
 
     // Create a new instance of the logger and log three more messages.
-    Logger logger(file_spec.getLoggerName());
+    Logger logger(file_spec.getLoggerName().c_str());
 
     LOG_FATAL(logger, LOG_NO_SUCH_MESSAGE).arg("test");
     ids.push_back(LOG_NO_SUCH_MESSAGE);
@@ -275,7 +275,7 @@ TEST_F(LoggerManagerTest, FileSizeRollover) {
     // three files as for the log4cplus implementation, the files appear to
     // be rolled after the message is logged.
     {
-        Logger logger(file_spec.getLoggerName());
+        Logger logger(file_spec.getLoggerName().c_str());
         LOG_FATAL(logger, LOG_NO_SUCH_MESSAGE).arg(big_arg);
         LOG_FATAL(logger, LOG_DUPLICATE_NAMESPACE).arg(big_arg);
     }
@@ -295,7 +295,7 @@ TEST_F(LoggerManagerTest, FileSizeRollover) {
     // a .3 version does not exist.
     manager.process(spec);
     {
-        Logger logger(file_spec.getLoggerName());
+        Logger logger(file_spec.getLoggerName().c_str());
         LOG_FATAL(logger, LOG_NO_MESSAGE_TEXT).arg(big_arg);
     }
 

src/lib/log/tests/logger_unittest.cc

@@ -58,8 +58,8 @@ TEST_F(LoggerTest, Name) {
 
 TEST_F(LoggerTest, GetLogger) {
 
-    const string name1 = "alpha";
-    const string name2 = "beta";
+    const char* name1 = "alpha";
+    const char* name2 = "beta";
 
     // Instantiate two loggers that should be the same
     Logger logger1(name1);
@@ -348,3 +348,30 @@ TEST_F(LoggerTest, IsDebugEnabledLevel) {
     EXPECT_TRUE(logger.isDebugEnabled(MID_LEVEL));
     EXPECT_TRUE(logger.isDebugEnabled(MAX_DEBUG_LEVEL));
 }
+
+// Check that if a logger name is too long, it triggers the appropriate
+// assertion.
+
+TEST_F(LoggerTest, LoggerNameLength) {
+    // The following statements should just declare a logger and nothing
+    // should happen.
+    string ok1(Logger::MAX_LOGGER_NAME_SIZE - 1, 'x');
+    Logger l1(ok1.c_str());
+    EXPECT_EQ(getRootLoggerName() + "." + ok1, l1.getName());
+
+    string ok2(Logger::MAX_LOGGER_NAME_SIZE, 'x');
+    Logger l2(ok2.c_str());
+    EXPECT_EQ(getRootLoggerName() + "." + ok2, l2.getName());
+
+    // Note: Not all systems have EXPECT_DEATH.  As it is a macro we can just
+    // test for its presence and bypass the test if not available.
+#ifdef EXPECT_DEATH
+    // Too long a logger name should trigger an assertion failure.
+    // Note that we just check that it dies - we don't check what message is
+    // output.
+    EXPECT_DEATH({
+                    string ok3(Logger::MAX_LOGGER_NAME_SIZE + 1, 'x');
+                    Logger l3(ok3.c_str());
+                 }, ".*");
+#endif
+}

src/lib/log/tests/message_initializer_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -12,11 +12,11 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <cstddef>
-#include <string>
-#include <gtest/gtest.h>
 #include <log/message_dictionary.h>
 #include <log/message_initializer.h>
+#include <boost/lexical_cast.hpp>
+#include <gtest/gtest.h>
+#include <string>
 
 using namespace isc;
 using namespace isc::log;
@@ -40,27 +40,36 @@ const char* values2[] = {
 }
 
 // Statically initialize the global dictionary with those messages.  Three sets
-// are used to check that the declaration of separate initializer objects really// does combine the messages. (The third set is declared in the separately-
-// compiled file message_identifier_initializer_unittest_2.cc.)
-
-MessageInitializer init_message_initializer_unittest_1(values1);
-MessageInitializer init_message_initializer_unittest_2(values2);
-
-
-class MessageInitializerTest : public ::testing::Test {
-protected:
-    MessageInitializerTest()
-    {
-    }
-};
+// are used to check that the declaration of separate initializer objects
+// really does combine the messages. (The third set - declaring message IDs
+// GLOBAL5 and GLOBAL6) is declared in the separately-compiled file,
+// message_identifier_initializer_1a_unittest.cc.)
 
+const MessageInitializer init_message_initializer_unittest_1(values1);
+const MessageInitializer init_message_initializer_unittest_2(values2);
 
 // Check that the global dictionary is initialized with the specified
 // messages.
 
-TEST_F(MessageInitializerTest, MessageTest) {
+TEST(MessageInitializerTest1, MessageTest) {
     MessageDictionary& global = MessageDictionary::globalDictionary();
 
+    // Pointers to the message arrays should have been stored, but none of the
+    // messages should yet be in the dictionary.
+    for (int i = 1; i <= 6; ++i) {
+        string symbol = string("GLOBAL") + boost::lexical_cast<std::string>(i);
+        EXPECT_EQ(string(""), global.getText(symbol));
+    }
+
+    // Load the dictionary - this should clear the message array pending count.
+    // (N.B. We do not check for a known value before the call, only that the
+    // value is not zero.  This is because libraries against which the test
+    // is linked may have registered their own message arrays.)
+    EXPECT_NE(0, MessageInitializer::getPendingCount());
+    MessageInitializer::loadDictionary();
+    EXPECT_EQ(0, MessageInitializer::getPendingCount());
+
+    // ... and check the messages loaded.
     EXPECT_EQ(string("global message one"), global.getText("GLOBAL1"));
     EXPECT_EQ(string("global message two"), global.getText("GLOBAL2"));
     EXPECT_EQ(string("global message three"), global.getText("GLOBAL3"));

src/lib/log/tests/message_initializer_unittest_2.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -33,7 +33,5 @@ const char* values3[] = {
 
 }
 
-// Statically initialize the global dictionary with those messages.
-// Three sets are used to check that the declaration of separate
-// initializer objects really does combine the messages.
-MessageInitializer init_message_initializer_unittest_3(values3);
+// Register the messages for loading into the global dictionary
+const MessageInitializer init_message_initializer_unittest_3(values3);

src/lib/log/tests/message_initializer_2_unittest.cc

@@ -0,0 +1,48 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <log/message_initializer.h>
+#include <gtest/gtest.h>
+
+using namespace isc::log;
+
+// Declare a set of messages to go into the global dictionary.
+
+namespace {
+const char* values[] = {
+    "GLOBAL1", "global message one",
+    "GLOBAL2", "global message two",
+    NULL
+};
+}
+
+TEST(MessageInitializerTest2, MessageLoadTest) {
+    // Load the maximum number of message arrays allowed.  Some arrays may
+    // already have been loaded because of static initialization from modules
+    // in libraries linked against the test program, hence the reason for the
+    // loop starting from the value returned by getPendingCount() instead of 0.
+    for (size_t i = MessageInitializer::getPendingCount();
+         i < MessageInitializer::MAX_MESSAGE_ARRAYS; ++i) {
+        MessageInitializer initializer1(values);
+    }
+
+    // Note: Not all systems have EXPECT_DEATH.  As it is a macro we can just
+    // test for its presence and bypass the test if not available.
+#ifdef EXPECT_DEATH
+    // Adding one more should take us over the limit.
+    EXPECT_DEATH({
+        MessageInitializer initializer2(values);
+        }, ".*");
+#endif
+}

src/lib/log/tests/run_initializer_unittests.cc

@@ -0,0 +1,24 @@
+// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <gtest/gtest.h>
+#include <util/unittests/run_all.h>
+
+#include <log/logger_support.h>
+
+int
+main(int argc, char* argv[]) {
+    ::testing::InitGoogleTest(&argc, argv);
+    return (isc::util::unittests::run_all());
+}

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

@@ -34,7 +34,7 @@ logger = isc.log.Logger("notify_out")
 # initialized yet. see trac ticket #1103
 from isc.dns import *
 
-ZONE_NEW_DATA_READY_CMD = 'zone_new_data_ready'
+ZONE_NEW_DATA_READY_CMD = 'notify'
 _MAX_NOTIFY_NUM = 30
 _MAX_NOTIFY_TRY_NUM = 5
 _EVENT_NONE = 0
@@ -164,17 +164,19 @@ class NotifyOut:
         the only interface for class NotifyOut which can be called
         by other object.
           Internally, the function only set the zone's notify-reply
-        timeout to now, then notify message will be sent out. '''
+        timeout to now, then notify message will be sent out.
+        Returns False if the zone/class is not known, True if it is
+        (even if there are no slaves)'''
         if zone_name[len(zone_name) - 1] != '.':
             zone_name += '.'
 
         zone_id = (zone_name, zone_class)
         if zone_id not in self._notify_infos:
-            return
+            return False
 
         # Has no slave servers, skip it.
         if (len(self._notify_infos[zone_id].notify_slaves) <= 0):
-            return
+            return True
 
         with self._lock:
             if (self.notify_num >= _MAX_NOTIFY_NUM) or (zone_id in self._notifying_zones):
@@ -186,6 +188,7 @@ class NotifyOut:
                 self._notifying_zones.append(zone_id)
                 if not self._nonblock_event.isSet():
                     self._nonblock_event.set()
+        return True
 
     def _dispatcher(self, started_event):
         started_event.set() # Let the master know we are alive already
@@ -250,7 +253,9 @@ class NotifyOut:
         self._thread.join()
 
         # Clean up
+        self._write_sock.close()
         self._write_sock = None
+        self._read_sock.close()
         self._read_sock = None
         self._thread = None
 

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

@@ -114,38 +114,48 @@ class TestNotifyOut(unittest.TestCase):
         notify_out._MAX_NOTIFY_NUM = 2
 
         self._notify._nonblock_event.clear()
-        self._notify.send_notify('example.net')
+        self.assertTrue(self._notify.send_notify('example.net'))
         self.assertTrue(self._notify._nonblock_event.isSet())
         self.assertEqual(self._notify.notify_num, 1)
         self.assertEqual(self._notify._notifying_zones[0], ('example.net.', 'IN'))
 
-        self._notify.send_notify('example.com')
+        self.assertTrue(self._notify.send_notify('example.com'))
         self.assertEqual(self._notify.notify_num, 2)
         self.assertEqual(self._notify._notifying_zones[1], ('example.com.', 'IN'))
 
         # notify_num is equal to MAX_NOTIFY_NUM, append it to waiting_zones list.
         self._notify._nonblock_event.clear()
-        self._notify.send_notify('example.com', 'CH')
+        self.assertTrue(self._notify.send_notify('example.com', 'CH'))
         # add waiting zones won't set nonblock_event.
         self.assertFalse(self._notify._nonblock_event.isSet())
         self.assertEqual(self._notify.notify_num, 2)
         self.assertEqual(1, len(self._notify._waiting_zones))
 
         # zone_id is already in notifying_zones list, append it to waiting_zones list.
-        self._notify.send_notify('example.net')
+        self.assertTrue(self._notify.send_notify('example.net'))
         self.assertEqual(2, len(self._notify._waiting_zones))
         self.assertEqual(self._notify._waiting_zones[1], ('example.net.', 'IN'))
 
         # zone_id is already in waiting_zones list, skip it.
-        self._notify.send_notify('example.net')
+        self.assertTrue(self._notify.send_notify('example.net'))
         self.assertEqual(2, len(self._notify._waiting_zones))
 
         # has no slave masters, skip it.
-        self._notify.send_notify('example.org.', 'CH')
+        self.assertTrue(self._notify.send_notify('example.org.', 'CH'))
         self.assertEqual(self._notify.notify_num, 2)
         self.assertEqual(2, len(self._notify._waiting_zones))
 
-        self._notify.send_notify('example.org.')
+        self.assertTrue(self._notify.send_notify('example.org.'))
+        self.assertEqual(self._notify.notify_num, 2)
+        self.assertEqual(2, len(self._notify._waiting_zones))
+
+        # zone does not exist, should return False, and no change in other
+        # values
+        self.assertFalse(self._notify.send_notify('does.not.exist.'))
+        self.assertEqual(self._notify.notify_num, 2)
+        self.assertEqual(2, len(self._notify._waiting_zones))
+
+        self.assertFalse(self._notify.send_notify('example.net.', 'CH'))
         self.assertEqual(self._notify.notify_num, 2)
         self.assertEqual(2, len(self._notify._waiting_zones))
 
@@ -185,6 +195,11 @@ class TestNotifyOut(unittest.TestCase):
         # Now make one socket be readable
         self._notify._notify_infos[('example.net.', 'IN')].notify_timeout = time.time() + 10
         self._notify._notify_infos[('example.com.', 'IN')].notify_timeout = time.time() + 10
+
+        if self._notify._read_sock is not None:
+            self._notify._read_sock.close()
+        if self._notify._write_sock is not None:
+            self._notify._write_sock.close()
         self._notify._read_sock, self._notify._write_sock = socket.socketpair()
         self._notify._write_sock.send(SOCK_DATA)
         replied_zones, timeout_zones = self._notify._wait_for_notify_reply()

src/lib/util/buffer.h

@@ -122,10 +122,10 @@ public:
     /// an exception of class \c isc::dns::InvalidBufferPosition will be thrown.
     /// \param position The new position (offset from the beginning of the
     /// buffer).
-    void setPosition(size_t position)
-    {
-        if (position > len_)
-            isc_throw(InvalidBufferPosition, "position is too large");
+    void setPosition(size_t position) {
+        if (position > len_) {
+            throwError("position is too large");
+        }
         position_ = position;
     }
     //@}
@@ -137,10 +137,9 @@ public:
     ///
     /// If the remaining length of the buffer is smaller than 8-bit, an
     /// exception of class \c isc::dns::InvalidBufferPosition will be thrown.
-    uint8_t readUint8()
-    {
+    uint8_t readUint8() {
         if (position_ + sizeof(uint8_t) > len_) {
-            isc_throw(InvalidBufferPosition, "read beyond end of buffer");
+            throwError("read beyond end of buffer");
         }
 
         return (data_[position_++]);
@@ -150,13 +149,12 @@ public:
     ///
     /// If the remaining length of the buffer is smaller than 16-bit, an
     /// exception of class \c isc::dns::InvalidBufferPosition will be thrown.
-    uint16_t readUint16()
-    {
+    uint16_t readUint16() {
         uint16_t data;
         const uint8_t* cp;
 
         if (position_ + sizeof(data) > len_) {
-            isc_throw(InvalidBufferPosition, "read beyond end of buffer");
+            throwError("read beyond end of buffer");
         }
 
         cp = &data_[position_];
@@ -171,13 +169,12 @@ public:
     ///
     /// If the remaining length of the buffer is smaller than 32-bit, an
     /// exception of class \c isc::dns::InvalidBufferPosition will be thrown.
-    uint32_t readUint32()
-    {
+    uint32_t readUint32() {
         uint32_t data;
         const uint8_t* cp;
 
         if (position_ + sizeof(data) > len_) {
-            isc_throw(InvalidBufferPosition, "read beyond end of buffer");
+            throwError("read beyond end of buffer");
         }
 
         cp = &data_[position_];
@@ -196,10 +193,9 @@ public:
     /// If the remaining length of the buffer is smaller than the specified
     /// length, an exception of class \c isc::dns::InvalidBufferPosition will
     /// be thrown.
-    void readData(void* data, size_t len)
-    {
+    void readData(void* data, size_t len) {
         if (position_ + len > len_) {
-            isc_throw(InvalidBufferPosition, "read beyond end of buffer");
+            throwError("read beyond end of buffer");
         }
 
         memcpy(data, &data_[position_], len);
@@ -215,10 +211,9 @@ public:
     /// @param Reference to a buffer (data will be stored there).
     /// @param Size specified number of bytes to read in a vector.
     ///
-    void readVector(std::vector<uint8_t>& data, size_t len)
-    {
+    void readVector(std::vector<uint8_t>& data, size_t len) {
         if (position_ + len > len_) {
-            isc_throw(InvalidBufferPosition, "read beyond end of buffer");
+            throwError("read beyond end of buffer");
         }
 
         data.resize(len);
@@ -226,6 +221,15 @@ public:
     }
 
 private:
+    /// \brief A common helper to throw an exception on invalid operation.
+    ///
+    /// Experiments showed that throwing from each method makes the buffer
+    /// operation slower, so we consolidate it here, and let the methods
+    /// call this.
+    static void throwError(const char* msg) {
+        isc_throw(InvalidBufferPosition, msg);
+    }
+
     size_t position_;
 
     // XXX: The following must be private, but for a short term workaround with

src/lib/util/python/gen_wiredata.py.in

@@ -822,6 +822,29 @@ class RP(RR):
         f.write('# MAILBOX=%s TEXT=%s\n' % (self.mailbox, self.text))
         f.write('%s %s\n' % (mailbox_wire, text_wire))
 
+class SSHFP(RR):
+    '''Implements rendering SSHFP RDATA in the test data format.
+
+    Configurable parameters are as follows (see the description of the
+    same name of attribute for the default value):
+    - algorithm (int): The algorithm number.
+    - fingerprint_type (int): The fingerprint type.
+    - fingerprint (string): The fingerprint.
+    '''
+    algorithm = 2
+    fingerprint_type = 1
+    fingerprint = '123456789abcdef67890123456789abcdef67890'
+    def dump(self, f):
+        if self.rdlen is None:
+            self.rdlen = 2 + (len(self.fingerprint) / 2)
+        else:
+            self.rdlen = int(self.rdlen)
+        self.dump_header(f, self.rdlen)
+        f.write('# ALGORITHM=%d FINGERPRINT_TYPE=%d FINGERPRINT=%s\n' % (self.algorithm,
+                                                                         self.fingerprint_type,
+                                                                         self.fingerprint))
+        f.write('%02x %02x %s\n' % (self.algorithm, self.fingerprint_type, self.fingerprint))
+
 class MINFO(RR):
     '''Implements rendering MINFO RDATA in the test data format.