[2212] Merge branch 'trac2209' into trac2212-2

src/lib/datasrc/client_list.cc

@@ -12,17 +12,21 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <util/memory_segment_local.h>
 
 #include "client_list.h"
 #include "client.h"
 #include "factory.h"
 #include "memory/memory_client.h"
+#include "memory/zone_table_segment.h"
+#include "memory/zone_writer.h"
+#include "memory/zone_data_loader.h"
 #include "logger.h"
 #include <dns/masterload.h>
+#include <util/memory_segment_local.h>
 
 #include <memory>
 #include <boost/foreach.hpp>
+#include <boost/bind.hpp>
 
 using namespace isc::data;
 using namespace isc::dns;
@@ -337,33 +341,89 @@ ConfigurableClientList::findInternal(MutableResult& candidate,
     // and the need_updater parameter is true, get the zone there.
 }
 
+// We still provide this method for backward compatibility. But to not have
+// duplicate code, it is a thin wrapper around getCachedZoneWriter only.
 ConfigurableClientList::ReloadResult
 ConfigurableClientList::reload(const Name& name) {
+    ZoneWriterPair result(getCachedZoneWriter(name));
+    if (result.second) {
+        result.second->load();
+        result.second->install();
+        result.second->cleanup();
+    }
+    return (result.first);
+}
+
+namespace {
+
+// We would like to use boost::bind for this. However, the loadZoneData takes
+// a reference, while we have a shared pointer to the iterator -- and we need
+// to keep it alive as long as the ZoneWriter is alive. Therefore we can't
+// really just dereference it and pass it, since it would get destroyed once
+// the getCachedZoneWriter would end. This class holds the shared pointer
+// alive, otherwise is mostly simple.
+//
+// It might be doable with nested boost::bind, but it would probably look
+// more awkward and complicated than this.
+class IteratorLoader {
+public:
+    IteratorLoader(const RRClass& rrclass, const Name& name,
+                   const ZoneIteratorPtr& iterator) :
+        rrclass_(rrclass),
+        name_(name),
+        iterator_(iterator)
+    {}
+    memory::ZoneData* operator()(util::MemorySegment& segment) {
+        return (memory::loadZoneData(segment, rrclass_, name_, *iterator_));
+    }
+private:
+    const RRClass rrclass_;
+    const Name name_;
+    ZoneIteratorPtr iterator_;
+};
+
+// We can't use the loadZoneData function directly in boost::bind, since
+// it is overloaded and the compiler can't choose the correct version
+// reliably and fails. So we simply wrap it into an unique name.
+memory::ZoneData*
+loadZoneDataFromFile(util::MemorySegment& segment, const RRClass& rrclass,
+                     const Name& name, const string& filename)
+{
+    return (memory::loadZoneData(segment, rrclass, name, filename));
+}
+
+}
+
+ConfigurableClientList::ZoneWriterPair
+ConfigurableClientList::getCachedZoneWriter(const Name& name) {
     if (!allow_cache_) {
-        return (CACHE_DISABLED);
+        return (ZoneWriterPair(CACHE_DISABLED, ZoneWriterPtr()));
     }
     // Try to find the correct zone.
     MutableResult result;
     findInternal(result, name, true, true);
     if (!result.finder) {
-        return (ZONE_NOT_FOUND);
+        return (ZoneWriterPair(ZONE_NOT_FOUND, ZoneWriterPtr()));
     }
-    // Try to convert the finder to in-memory one. If it is the cache,
-    // it should work.
-    // It is of a different type or there's no cache.
+    // Try to get the in-memory cache for the zone. If there's none,
+    // we can't provide the result.
     if (!result.info->cache_) {
-        return (ZONE_NOT_CACHED);
+        return (ZoneWriterPair(ZONE_NOT_CACHED, ZoneWriterPtr()));
     }
+    memory::LoadAction load_action;
     DataSourceClient* client(result.info->data_src_client_);
     if (client) {
-        // Now do the final reload. If it does not exist in client,
+        // Now finally provide the writer.
+        // If it does not exist in client,
         // DataSourceError is thrown, which is exactly the result what we
         // want, so no need to handle it.
         ZoneIteratorPtr iterator(client->getIterator(name));
         if (!iterator) {
             isc_throw(isc::Unexpected, "Null iterator from " << name);
         }
-        result.info->cache_->load(name, *iterator);
+        // And wrap the iterator into the correct functor (which
+        // keeps it alive as long as it is needed).
+        load_action = IteratorLoader(rrclass_, name, iterator);
     } else {
         // The MasterFiles special case
         const string filename(result.info->cache_->getFileName(name));
@@ -371,9 +431,13 @@ ConfigurableClientList::reload(const Name& name) {
             isc_throw(isc::Unexpected, "Confused about missing both filename "
                       "and data source");
         }
-        result.info->cache_->load(name, filename);
+        // boost::bind is enough here.
+        load_action = boost::bind(loadZoneDataFromFile, _1, rrclass_, name,
+                                  filename);
     }
-    return (ZONE_RELOADED);
+    return (ZoneWriterPair(ZONE_RELOADED,
+                           ZoneWriterPtr(result.info->cache_->getZoneTableSegment().
+                                         getZoneWriter(load_action, name, rrclass_))));
 }
 
 // NOTE: This function is not tested, it would be complicated. However, the

