[2292] Don't pass SegmentObjectHolder

It is supposed to be mostly private. Use it internally only.

src/lib/datasrc/memory/memory_client.cc

@@ -631,7 +631,8 @@ InMemoryClient::InMemoryClientImpl::load(
     delete tstr;
 
     const result::Result result(zone_table_->addZone(mem_sgmt_, rrclass_,
-                                                     zone_name, holder));
+                                                     zone_name,
+                                                     holder.release()));
     if (result == result::SUCCESS) {
         // Only increment the zone count if the zone doesn't already
         // exist.

src/lib/datasrc/memory/zone_table.cc

@@ -69,9 +69,13 @@ ZoneTable::destroy(util::MemorySegment& mem_sgmt, ZoneTable* ztable,
 
 result::Result
 ZoneTable::addZone(util::MemorySegment& mem_sgmt, RRClass zone_class,
-                   const Name& zone_name,
-                   SegmentObjectHolder<ZoneData, RRClass>& content)
+                   const Name& zone_name, ZoneData* content)
 {
+    if (content == NULL) {
+        isc_throw(isc::BadValue, "Zone content must not be NULL");
+    }
+    SegmentObjectHolder<ZoneData, RRClass> holder(mem_sgmt, content,
+                                                  zone_class);
     // Get the node where we put the zone
     ZoneTableNode* node(NULL);
     switch (zones_->insert(mem_sgmt, zone_name, &node)) {
@@ -87,7 +91,7 @@ ZoneTable::addZone(util::MemorySegment& mem_sgmt, RRClass zone_class,
     assert(node != NULL);
 
     // We can release now, setData never throws
-    ZoneData* old = node->setData(content.release());
+    ZoneData* old = node->setData(holder.release());
     if (old != NULL) {
         ZoneData::destroy(mem_sgmt, old, zone_class);
         return (result::EXIST);

src/lib/datasrc/memory/zone_table.h

@@ -36,10 +36,6 @@ namespace memory {
 // forward declaration: in this header it's mostly an opaque type.
 class ZoneData;
 
-namespace detail {
-template<class T, class C> class SegmentObjectHolder;
-}
-
 /// \brief A conceptual table of authoritative zones.
 ///
 /// This class is actually a simple wrapper for a \c DomainTree whose data is
@@ -147,7 +143,9 @@ public:
     /// \param zone_class The RR class of the zone.  It must be the RR class
     ///     that is supposed to be associated to the zone table.
     /// \param content This one should hold the zone content (the ZoneData).
-    ///     When it is added successfully, it is released from the holder.
+    ///     The ownership is passed onto the zone table. Must not be null.
+    ///     Must correspond to the name and class and must be allocated from
+    ///     mem_sgmt.
     /// \return \c result::SUCCESS If the zone is successfully
     ///     added to the zone table.
     /// \return \c result::EXIST The zone table already contained
@@ -156,8 +154,7 @@ public:
     result::Result addZone(util::MemorySegment& mem_sgmt,
                            dns::RRClass zone_class,
                            const dns::Name& zone_name,
-                           detail::SegmentObjectHolder<ZoneData,
-                           isc::dns::RRClass>& content);
+                           ZoneData* content);
 
     /// Find a zone that best matches the given name in the \c ZoneTable.
     ///

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

@@ -89,11 +89,15 @@ TEST_F(ZoneTableTest, create) {
 }
 
 TEST_F(ZoneTableTest, addZone) {
+    // It doesn't accept empty (NULL) zones
+    EXPECT_THROW(zone_table->addZone(mem_sgmt_, zclass_, zname1, NULL),
+                 isc::BadValue);
+
     SegmentObjectHolder<ZoneData, RRClass> holder1(
         mem_sgmt_, ZoneData::create(mem_sgmt_, zname1), zclass_);
     // Normal successful case.
     EXPECT_EQ(result::SUCCESS, zone_table->addZone(mem_sgmt_, zclass_,
-                                                   zname1, holder1));
+                                                   zname1, holder1.release()));
     // It got released by it
     EXPECT_EQ(NULL, holder1.get());
 
@@ -101,7 +105,7 @@ TEST_F(ZoneTableTest, addZone) {
     SegmentObjectHolder<ZoneData, RRClass> holder2(
         mem_sgmt_, ZoneData::create(mem_sgmt_, zname1), zclass_);
     EXPECT_EQ(result::EXIST, zone_table->addZone(mem_sgmt_, zclass_,
-                                                 zname1, holder2));
+                                                 zname1, holder2.release()));
     // It releases this one even when we replace the old zone
     EXPECT_EQ(NULL, holder2.get());
 
@@ -111,16 +115,18 @@ TEST_F(ZoneTableTest, addZone) {
     // names are compared in a case insensitive manner.
     EXPECT_EQ(result::EXIST, zone_table->addZone(mem_sgmt_, zclass_,
                                                  Name("EXAMPLE.COM"),
-                                                 holder3));
+                                                 holder3.release()));
     // Add some more different ones.  Should just succeed.
     SegmentObjectHolder<ZoneData, RRClass> holder4(
         mem_sgmt_, ZoneData::create(mem_sgmt_, zname2), zclass_);
     EXPECT_EQ(result::SUCCESS,
-              zone_table->addZone(mem_sgmt_, zclass_, zname2, holder4));
+              zone_table->addZone(mem_sgmt_, zclass_, zname2,
+                                  holder4.release()));
     SegmentObjectHolder<ZoneData, RRClass> holder5(
         mem_sgmt_, ZoneData::create(mem_sgmt_, zname3), zclass_);
     EXPECT_EQ(result::SUCCESS,
-              zone_table->addZone(mem_sgmt_, zclass_, zname3, holder5));
+              zone_table->addZone(mem_sgmt_, zclass_, zname3,
+                                  holder5.release()));
 
     // Have the memory segment throw an exception in extending the internal
     // tree.  It still shouldn't cause memory leak (which would be detected
