From: Jelte Jansen <jelte@isc.org>
Date: Wed, 16 May 2012 13:40:00 +0000 (+0200)
Subject: [1207] more review comments addressed
X-Git-Tag: trac2351_base~226^2~90^2~11
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=d19d162c3e89784b4e013e6892e7274c4bc0f861;p=thirdparty%2Fkea.git

[1207] more review comments addressed

- RAII InMemoryClient's createInstance
- remove dead code
- added more comments
---

diff --git a/src/bin/auth/tests/command_unittest.cc b/src/bin/auth/tests/command_unittest.cc
index 24c7dc3495..c4a81a1369 100644
--- a/src/bin/auth/tests/command_unittest.cc
+++ b/src/bin/auth/tests/command_unittest.cc
@@ -300,7 +300,7 @@ TEST_F(AuthCommandTest,
             Element::fromJSON("{\"type\": \"memory\"}")));
     // The 'public' factory API does not allow for direct internal calls such
     // as addZone, so purely for this test we do a quick cast
-    static_cast<InMemoryClient*>(&dsrc->getInstance())->addZone(
+    static_cast<InMemoryClient&>(dsrc->getInstance()).addZone(
         ZoneFinderPtr(new InMemoryZoneFinder(RRClass::IN(),
                                              Name("example.org"))));
     server_.setInMemoryClient(RRClass::IN(), dsrc);
@@ -320,7 +320,7 @@ TEST_F(AuthCommandTest,
     // Some error cases. First, the zone has no configuration.
     // The 'public' factory API does not allow for direct internal calls such
     // as addZone, so purely for this test we do a quick cast
-    static_cast<InMemoryClient*>(&dsrc->getInstance())->addZone(
+    static_cast<InMemoryClient&>(dsrc->getInstance()).addZone(
         ZoneFinderPtr(new InMemoryZoneFinder(RRClass::IN(),
                                              Name("example.com"))));
     result_ = execAuthServerCommand(server_, "loadzone",
diff --git a/src/lib/datasrc/client.h b/src/lib/datasrc/client.h
index d70c7c6f27..dab081f38d 100644
--- a/src/lib/datasrc/client.h
+++ b/src/lib/datasrc/client.h
@@ -371,6 +371,8 @@ public:
     /// \exception NotImplemented Thrown if this method is not supported
     ///            by the datasource
     ///
+    /// \note This is a tentative API, and this method may likely to be
+    ///       removed in the near future.
     /// \return The number of zones known to this datasource
     virtual unsigned int getZoneCount() const {
         isc_throw(isc::NotImplemented,
diff --git a/src/lib/datasrc/memory_datasrc_link.cc b/src/lib/datasrc/memory_datasrc_link.cc
index d5a3f85277..92b3cb116f 100644
--- a/src/lib/datasrc/memory_datasrc_link.cc
+++ b/src/lib/datasrc/memory_datasrc_link.cc
@@ -33,6 +33,8 @@ using namespace isc::data;
 namespace isc {
 namespace datasrc {
 
+/// This exception is raised if there is an error in the configuration
+/// that has been passed; missing information, duplicate values, etc.
 class InMemoryConfigError : public isc::Exception {
 public:
     InMemoryConfigError(const char* file, size_t line, const char* what) :
@@ -138,11 +140,10 @@ checkConfig(ConstElementPtr config, ElementPtr errors) {
         }
         if (!config->contains("zones")) {
             // Assume empty list of zones
-            //addError(errors, "No 'zones' element in memory backend config");
-            //result = false;
         } else if (!config->get("zones") ||
                    config->get("zones")->getType() != Element::list) {
-            addError(errors, "'zones' element in memory backend config is not a list");
+            addError(errors,
+                     "'zones' element in memory backend config is not a list");
             result = false;
         } else {
             BOOST_FOREACH(ConstElementPtr zone_config,
@@ -161,9 +162,15 @@ checkConfig(ConstElementPtr config, ElementPtr errors) {
 // client must be freshly allocated, and config_value should have been
 // checked by the caller
 void
-applyConfig(isc::datasrc::InMemoryClient* client,
+applyConfig(isc::datasrc::InMemoryClient& client,
             isc::data::ConstElementPtr config_value)
 {
+    // XXX: We have lost the context to get to the default values here,
+    // as a temporary workaround we hardcode the IN class here.
+    isc::dns::RRClass rrclass = RRClass::IN();
+    if (config_value->contains("class")) {
+        rrclass = RRClass(config_value->get("class")->stringValue());
+    }
     ConstElementPtr zones_config = config_value->get("zones");
     if (!zones_config) {
         // XXX: Like the RR class, we cannot retrieve the default value here,
@@ -171,10 +178,6 @@ applyConfig(isc::datasrc::InMemoryClient* client,
         return;
     }
 
-    isc::dns::RRClass rrclass = RRClass::IN();
-    if (config_value->contains("class")) {
-        rrclass = RRClass(config_value->get("class")->stringValue());
-    }
 
     BOOST_FOREACH(ConstElementPtr zone_config, zones_config->listValue()) {
         ConstElementPtr origin = zone_config->get("origin");
@@ -220,7 +223,7 @@ applyConfig(isc::datasrc::InMemoryClient* client,
         }
 
         boost::shared_ptr<InMemoryZoneFinder> zone_finder(imzf);
-        const result::Result result = client->addZone(zone_finder);
+        const result::Result result = client.addZone(zone_finder);
         if (result == result::EXIST) {
             isc_throw(InMemoryConfigError, "zone "<< origin->str()
                       << " already exists");
@@ -250,21 +253,17 @@ createInstance(isc::data::ConstElementPtr config, std::string& error) {
         error = "Configuration error: " + errors->str();
         return (NULL);
     }
-    isc::datasrc::InMemoryClient* client = NULL;
     try {
-        client = new isc::datasrc::InMemoryClient();
-        applyConfig(client, config);
-        return (client);
+        std::auto_ptr<InMemoryClient> client(new isc::datasrc::InMemoryClient());
+        applyConfig(*client, config);
+        return (client.release());
     } catch (const isc::Exception& isce) {
-        delete client;
         error = isce.what();
         return (NULL);
     } catch (const std::exception& exc) {
-        delete client;
         error = std::string("Error creating memory datasource: ") + exc.what();
         return (NULL);
     } catch (...) {
-        delete client;
         error = std::string("Error creating memory datasource, "
                             "unknown exception");
         return (NULL);