src/lib/datasrc/client_list.h

@@ -42,6 +42,7 @@ typedef boost::shared_ptr<DataSourceClientContainer>
 // and hide real definitions except for itself and tests.
 namespace memory {
 class InMemoryClient;
+class ZoneWriter;
 }
 
 /// \brief The list of data source clients.
@@ -288,6 +289,33 @@ public:
     ///      the original data source no longer contains the cached zone.
     ReloadResult reload(const dns::Name& zone);
 
+    /// \brief Convenience type shortcut
+    typedef boost::shared_ptr<memory::ZoneWriter> ZoneWriterPtr;
+
+    /// \brief Return value of getCachedZoneWriter()
+    ///
+    /// A pair containing status and the zone writer, for the
+    /// getCachedZoneWriter() method.
+    typedef std::pair<ReloadResult, ZoneWriterPtr> ZoneWriterPair;
+
+    /// \brief Return a zone writer that can be used to reload a zone.
+    ///
+    /// This looks up a cached copy of zone and returns the ZoneWriter
+    /// that can be used to reload the content of the zone. This can
+    /// be used instead of reload() -- reload() works synchronously, which
+    /// is not what is needed every time.
+    ///
+    /// \param zone The origin of the zone to reload.
+    /// \return The result has two parts. The first one is a status describing
+    ///     if it worked or not (and in case it didn't, also why). If the status
+    ///     is ZONE_RELOADED, the second part contains a shared pointer to the
+    ///     writer. If the status is anything else, the second part is NULL.
+    /// \throw DataSourceError or anything else that the data source
+    ///      containing the zone might throw is propagated.
+    /// \throw DataSourceError if something unexpected happens, like when
+    ///      the original data source no longer contains the cached zone.
+    ZoneWriterPair getCachedZoneWriter(const dns::Name& zone);
+
     /// \brief Implementation of the ClientList::find.
     virtual FindResult find(const dns::Name& zone,
                             bool want_exact_match = false,

src/lib/datasrc/memory/memory_client.cc

@@ -22,6 +22,7 @@
 #include <datasrc/memory/treenode_rrset.h>
 #include <datasrc/memory/zone_finder.h>
 #include <datasrc/memory/zone_data_loader.h>
+#include <datasrc/memory/zone_table_segment.h>
 
 #include <util/memory_segment_local.h>
 
@@ -66,7 +67,14 @@ public:
 
 InMemoryClient::InMemoryClient(util::MemorySegment& mem_sgmt,
                                RRClass rrclass) :
-    mem_sgmt_(mem_sgmt),
+    // FIXME: We currently use the temporary and "unsupported"
+    // constructor of the zone table segment. Once we clarify
+    // how the config thing, we want to change it.
+    zone_table_segment_(ZoneTableSegment::create(mem_sgmt)),
+    // Use the memory segment from the zone table segment. Currently,
+    // it is the same one as the one in parameter, but that will
+    // probably change.
+    mem_sgmt_(zone_table_segment_->getMemorySegment()),
     rrclass_(rrclass),
     zone_count_(0)
 {
@@ -76,13 +84,19 @@ InMemoryClient::InMemoryClient(util::MemorySegment& mem_sgmt,
     file_name_tree_ = FileNameTree::create(mem_sgmt_, false);
 
     zone_table_ = holder.release();
+    // TODO: Once the table is created inside the zone table segment, use that
+    // one.
+    zone_table_segment_->getHeader().setTable(zone_table_);
 }
 
 InMemoryClient::~InMemoryClient() {
     FileNameDeleter deleter;
     FileNameTree::destroy(mem_sgmt_, file_name_tree_, deleter);
 
+    // TODO: Once the table is created inside the zone table segment, do not
+    // destroy it here.
     ZoneTable::destroy(mem_sgmt_, zone_table_, rrclass_);
+    ZoneTableSegment::destroy(zone_table_segment_);
 }
 
 result::Result
@@ -207,7 +221,7 @@ private:
     bool separate_rrs_;
     bool ready_;
 public:
-    MemoryIterator(const RRClass rrclass,
+    MemoryIterator(const RRClass& rrclass,
                    const ZoneTree& tree, const Name& origin,
                    bool separate_rrs) :
         rrclass_(rrclass),

src/lib/datasrc/memory/memory_client.h

@@ -34,6 +34,8 @@ class RRsetList;
 namespace datasrc {
 namespace memory {
 
+class ZoneTableSegment;
+
 /// \brief A data source client that holds all necessary data in memory.
 ///
 /// The \c InMemoryClient class provides an access to a conceptual data
@@ -175,6 +177,18 @@ public:
     getJournalReader(const isc::dns::Name& zone, uint32_t begin_serial,
                      uint32_t end_serial) const;
 
+    /// \brief Get the zone table segment used
+    ///
+    /// This is a low-level function, used to some internal handling when,
+    /// for example, reloading the data inside the in-memory data source.
+    /// It should not be generally used.
+    ///
+    /// \todo Consider making this private and add a friend declaration
+    ///     for the ClientList.
+    ZoneTableSegment& getZoneTableSegment() {
+        return (*zone_table_segment_);
+    }
+
 private:
     // Some type aliases
     typedef DomainTree<std::string> FileNameTree;
@@ -186,6 +200,7 @@ private:
                                 const std::string& filename,
                                 ZoneData* zone_data);
 
+    ZoneTableSegment* zone_table_segment_;
     util::MemorySegment& mem_sgmt_;
     const isc::dns::RRClass rrclass_;
     unsigned int zone_count_;

src/lib/datasrc/memory/zone_data_loader.cc

@@ -165,7 +165,7 @@ ZoneDataLoader::getCurrentName() const {
 
 ZoneData*
 loadZoneDataInternal(util::MemorySegment& mem_sgmt,
-                     const isc::dns::RRClass rrclass,
+                     const isc::dns::RRClass& rrclass,
                      const Name& zone_name,
                      boost::function<void(LoadCallback)> rrset_installer)
 {
@@ -223,7 +223,7 @@ generateRRsetFromIterator(ZoneIterator* iterator, LoadCallback callback) {
 
 ZoneData*
 loadZoneData(util::MemorySegment& mem_sgmt,
-             const isc::dns::RRClass rrclass,
+             const isc::dns::RRClass& rrclass,
              const isc::dns::Name& zone_name,
              const std::string& zone_file)
 {
@@ -236,7 +236,7 @@ loadZoneData(util::MemorySegment& mem_sgmt,
 
 ZoneData*
 loadZoneData(util::MemorySegment& mem_sgmt,
-             const isc::dns::RRClass rrclass,
+             const isc::dns::RRClass& rrclass,
              const isc::dns::Name& zone_name,
              ZoneIterator& iterator)
 {

src/lib/datasrc/memory/zone_data_loader.h

@@ -48,7 +48,7 @@ struct EmptyZone : public InvalidParameter {
 /// \param zone_name The name of the zone that is being loaded.
 /// \param zone_file Filename which contains the zone data for \c zone_name.
 ZoneData* loadZoneData(util::MemorySegment& mem_sgmt,
-                       const isc::dns::RRClass rrclass,
+                       const isc::dns::RRClass& rrclass,
                        const isc::dns::Name& zone_name,
                        const std::string& zone_file);
 
@@ -65,7 +65,7 @@ ZoneData* loadZoneData(util::MemorySegment& mem_sgmt,
 /// \param zone_name The name of the zone that is being loaded.
 /// \param iterator Iterator that returns RRsets to load into the zone.
 ZoneData* loadZoneData(util::MemorySegment& mem_sgmt,
-                       const isc::dns::RRClass rrclass,
+                       const isc::dns::RRClass& rrclass,
                        const isc::dns::Name& zone_name,
                        ZoneIterator& iterator);
 

src/lib/datasrc/memory/zone_table_segment.cc

@@ -28,6 +28,11 @@ ZoneTableSegment::create(const isc::data::Element&) {
     return (new ZoneTableSegmentLocal);
 }
 
+ZoneTableSegment*
+ZoneTableSegment::create(isc::util::MemorySegment& segment) {
+    return (new ZoneTableSegmentLocal(segment));
+}
+
 void
 ZoneTableSegment::destroy(ZoneTableSegment *segment) {
     delete segment;

src/lib/datasrc/memory/zone_table_segment.h

@@ -121,6 +121,18 @@ public:
     /// \return Returns a ZoneTableSegment object
     static ZoneTableSegment* create(const isc::data::Element& config);
 
+    /// \brief Temporary/Testing version of create.
+    ///
+    /// This exists as a temporary solution during the migration phase
+    /// towards using the ZoneTableSegment. It doesn't take a config,
+    /// but a memory segment instead. If you can, you should use the
+    /// other version, this one will be gone soon.
+    ///
+    /// \param segment The memory segment to use.
+    /// \return Returns a new ZoneTableSegment object.
+    /// \todo Remove this method.
+    static ZoneTableSegment* create(isc::util::MemorySegment& segment);
+
     /// \brief Destroy a ZoneTableSegment
     ///
     /// This method destroys the passed ZoneTableSegment. It must be

src/lib/datasrc/memory/zone_table_segment_local.h

@@ -37,7 +37,13 @@ protected:
     /// Instances are expected to be created by the factory method
     /// (\c ZoneTableSegment::create()), so this constructor is
     /// protected.
-    ZoneTableSegmentLocal()
+    ZoneTableSegmentLocal() :
+        mem_sgmt_(mem_sgmt_local_)
+    {}
+    // TODO: A temporary constructor, for tests for now. Needs to
+    // be removed.
+    ZoneTableSegmentLocal(isc::util::MemorySegment& segment) :
+        mem_sgmt_(segment)
     {}
 public:
     /// \brief Destructor
@@ -60,7 +66,8 @@ public:
                                       const dns::RRClass& rrclass);
 private:
     ZoneTableHeader header_;
-    isc::util::MemorySegmentLocal mem_sgmt_;
+    isc::util::MemorySegmentLocal mem_sgmt_local_;
+    isc::util::MemorySegment& mem_sgmt_;
 };
 
 } // namespace memory

src/lib/datasrc/tests/client_list_unittest.cc

@@ -20,6 +20,7 @@
 #include <datasrc/data_source.h>
 #include <datasrc/memory/memory_client.h>
 #include <datasrc/memory/zone_finder.h>
+#include <datasrc/memory/zone_writer.h>
 
 #include <dns/rrclass.h>
 #include <dns/rrttl.h>
@@ -844,116 +845,169 @@ TEST_F(ListTest, BadMasterFile) {
                    true);
 }
 
+// This allows us to test two versions of the reloading code
+// (One by calling reload(), one by obtaining a ZoneWriter and
+// plaping with that). Once we deprecate reload(), we should revert this
+// change and not use typed tests any more.
+template<class UpdateType>
+class ReloadTest : public ListTest {
+public:
+    ConfigurableClientList::ReloadResult doReload(const Name& origin);
+};
+
+// Version with calling reload()
+class ReloadUpdateType {};
+template<>
+ConfigurableClientList::ReloadResult
+ReloadTest<ReloadUpdateType>::doReload(const Name& origin) {
+    return (list_->reload(origin));
+};
+
+// Version with the ZoneWriter
+class WriterUpdateType {};
+template<>
+ConfigurableClientList::ReloadResult
+ReloadTest<WriterUpdateType>::doReload(const Name& origin) {
+    ConfigurableClientList::ZoneWriterPair
+        result(list_->getCachedZoneWriter(origin));
+    if (result.first == ConfigurableClientList::ZONE_RELOADED) {
+        // Can't use ASSERT_NE here, it would wan't to return(), which
+        // it can't in non-void function.
+        if (result.second) {
+            result.second->load();
+            result.second->install();
+            result.second->cleanup();
+        } else {
+            ADD_FAILURE() << "getCachedZoneWriter returned ZONE_RELOADED, "
+                "but the writer is NULL";
+        }
+    } else {
+        EXPECT_EQ(static_cast<memory::ZoneWriter*>(NULL),
+                  result.second.get());
+    }
+    return (result.first);
+}
+
+// Typedefs for the GTEST guts to make it work
+typedef ::testing::Types<ReloadUpdateType, WriterUpdateType> UpdateTypes;
+TYPED_TEST_CASE(ReloadTest, UpdateTypes);
+
 // Test we can reload a zone
-TEST_F(ListTest, reloadSuccess) {
-    list_->configure(config_elem_zones_, true);
+TYPED_TEST(ReloadTest, reloadSuccess) {
+    this->list_->configure(this->config_elem_zones_, true);
     const Name name("example.org");
-    prepareCache(0, name);
+    this->prepareCache(0, name);
     // The cache currently contains a tweaked version of zone, which doesn't
     // have apex NS.  So the lookup should result in NXRRSET.
     EXPECT_EQ(ZoneFinder::NXRRSET,
-              list_->find(name).finder_->find(name, RRType::NS())->code);
+              this->list_->find(name).finder_->find(name, RRType::NS())->code);
     // Now reload the full zone. It should be there now.
-    EXPECT_EQ(ConfigurableClientList::ZONE_RELOADED, list_->reload(name));
+    EXPECT_EQ(ConfigurableClientList::ZONE_RELOADED, this->doReload(name));
     EXPECT_EQ(ZoneFinder::SUCCESS,
-              list_->find(name).finder_->find(name, RRType::NS())->code);
+              this->list_->find(name).finder_->find(name, RRType::NS())->code);
 }
 
 // The cache is not enabled. The load should be rejected.
-TEST_F(ListTest, reloadNotEnabled) {
-    list_->configure(config_elem_zones_, false);
+TYPED_TEST(ReloadTest, reloadNotEnabled) {
+    this->list_->configure(this->config_elem_zones_, false);
     const Name name("example.org");
     // We put the cache in even when not enabled. This won't confuse the thing.
-    prepareCache(0, name);
+    this->prepareCache(0, name);
     // See the reloadSuccess test.  This should result in NXRRSET.
     EXPECT_EQ(ZoneFinder::NXRRSET,
-              list_->find(name).finder_->find(name, RRType::NS())->code);
+              this->list_->find(name).finder_->find(name, RRType::NS())->code);
     // Now reload. It should reject it.
-    EXPECT_EQ(ConfigurableClientList::CACHE_DISABLED, list_->reload(name));
+    EXPECT_EQ(ConfigurableClientList::CACHE_DISABLED, this->doReload(name));
     // Nothing changed here
     EXPECT_EQ(ZoneFinder::NXRRSET,
-              list_->find(name).finder_->find(name, RRType::NS())->code);
+              this->list_->find(name).finder_->find(name, RRType::NS())->code);
 }
 
 // Test several cases when the zone does not exist
-TEST_F(ListTest, reloadNoSuchZone) {
-    list_->configure(config_elem_zones_, true);
+TYPED_TEST(ReloadTest, reloadNoSuchZone) {
+    this->list_->configure(this->config_elem_zones_, true);
     const Name name("example.org");
     // We put the cache in even when not enabled. This won't confuse the
     // reload method, as that one looks at the real state of things, not
     // at the configuration.
-    prepareCache(0, Name("example.com"));
+    this->prepareCache(0, Name("example.com"));
     // Not in the data sources
     EXPECT_EQ(ConfigurableClientList::ZONE_NOT_FOUND,
-              list_->reload(Name("example.cz")));
+              this->doReload(Name("exmaple.cz")));
     // Not cached
-    EXPECT_EQ(ConfigurableClientList::ZONE_NOT_FOUND, list_->reload(name));
+    EXPECT_EQ(ConfigurableClientList::ZONE_NOT_FOUND, this->doReload(name));
     // Partial match
     EXPECT_EQ(ConfigurableClientList::ZONE_NOT_FOUND,
-              list_->reload(Name("sub.example.com")));
+              this->doReload(Name("sub.example.com")));
     // Nothing changed here - these zones don't exist
     EXPECT_EQ(static_cast<isc::datasrc::DataSourceClient*>(NULL),
-              list_->find(name).dsrc_client_);
+              this->list_->find(name).dsrc_client_);
     EXPECT_EQ(static_cast<isc::datasrc::DataSourceClient*>(NULL),
-              list_->find(Name("example.cz")).dsrc_client_);
+              this->list_->find(Name("example.cz")).dsrc_client_);
     EXPECT_EQ(static_cast<isc::datasrc::DataSourceClient*>(NULL),
-              list_->find(Name("sub.example.com"), true).dsrc_client_);
+              this->list_->find(Name("sub.example.com"), true).dsrc_client_);
     // Not reloaded, so NS shouldn't be visible yet.
     EXPECT_EQ(ZoneFinder::NXRRSET,
-              list_->find(Name("example.com")).finder_->
+              this->list_->find(Name("example.com")).finder_->
               find(Name("example.com"), RRType::NS())->code);
 }
 
 // Check we gracefuly throw an exception when a zone disappeared in
 // the underlying data source when we want to reload it
-TEST_F(ListTest, reloadZoneGone) {
-    list_->configure(config_elem_, true);
+TYPED_TEST(ReloadTest, reloadZoneGone) {
+    this->list_->configure(this->config_elem_, true);
     const Name name("example.org");
     // We put in a cache for non-existant zone. This emulates being loaded
     // and then the zone disappearing. We prefill the cache, so we can check
     // it.
-    prepareCache(0, name);
+    this->prepareCache(0, name);
     // The (cached) zone contains zone's SOA
     EXPECT_EQ(ZoneFinder::SUCCESS,
-              list_->find(name).finder_->find(name, RRType::SOA())->code);
+              this->list_->find(name).finder_->find(name,
+                                                    RRType::SOA())->code);
     // The zone is not there, so abort the reload.
-    EXPECT_THROW(list_->reload(name), DataSourceError);
+    EXPECT_THROW(this->doReload(name), DataSourceError);
     // The (cached) zone is not hurt.
     EXPECT_EQ(ZoneFinder::SUCCESS,
-              list_->find(name).finder_->find(name, RRType::SOA())->code);
+              this->list_->find(name).finder_->find(name,
+                                                    RRType::SOA())->code);
 }
 
 // The underlying data source throws. Check we don't modify the state.
-TEST_F(ListTest, reloadZoneThrow) {
-    list_->configure(config_elem_zones_, true);
+TYPED_TEST(ReloadTest, reloadZoneThrow) {
+    this->list_->configure(this->config_elem_zones_, true);
     const Name name("noiter.org");
-    prepareCache(0, name);
+    this->prepareCache(0, name);
     // The zone contains stuff now
     EXPECT_EQ(ZoneFinder::SUCCESS,
-              list_->find(name).finder_->find(name, RRType::SOA())->code);
+              this->list_->find(name).finder_->find(name,
+                                                    RRType::SOA())->code);
     // The iterator throws, so abort the reload.
-    EXPECT_THROW(list_->reload(name), isc::NotImplemented);
+    EXPECT_THROW(this->doReload(name), isc::NotImplemented);
     // The zone is not hurt.
     EXPECT_EQ(ZoneFinder::SUCCESS,
-              list_->find(name).finder_->find(name, RRType::SOA())->code);
+              this->list_->find(name).finder_->find(name,
+                                                    RRType::SOA())->code);
 }
 
-TEST_F(ListTest, reloadNullIterator) {
-    list_->configure(config_elem_zones_, true);
+TYPED_TEST(ReloadTest, reloadNullIterator) {
+    this->list_->configure(this->config_elem_zones_, true);
     const Name name("null.org");
-    prepareCache(0, name);
+    this->prepareCache(0, name);
     // The zone contains stuff now
     EXPECT_EQ(ZoneFinder::SUCCESS,
-              list_->find(name).finder_->find(name, RRType::SOA())->code);
+              this->list_->find(name).finder_->find(name,
+                                                    RRType::SOA())->code);
     // The iterator throws, so abort the reload.
-    EXPECT_THROW(list_->reload(name), isc::Unexpected);
+    EXPECT_THROW(this->doReload(name), isc::Unexpected);
     // The zone is not hurt.
     EXPECT_EQ(ZoneFinder::SUCCESS,
-              list_->find(name).finder_->find(name, RRType::SOA())->code);
+              this->list_->find(name).finder_->find(name,
+                                                    RRType::SOA())->code);
 }
 
 // Test we can reload the master files too (special-cased)