@@ -129,7 +135,7 @@ TEST_F(ZoneTableTest, addZone) {
         mem_sgmt_, ZoneData::create(mem_sgmt_, Name("example.org")), zclass_);
     mem_sgmt_.setThrowCount(1);
     EXPECT_THROW(zone_table->addZone(mem_sgmt_, zclass_, Name("example.org"),
-                                     holder6),
+                                     holder6.release()),
                  std::bad_alloc);
 }
 
@@ -138,15 +144,17 @@ TEST_F(ZoneTableTest, findZone) {
         mem_sgmt_, ZoneData::create(mem_sgmt_, zname1), zclass_);
     ZoneData* zone_data = holder1.get();
     EXPECT_EQ(result::SUCCESS, zone_table->addZone(mem_sgmt_, zclass_, zname1,
-                                                   holder1));
+                                                   holder1.release()));
     SegmentObjectHolder<ZoneData, RRClass> holder2(
         mem_sgmt_, ZoneData::create(mem_sgmt_, zname2), zclass_);
     EXPECT_EQ(result::SUCCESS,
-              zone_table->addZone(mem_sgmt_, zclass_, zname2, holder2));
+              zone_table->addZone(mem_sgmt_, zclass_, zname2,
+                                  holder2.release()));
     SegmentObjectHolder<ZoneData, RRClass> holder3(
         mem_sgmt_, ZoneData::create(mem_sgmt_, zname3), zclass_);
     EXPECT_EQ(result::SUCCESS,
-              zone_table->addZone(mem_sgmt_, zclass_, zname3, holder3));
+              zone_table->addZone(mem_sgmt_, zclass_, zname3,
+                                  holder3.release()));
 
     const ZoneTable::FindResult find_result1 =
         zone_table->findZone(Name("example.com"));
@@ -170,7 +178,8 @@ TEST_F(ZoneTableTest, findZone) {
     SegmentObjectHolder<ZoneData, RRClass> holder4(
         mem_sgmt_, ZoneData::create(mem_sgmt_, Name("com")), zclass_);
     EXPECT_EQ(result::SUCCESS, zone_table->addZone(mem_sgmt_, zclass_,
-                                                   Name("com"), holder4));
+                                                   Name("com"),
+                                                   holder4.release()));
     EXPECT_EQ(zone_data,
               zone_table->findZone(Name("www.example.com")).zone_data);
 }