From: Jelte Jansen <jelte@isc.org>
Date: Wed, 31 Oct 2012 09:59:00 +0000 (+0100)
Subject: [2213] more review comments
X-Git-Tag: trac2487_base~23^2~1
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=0192fd73649d14ade8dc284c64f38d5e0d8b0705;p=thirdparty%2Fkea.git

[2213] more review comments

bit of code cleanup around ifs, removed now obsolete log message
---

diff --git a/src/bin/auth/auth_messages.mes b/src/bin/auth/auth_messages.mes
index 163b6ba4ad..221d1221eb 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 218382fd62..ab572c381e 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"));