Browse Source

[2213] more review comments

bit of code cleanup around ifs, removed now obsolete log message
Jelte Jansen 12 years ago
parent
commit
0192fd7364
2 changed files with 19 additions and 21 deletions
  1. 0 5
      src/bin/auth/auth_messages.mes
  2. 19 16
      src/bin/auth/datasrc_clients_mgr.h

+ 0 - 5
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
 database. It is issued during server startup is an indication that the
 initialization is proceeding normally.
 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
 % AUTH_MEM_DATASRC_DISABLED memory data source is disabled for class %1
 This is a debug message reporting that the authoritative server has
 This is a debug message reporting that the authoritative server has
 discovered that the memory data source is disabled for the given class.
 discovered that the memory data source is disabled for the given class.

+ 19 - 16
src/bin/auth/datasrc_clients_mgr.h

@@ -272,13 +272,12 @@ public:
         if (!args->contains("origin")) {
         if (!args->contains("origin")) {
             isc_throw(CommandError,
             isc_throw(CommandError,
                       "loadZone argument has no 'origin' value");
                       "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) {
         if (args->get("origin")->getType() != data::Element::string) {
@@ -289,16 +288,20 @@ public:
             if (args->get("class")->getType() != data::Element::string) {
             if (args->get("class")->getType() != data::Element::string) {
                 isc_throw(CommandError,
                 isc_throw(CommandError,
                           "loadZone argument 'class' value not a string");
                           "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);
         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
     // TODO: currently, we hardcode IN as the default for the optional
     // 'class' argument. We should really derive this from the specification,
     // '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
     // as we need to do a tiny bit of design work for that), this
     // code can be replaced with the original part:
     // code can be replaced with the original part:
     // assert(arg->get("class"));
     // assert(arg->get("class"));