-TEST_F(ListTest, reloadMasterFile) {
+TYPED_TEST(ReloadTest, reloadMasterFile) {
     const char* const install_cmd = INSTALL_PROG " -c " TEST_DATA_DIR
         "/root.zone " TEST_DATA_BUILDDIR "/root.zone.copied";
     if (system(install_cmd) != 0) {
@@ -971,21 +1025,22 @@ TEST_F(ListTest, reloadMasterFile) {
         "       \".\": \"" TEST_DATA_BUILDDIR "/root.zone.copied\""
         "   }"
         "}]"));
-    list_->configure(elem, true);
+    this->list_->configure(elem, true);
     // Add a record that is not in the zone
     EXPECT_EQ(ZoneFinder::NXDOMAIN,
-              list_->find(Name(".")).finder_->find(Name("nosuchdomain"),
-                                                   RRType::TXT())->code);
+              this->list_->find(Name(".")).finder_->find(Name("nosuchdomain"),
+                                                         RRType::TXT())->code);
     ofstream f;
     f.open(TEST_DATA_BUILDDIR "/root.zone.copied", ios::out | ios::app);
     f << "nosuchdomain.\t\t3600\tIN\tTXT\ttest" << std::endl;
     f.close();
     // Do the reload.
-    EXPECT_EQ(ConfigurableClientList::ZONE_RELOADED, list_->reload(Name(".")));
+    EXPECT_EQ(ConfigurableClientList::ZONE_RELOADED,
+              this->doReload(Name(".")));
     // It is here now.
     EXPECT_EQ(ZoneFinder::SUCCESS,
-              list_->find(Name(".")).finder_->find(Name("nosuchdomain"),
-                                                   RRType::TXT())->code);
+              this->list_->find(Name(".")).finder_->find(Name("nosuchdomain"),
+                                                         RRType::TXT())->code);
 }
 
 }

