Browse Source

[1793] Eliminate several state variables

The values are deduced from something else, no need to keep them. This
eliminates the ugly initialization with wrong values.
JINMEI Tatuya 13 years ago
parent
commit
264691395b
1 changed files with 25 additions and 41 deletions
  1. 25 41
      src/bin/auth/command.cc

+ 25 - 41
src/bin/auth/command.cc

@@ -144,58 +144,45 @@ public:
 // Handle the "loadzone" command.
 // Handle the "loadzone" command.
 class LoadZoneCommand : public AuthCommand {
 class LoadZoneCommand : public AuthCommand {
 public:
 public:
-    LoadZoneCommand() :
-        // Just fill in something, these can't be created "without data",
-        // by constructor without parameters. These values will be overwritten
-        // in validate().
-        origin_(Name::ROOT_NAME()),
-        zone_class_(RRClass::IN())
-    { }
     virtual void exec(AuthSrv& server, isc::data::ConstElementPtr args) {
     virtual void exec(AuthSrv& server, isc::data::ConstElementPtr args) {
         // parse and validate the args.
         // parse and validate the args.
         if (!validate(server, args)) {
         if (!validate(server, args)) {
             return;
             return;
         }
         }
 
 
-        getZoneConfig(server);
+        const ConstElementPtr zone_config = getZoneConfig(server);
 
 
         // Load a new zone and replace the current zone with the new one.
         // Load a new zone and replace the current zone with the new one.
         // TODO: eventually this should be incremental or done in some way
         // TODO: eventually this should be incremental or done in some way
         // that doesn't block other server operations.
         // that doesn't block other server operations.
         // TODO: we may (should?) want to check the "last load time" and
         // TODO: we may (should?) want to check the "last load time" and
         // the timestamp of the file and skip loading if the file isn't newer.
         // the timestamp of the file and skip loading if the file isn't newer.
-        const ConstElementPtr type(zone_config_->get("filetype"));
+        const ConstElementPtr type(zone_config->get("filetype"));
         boost::shared_ptr<InMemoryZoneFinder> zone_finder(
         boost::shared_ptr<InMemoryZoneFinder> zone_finder(
-            new InMemoryZoneFinder(old_zone_finder->getClass(),
-                                   old_zone_finder->getOrigin()));
+            new InMemoryZoneFinder(old_zone_finder_->getClass(),
+                                   old_zone_finder_->getOrigin()));
         if (type && type->stringValue() == "sqlite3") {
         if (type && type->stringValue() == "sqlite3") {
             scoped_ptr<DataSourceClientContainer>
             scoped_ptr<DataSourceClientContainer>
                 container(new DataSourceClientContainer("sqlite3",
                 container(new DataSourceClientContainer("sqlite3",
                                                         Element::fromJSON(
                                                         Element::fromJSON(
                     "{\"database_file\": \"" +
                     "{\"database_file\": \"" +
-                    zone_config_->get("file")->stringValue() + "\"}")));
+                    zone_config->get("file")->stringValue() + "\"}")));
             zone_finder->load(*container->getInstance().getIterator(
             zone_finder->load(*container->getInstance().getIterator(
-                old_zone_finder->getOrigin()));
+                old_zone_finder_->getOrigin()));
         } else {
         } else {
-            zone_finder->load(old_zone_finder->getFileName());
+            zone_finder->load(old_zone_finder_->getFileName());
         }
         }
-        old_zone_finder->swap(*zone_finder);
+        old_zone_finder_->swap(*zone_finder);
         LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_LOAD_ZONE)
         LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_LOAD_ZONE)
                   .arg(zone_finder->getOrigin()).arg(zone_finder->getClass());
                   .arg(zone_finder->getOrigin()).arg(zone_finder->getClass());
     }
     }
 
 
 private:
 private:
     // zone finder to be updated with the new file.
     // zone finder to be updated with the new file.
-    boost::shared_ptr<InMemoryZoneFinder> old_zone_finder;
-    // The configuration corresponding to the zone.
-    ConstElementPtr zone_config_;
-
-    // Parameters of the zone
-    Name origin_;
-    RRClass zone_class_;
+    boost::shared_ptr<InMemoryZoneFinder> old_zone_finder_;
 
 
     // A helper private method to parse and validate command parameters.
     // A helper private method to parse and validate command parameters.
-    // On success, it sets 'old_zone_finder' to the zone to be updated.
+    // 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
     // It returns true if everything is okay; and false if the command is
     // valid but there's no need for further process.
     // valid but there's no need for further process.
     bool validate(AuthSrv& server, isc::data::ConstElementPtr args) {
     bool validate(AuthSrv& server, isc::data::ConstElementPtr args) {
@@ -220,11 +207,11 @@ private:
         }
         }
 
 
         ConstElementPtr class_elem = args->get("class");
         ConstElementPtr class_elem = args->get("class");
