[svn] commit: r3987 - in /branches/trac447/src/lib/datasrc: memory_datasrc.cc tests/memory_datasrc_unittest.cc

[BIND 10 source code commits]

Author: vorner
Date: Thu Dec 23 12:46:54 2010
New Revision: 3987

Log:
Update according to review comments

* Removed delegation code, as it is wrong and needs to be addressed in a
  more sophisticated way.
* Make MemoryZone::add strong exception safe at the interface level.

Modified:
    branches/trac447/src/lib/datasrc/memory_datasrc.cc
    branches/trac447/src/lib/datasrc/tests/memory_datasrc_unittest.cc

Modified: branches/trac447/src/lib/datasrc/memory_datasrc.cc
==============================================================================
--- branches/trac447/src/lib/datasrc/memory_datasrc.cc (original)
+++ branches/trac447/src/lib/datasrc/memory_datasrc.cc Thu Dec 23 12:46:54 2010
@@ -114,6 +114,13 @@
         if (node->isEmpty()) {
             domain.reset(new Domain);
             node->setData(domain);
+            /*
+             * If something throws exception past this point, we did change
+             * the internal structure. But as an empty domain is taken as
+             * nonexistant one, it is strongly exception safe at the interface
+             * level even when the internal representation of the state
+             * changes.
+             */
         } else { // Get existing one
             domain = node->getData();
         }
@@ -133,16 +140,12 @@
         try {
             // Get the relative name
             Name relative_name(getRelativeName(full_name));
-            // What we return when we succeed.
-            bool delegation(false);
             // Get the node
             DomainNode* node;
             switch (domains_.find(relative_name, &node)) {
                 case DomainTree::PARTIALMATCH:
-                    // We change to looking for a NS record in the node
-                    delegation = true;
-                    type = RRType::NS();
-                    break; // And handle it the usual way
+                    // Pretend it was not found for now
+                    // TODO: Implement real delegation
                 case DomainTree::NOTFOUND:
                     return (FindResult(NXDOMAIN, ConstRRsetPtr()));
                 case DomainTree::EXACTMATCH: // This one is OK, handle it
@@ -156,17 +159,20 @@
             Domain::const_iterator found(node->getData()->find(type));
             if (found != node->getData()->end()) {
                 // Good, it is here
-                // Return either SUCCESS or DELEGATION
-                return (FindResult(delegation ? DELEGATION : SUCCESS,
-                    found->second));
+                return (FindResult(SUCCESS, found->second));
             } else {
                 /*
                  * TODO Look for CNAME and DNAME (it should be OK to do so when
                  * the value is not found, as CNAME/DNAME domain should be
                  * empty otherwise.)
                  */
-                if (delegation) {
-                    // If we were trying a delegation, then we have NXDOMAIN
+                if(node->getData()->empty()) {
+                    /*
+                     * In case the domain is empty, it is the same as if it
+                     * does not exist. It probably got here by unsuccessful
+                     * add, so just remove it, it is useless.
+                     */
+                    node->setData(DomainPtr());
                     return (FindResult(NXDOMAIN, ConstRRsetPtr()));
                 } else {
                     return (FindResult(NXRRSET, ConstRRsetPtr()));

Modified: branches/trac447/src/lib/datasrc/tests/memory_datasrc_unittest.cc
==============================================================================
--- branches/trac447/src/lib/datasrc/tests/memory_datasrc_unittest.cc (original)
+++ branches/trac447/src/lib/datasrc/tests/memory_datasrc_unittest.cc Thu Dec 23 12:46:54 2010
@@ -126,9 +126,7 @@
         rr_ns_(new RRset(origin_, class_, RRType::NS(), RRTTL(300))),
         rr_ns_a_(new RRset(ns_name_, class_, RRType::A(), RRTTL(300))),
         rr_ns_aaaa_(new RRset(ns_name_, class_, RRType::AAAA(), RRTTL(300))),
-        rr_a_(new RRset(origin_, class_, RRType::A(), RRTTL(300))),
-        rr_delegation_(new RRset(Name("subdomain.example.org"), class_,
-            RRType::NS(), RRTTL(300)))
+        rr_a_(new RRset(origin_, class_, RRType::A(), RRTTL(300)))
     {
     }
     // Some data to test with
@@ -153,9 +151,7 @@
         // AAAA of ns.example.org
         rr_ns_aaaa_,
         // A of example.org
-        rr_a_,
-        // A subdomain.example.org NS, for delegation
-        rr_delegation_;
+        rr_a_;
 
     /**
      * \brief Test one find query to the zone.
@@ -233,7 +229,6 @@
     EXPECT_NO_THROW(EXPECT_EQ(SUCCESS, zone_.add(rr_ns_a_)));
     EXPECT_NO_THROW(EXPECT_EQ(SUCCESS, zone_.add(rr_ns_aaaa_)));
     EXPECT_NO_THROW(EXPECT_EQ(SUCCESS, zone_.add(rr_a_)));
-    EXPECT_NO_THROW(EXPECT_EQ(SUCCESS, zone_.add(rr_delegation_)));
 
     // These two should be successful
     findTest(origin_, RRType::NS(), Zone::SUCCESS, rr_ns_);
@@ -246,10 +241,6 @@
     // These domains don't exist (and one is out of the zone)
     findTest(Name("nothere.example.org"), RRType::A(), Zone::NXDOMAIN);
     findTest(Name("example.net"), RRType::A(), Zone::NXDOMAIN);
-
-    // This should delegate
-    findTest(Name("a.subdomain.example.org"), RRType::MX(), Zone::DELEGATION,
-        rr_delegation_);
-}
-
-}
+}
+
+}