BIND 10 trac2213, updated. 0192fd73649d14ade8dc284c64f38d5e0d8b0705 [2213] more review comments

[BIND 10 source code commits]

The branch, trac2213 has been updated
       via  0192fd73649d14ade8dc284c64f38d5e0d8b0705 (commit)
      from  08baa5371d0939356ba56f24f2d537834f7e9bac (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 0192fd73649d14ade8dc284c64f38d5e0d8b0705
Author: Jelte Jansen <jelte at isc.org>
Date:   Wed Oct 31 10:59:00 2012 +0100

    [2213] more review comments
    
    bit of code cleanup around ifs, removed now obsolete log message

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

Summary of changes:
 src/bin/auth/auth_messages.mes     |    5 -----
 src/bin/auth/datasrc_clients_mgr.h |   35 +++++++++++++++++++----------------
 2 files changed, 19 insertions(+), 21 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/auth/auth_messages.mes b/src/bin/auth/auth_messages.mes
index 163b6ba..221d122 100644
--- a/src/bin/auth/auth_messages.mes
+++ b/src/bin/auth/auth_messages.mes
@@ -178,11 +178,6 @@ has requested the keyring holding TSIG keys from the configuration
 database. It is issued during server startup is an indication that the
 initialization is proceeding normally.
 
-% AUTH_LOAD_ZONE loaded zone %1/%2
-This debug message is issued during the processing of the 'loadzone' command
-when the authoritative server has successfully loaded the named zone of the
-named class.
-
 % AUTH_MEM_DATASRC_DISABLED memory data source is disabled for class %1
 This is a debug message reporting that the authoritative server has
 discovered that the memory data source is disabled for the given class.
diff --git a/src/bin/auth/datasrc_clients_mgr.h b/src/bin/auth/datasrc_clients_mgr.h
index 218382f..ab572c3 100644
--- a/src/bin/auth/datasrc_clients_mgr.h
+++ b/src/bin/auth/datasrc_clients_mgr.h
@@ -272,13 +272,12 @@ public:
         if (!args->contains("origin")) {
             isc_throw(CommandError,
                       "loadZone argument has no 'origin' value");
-        } else {
-            // Also check if it really is a valid name
-            try {
-                dns::Name(args->get("origin")->stringValue());
-            } catch (const isc::Exception& exc) {
-                isc_throw(CommandError, "bad origin: " << exc.what());
-            }
+        }
+        // Also check if it really is a valid name
+        try {
+            dns::Name(args->get("origin")->stringValue());
+        } catch (const isc::Exception& exc) {
+            isc_throw(CommandError, "bad origin: " << exc.what());
         }
 
         if (args->get("origin")->getType() != data::Element::string) {
@@ -289,16 +288,20 @@ public:
             if (args->get("class")->getType() != data::Element::string) {
                 isc_throw(CommandError,
                           "loadZone argument 'class' value not a string");
-            } else {
-                try {
-                    dns::RRClass(args->get("class")->stringValue());
-                } catch (const isc::Exception& exc) {
-                    isc_throw(CommandError, "bad class: " << exc.what());
-                }
+            }
+            // Also check if it is a valid class
+            try {
+                dns::RRClass(args->get("class")->stringValue());
+            } catch (const isc::Exception& exc) {
+                isc_throw(CommandError, "bad class: " << exc.what());
             }
         }
 
-        // Slightly more advanced checks
+        // Note: we could do some more advanced checks here,
+        // e.g. check if the zone is known at all in the configuration.
+        // For now these are skipped, but one obvious way to
+        // implement it would be to factor out the code from
+        // the start of doLoadZone(), and call it here too
 
         sendCommand(datasrc_clientmgr_internal::LOADZONE, args);
     }
@@ -557,8 +560,8 @@ DataSrcClientsBuilderBase<MutexType, CondVarType>::doLoadZone(
 
     // TODO: currently, we hardcode IN as the default for the optional
     // 'class' argument. We should really derive this from the specification,
-    // but atm the config/command API does not allow that to be done
-    // easily. Once that is in place (tickets have yet to be created,
+    // but at the moment the config/command API does not allow that to be
+    // done easily. Once that is in place (tickets have yet to be created,
     // as we need to do a tiny bit of design work for that), this
     // code can be replaced with the original part:
     // assert(arg->get("class"));