BIND 10 trac1976-cont-3, updated. 9c128152d15049af019ad58e20be23810b518141 [1976] Comment refinements

BIND 10 source code commits bind10-changes at lists.isc.org
Thu Jul 19 10:39:49 UTC 2012


The branch, trac1976-cont-3 has been updated
       via  9c128152d15049af019ad58e20be23810b518141 (commit)
       via  4a972b4645aa694071027e0685e61a6e993cac4a (commit)
      from  64d096e98b2f1803e0e5f5f36986a59c4cd90dff (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 9c128152d15049af019ad58e20be23810b518141
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Thu Jul 19 12:07:26 2012 +0200

    [1976] Comment refinements

commit 4a972b4645aa694071027e0685e61a6e993cac4a
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Thu Jul 19 12:04:13 2012 +0200

    [1976] Simplify the loadzone command handler
    
    As most of the handling moved to the ConfigurableClientList, it got so
    simple the separate validate() method needed more care to pass values
    around than the work it actually did, so it got inlined.

-----------------------------------------------------------------------

Summary of changes:
 src/bin/auth/command.cc                       |   64 ++++++++-----------------
 src/lib/datasrc/client_list.cc                |    3 --
 src/lib/datasrc/client_list.h                 |    4 ++
 src/lib/datasrc/tests/client_list_unittest.cc |    7 ++-
 4 files changed, 30 insertions(+), 48 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/auth/command.cc b/src/bin/auth/command.cc
index 3822345..f0629e4 100644
--- a/src/bin/auth/command.cc
+++ b/src/bin/auth/command.cc
@@ -143,37 +143,42 @@ public:
 // Handle the "loadzone" command.
 class LoadZoneCommand : public AuthCommand {
 public:
-    LoadZoneCommand() :
-        zone_class_(RRClass::IN()), // We need to have something to compile
-        origin_(Name::ROOT_NAME())
-    {}
     virtual void exec(AuthSrv& server, isc::data::ConstElementPtr args) {
-        // parse and validate the args.
-        if (!validate(args)) {
-            return;
+        if (args == NULL) {
+            isc_throw(AuthCommandError, "Null argument");
+        }
+
+        ConstElementPtr class_elem = args->get("class");
+        RRClass zone_class(class_elem ? RRClass(class_elem->stringValue()) :
+            RRClass::IN());
+
+        ConstElementPtr origin_elem = args->get("origin");
+        if (!origin_elem) {
+            isc_throw(AuthCommandError, "Zone origin is missing");
         }
+        Name origin(origin_elem->stringValue());
 
         const boost::shared_ptr<isc::datasrc::ConfigurableClientList>
-            list(server.getClientList(zone_class_));
+            list(server.getClientList(zone_class));
 
         if (!list) {
             isc_throw(AuthCommandError, "There's no client list for "
-                      "class " << zone_class_);
+                      "class " << zone_class);
         }
 
-        switch (list->reload(origin_)) {
+        switch (list->reload(origin)) {
             case ConfigurableClientList::ZONE_RELOADED:
                 // Everything worked fine.
                 LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_LOAD_ZONE)
-                    .arg(zone_class_).arg(origin_);
+                    .arg(zone_class).arg(origin);
                 return;
             case ConfigurableClientList::ZONE_NOT_FOUND:
-                isc_throw(AuthCommandError, "Zone " << origin_ << "/" <<
-                          zone_class_ << " was not found in any configured "
+                isc_throw(AuthCommandError, "Zone " << origin << "/" <<
+                          zone_class << " was not found in any configured "
                           "data source. Configure it first.");
             case ConfigurableClientList::ZONE_NOT_CACHED:
-                isc_throw(AuthCommandError, "Zone " << origin_ << "/" <<
-                          zone_class_ << " is not served from memory, but "
+                isc_throw(AuthCommandError, "Zone " << origin << "/" <<
+                          zone_class << " is not served from memory, but "
                           "direcly from the data source. It is not possible "
                           "to reload it into memory. Configure it to be cached "
                           "first.");
@@ -181,36 +186,9 @@ public:
                 // This is an internal error. Auth server must have the cache
                 // enabled.
                 isc_throw(isc::Unexpected, "Cache disabled in client list of "
-                          "class " << zone_class_);
+                          "class " << zone_class);
         }
     }
-
-private:
-    // Parsed arguments
-    RRClass zone_class_;
-    Name origin_;
-
-    // A helper private method to parse and validate command parameters.
-    // On success, it sets 'old_zone_finder_' to the zone to be updated.
-    // It returns true if everything is okay; and false if the command is
-    // valid but there's no need for further process.
-    bool validate(isc::data::ConstElementPtr args) {
-        if (args == NULL) {
-            isc_throw(AuthCommandError, "Null argument");
-        }
-
-        ConstElementPtr class_elem = args->get("class");
-        zone_class_ = class_elem ? RRClass(class_elem->stringValue()) :
-            RRClass::IN();
-
-        ConstElementPtr origin_elem = args->get("origin");
-        if (!origin_elem) {
-            isc_throw(AuthCommandError, "Zone origin is missing");
-        }
-        origin_ = Name(origin_elem->stringValue());
-
-        return (true);
-    }
 };
 
 // The factory of command objects.
diff --git a/src/lib/datasrc/client_list.cc b/src/lib/datasrc/client_list.cc
index bdb481e..59ed61b 100644
--- a/src/lib/datasrc/client_list.cc
+++ b/src/lib/datasrc/client_list.cc
@@ -254,9 +254,6 @@ ConfigurableClientList::findInternal(MutableResult& candidate,
 
     // TODO: In case we have only the datasource and not the finder
     // and the need_updater parameter is true, get the zone there.
-
-    // Return the partial match we have. In case we didn't want a partial
-    // match, this surely contains the original empty result.
 }
 
 ConfigurableClientList::ReloadResult
diff --git a/src/lib/datasrc/client_list.h b/src/lib/datasrc/client_list.h
index fc266c6..dde391d 100644
--- a/src/lib/datasrc/client_list.h
+++ b/src/lib/datasrc/client_list.h
@@ -331,6 +331,10 @@ private:
     ///
     /// The result is returned as parameter because MutableResult is not
     /// defined in the header file.
+    ///
+    /// If there's no match, the result is not modified. Therefore, this
+    /// expects to get a fresh result object each time it is called, not
+    /// to reuse it.
     void findInternal(MutableResult& result, const dns::Name& name,
                       bool want_exact_match, bool want_finder) const;
     const isc::dns::RRClass rrclass_;
diff --git a/src/lib/datasrc/tests/client_list_unittest.cc b/src/lib/datasrc/tests/client_list_unittest.cc
index b77d089..69bd586 100644
--- a/src/lib/datasrc/tests/client_list_unittest.cc
+++ b/src/lib/datasrc/tests/client_list_unittest.cc
@@ -255,7 +255,8 @@ public:
                                               0, 0, 0, 0, 0));
             finder->add(soa);
         }
-        // We leave the zone empty, so we can check it was reloaded.
+        // If we don't do prefill, we leave the zone empty. This way,
+        // we can check when it was reloaded.
         cache->addZone(finder);
         list_->getDataSources()[index].cache_ = cache;
     }
@@ -811,7 +812,9 @@ TEST_F(ListTest, reloadNotEnabled) {
 TEST_F(ListTest, reloadNoSuchZone) {
     list_->configure(config_elem_zones_, true);
     Name name("example.org");
-    // We put the cache in even when not enabled. This won't confuse the thing.
+    // We put the cache in even when not enabled. This won't confuse the
+    // reload method, as that one looks at the real state of things, not
+    // at the configuration.
     prepareCache(0, Name("example.com"));
     // Not in the data sources
     EXPECT_EQ(ConfigurableClientList::ZONE_NOT_FOUND,



More information about the bind10-changes mailing list