-        zone_class_ = class_elem ? RRClass(class_elem->stringValue()) :
-            RRClass::IN();
+        const RRClass zone_class =
+            class_elem ? RRClass(class_elem->stringValue()) : RRClass::IN();
 
 
         AuthSrv::InMemoryClientPtr datasrc(server.
         AuthSrv::InMemoryClientPtr datasrc(server.
-                                           getInMemoryClient(zone_class_));
+                                           getInMemoryClient(zone_class));
         if (datasrc == NULL) {
         if (datasrc == NULL) {
             isc_throw(AuthCommandError, "Memory data source is disabled");
             isc_throw(AuthCommandError, "Memory data source is disabled");
         }
         }
@@ -233,22 +220,22 @@ private:
         if (!origin_elem) {
         if (!origin_elem) {
             isc_throw(AuthCommandError, "Zone origin is missing");
             isc_throw(AuthCommandError, "Zone origin is missing");
         }
         }
-        origin_ = Name(origin_elem->stringValue());
+        const Name origin = Name(origin_elem->stringValue());
 
 
         // Get the current zone
         // Get the current zone
-        const InMemoryClient::FindResult result = datasrc->findZone(origin_);
+        const InMemoryClient::FindResult result = datasrc->findZone(origin);
         if (result.code != result::SUCCESS) {
         if (result.code != result::SUCCESS) {
-            isc_throw(AuthCommandError, "Zone " << origin_ <<
+            isc_throw(AuthCommandError, "Zone " << origin <<
                       " is not found in data source");
                       " is not found in data source");
         }
         }
 
 
-        old_zone_finder = boost::dynamic_pointer_cast<InMemoryZoneFinder>(
+        old_zone_finder_ = boost::dynamic_pointer_cast<InMemoryZoneFinder>(
             result.zone_finder);
             result.zone_finder);
 
 
         return (true);
         return (true);
     }
     }
 
 
-    void getZoneConfig(const AuthSrv &server) {
+    ConstElementPtr getZoneConfig(const AuthSrv &server) {
         if (!server.getConfigSession()) {
         if (!server.getConfigSession()) {
             // FIXME: This is a hack to make older tests pass. We should
             // FIXME: This is a hack to make older tests pass. We should
             // update these tests as well sometime and remove this hack.
             // update these tests as well sometime and remove this hack.
@@ -257,8 +244,7 @@ private:
 
 
             // We provide an empty map, which means no configuration --
             // We provide an empty map, which means no configuration --
             // defaults.
             // defaults.
-            zone_config_ = ConstElementPtr(new MapElement());
-            return;
+            return (ConstElementPtr(new MapElement()));
         }
         }
 
 
         // Find the config corresponding to the zone.
         // Find the config corresponding to the zone.
@@ -282,7 +268,7 @@ private:
             // anyway and we may want to change the configuration of
             // anyway and we may want to change the configuration of
             // datasources somehow.
             // datasources somehow.
             if (dsrc_config->get("type")->stringValue() == "memory" &&
             if (dsrc_config->get("type")->stringValue() == "memory" &&
-                RRClass(class_type) == zone_class_) {
+                RRClass(class_type) == old_zone_finder_->getClass()) {
                 zone_list = dsrc_config->get("zones");
                 zone_list = dsrc_config->get("zones");
                 break;
                 break;
             }
             }
@@ -296,18 +282,16 @@ private:
         // Now we need to walk the zones and find the correct one.
         // Now we need to walk the zones and find the correct one.
         for (size_t i(0); i < zone_list->size(); ++i) {
         for (size_t i(0); i < zone_list->size(); ++i) {
             const ConstElementPtr zone_config(zone_list->get(i));
             const ConstElementPtr zone_config(zone_list->get(i));
-            if (Name(zone_config->get("origin")->stringValue()) == origin_) {
+            if (Name(zone_config->get("origin")->stringValue()) ==
+                old_zone_finder_->getOrigin()) {
                 // The origins are the same, so we consider this config to be
                 // The origins are the same, so we consider this config to be
                 // for the zone.
                 // for the zone.
-                zone_config_ = zone_config;
-                break;
+                return (zone_config);
             }
             }
         }
         }
 
 
-        if (!zone_config_) {
-            isc_throw(AuthCommandError,
-                      "Corresponding zone configuration was not found");
-        }
+        isc_throw(AuthCommandError,
+                  "Corresponding zone configuration was not found");
     }
     }
 };
 };