From: Michal 'vorner' Vaner Date: Tue, 9 Oct 2012 11:39:30 +0000 (+0200) Subject: [2292] Don't pass SegmentObjectHolder X-Git-Tag: trac2351_base~1^2~9^2~3 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=7888a2127db48e004cb1cce24552db230cb3d685;p=thirdparty%2Fkea.git [2292] Don't pass SegmentObjectHolder It is supposed to be mostly private. Use it internally only. --- diff --git a/src/lib/datasrc/memory/memory_client.cc b/src/lib/datasrc/memory/memory_client.cc index a5824fa63b..e33a0f32e9 100644 --- a/src/lib/datasrc/memory/memory_client.cc +++ b/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. diff --git a/src/lib/datasrc/memory/zone_table.cc b/src/lib/datasrc/memory/zone_table.cc index 0e6ed3c4ed..4a1f184974 100644 --- a/src/lib/datasrc/memory/zone_table.cc +++ b/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& content) + const Name& zone_name, ZoneData* content) { + if (content == NULL) { + isc_throw(isc::BadValue, "Zone content must not be NULL"); + } + SegmentObjectHolder 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); diff --git a/src/lib/datasrc/memory/zone_table.h b/src/lib/datasrc/memory/zone_table.h index 4f309b7c2e..5ade3ac963 100644 --- a/src/lib/datasrc/memory/zone_table.h +++ b/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 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& content); + ZoneData* content); /// Find a zone that best matches the given name in the \c ZoneTable. /// diff --git a/src/lib/datasrc/tests/memory/zone_table_unittest.cc b/src/lib/datasrc/tests/memory/zone_table_unittest.cc index b9ee3d9441..64ec312c02 100644 --- a/src/lib/datasrc/tests/memory/zone_table_unittest.cc +++ b/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 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 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 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 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 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 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 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); }