src/lib/datasrc/tests/memory/memory_client_unittest.cc

@@ -39,6 +39,8 @@
 
 #include <gtest/gtest.h>
 
+#include <boost/lexical_cast.hpp>
+
 #include <new>                  // for bad_alloc
 
 using namespace isc::dns;
@@ -289,7 +291,8 @@ TEST_F(MemoryClientTest, loadMemoryAllocationFailures) {
     // Just to check that things get cleaned up
 
     for (int i = 1; i < 16; i++) {
-        SCOPED_TRACE("For throw count = " + i);
+        SCOPED_TRACE("For throw count = " +
+                     boost::lexical_cast<std::string>(i));
         mem_sgmt_.setThrowCount(i);
         EXPECT_THROW({
             // Include the InMemoryClient construction too here. Now,
@@ -762,4 +765,12 @@ TEST_F(MemoryClientTest, getJournalReaderNotImplemented) {
                  isc::NotImplemented);
 }
 
+TEST_F(MemoryClientTest, getZoneTableSegment) {
+    // It's hard to test this method. It returns a reference, so we can't even
+    // check for non-NULL. Checking it doesn't throw/crash is good enough for
+    // now, the method will be used in other functions, so checked it works
+    // implicitly.
+    EXPECT_NO_THROW(client_->getZoneTableSegment());
+}
+
 }