BIND 10 trac2292, updated. d78dd497ecb846ab2545ce1cb7b6e41341ff5995 [2292] Clarify test
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Oct 9 12:13:16 UTC 2012
The branch, trac2292 has been updated
via d78dd497ecb846ab2545ce1cb7b6e41341ff5995 (commit)
via 3a6ee0d12d06c1f9e5b9f261b3fcaab444a9ae80 (commit)
via 7888a2127db48e004cb1cce24552db230cb3d685 (commit)
from 869d14d7056ee7b0c3cf1dd0db1b375be755bba7 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit d78dd497ecb846ab2545ce1cb7b6e41341ff5995
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Tue Oct 9 14:11:25 2012 +0200
[2292] Clarify test
And make it slightly stronger (it now checks something that should be
mostly obvious too, but who knows, with software).
commit 3a6ee0d12d06c1f9e5b9f261b3fcaab444a9ae80
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Tue Oct 9 14:07:13 2012 +0200
[2292] Pass the old zone back
Instead of releasing it directly. While the internal release was more
convenient, it didn't allow for swapping things fast under a mutex and
then spending the time releasing it unlocked.
commit 7888a2127db48e004cb1cce24552db230cb3d685
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Tue Oct 9 13:39:30 2012 +0200
[2292] Don't pass SegmentObjectHolder
It is supposed to be mostly private. Use it internally only.
-----------------------------------------------------------------------
Summary of changes:
src/lib/datasrc/memory/memory_client.cc | 13 ++++--
src/lib/datasrc/memory/zone_table.cc | 17 ++++---
src/lib/datasrc/memory/zone_table.h | 29 +++++++-----
.../datasrc/tests/memory/domaintree_unittest.cc | 5 +-
.../datasrc/tests/memory/zone_table_unittest.cc | 49 ++++++++++++++------
5 files changed, 74 insertions(+), 39 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/memory/memory_client.cc b/src/lib/datasrc/memory/memory_client.cc
index a5824fa..1609611 100644
--- a/src/lib/datasrc/memory/memory_client.cc
+++ b/src/lib/datasrc/memory/memory_client.cc
@@ -630,15 +630,20 @@ InMemoryClient::InMemoryClientImpl::load(
const std::string* tstr = node->setData(new std::string(filename));
delete tstr;
- const result::Result result(zone_table_->addZone(mem_sgmt_, rrclass_,
- zone_name, holder));
- if (result == result::SUCCESS) {
+ const ZoneTable::AddResult result(zone_table_->addZone(mem_sgmt_, rrclass_,
+ zone_name,
+ holder.release()));
+ if (result.code == result::SUCCESS) {
// Only increment the zone count if the zone doesn't already
// exist.
++zone_count_;
}
+ // Destroy the old instance of the zone if there was any
+ if (result.zone_data != NULL) {
+ ZoneData::destroy(mem_sgmt_, result.zone_data, rrclass_);
+ }
- return (result);
+ return (result.code);
}
namespace {
diff --git a/src/lib/datasrc/memory/zone_table.cc b/src/lib/datasrc/memory/zone_table.cc
index 0e6ed3c..836b020 100644
--- a/src/lib/datasrc/memory/zone_table.cc
+++ b/src/lib/datasrc/memory/zone_table.cc
@@ -67,11 +67,15 @@ ZoneTable::destroy(util::MemorySegment& mem_sgmt, ZoneTable* ztable,
mem_sgmt.deallocate(ztable, sizeof(ZoneTable));
}
-result::Result
+ZoneTable::AddResult
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,12 +91,11 @@ 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);
+ return (AddResult(result::EXIST, old));
} else {
- return (result::SUCCESS);
+ return (AddResult(result::SUCCESS, NULL));
}
}
diff --git a/src/lib/datasrc/memory/zone_table.h b/src/lib/datasrc/memory/zone_table.h
index 4f309b7..024558e 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 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
@@ -78,6 +74,14 @@ private:
typedef DomainTreeNode<ZoneData> ZoneTableNode;
public:
+ /// \brief Result data of addZone() method.
+ struct AddResult {
+ AddResult(result::Result param_code, ZoneData* param_zone_data) :
+ code(param_code), zone_data(param_zone_data)
+ {}
+ const result::Result code;
+ ZoneData* const zone_data;
+ };
/// \brief Result data of findZone() method.
struct FindResult {
@@ -147,17 +151,18 @@ 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
- /// zone of the same origin. The old data is released and replaced
- /// by the new one.
- result::Result addZone(util::MemorySegment& mem_sgmt,
- dns::RRClass zone_class,
- const dns::Name& zone_name,
- detail::SegmentObjectHolder<ZoneData,
- isc::dns::RRClass>& content);
+ /// zone of the same origin. The old data is replaced and returned
+ /// inside the result.
+ AddResult addZone(util::MemorySegment& mem_sgmt,
+ dns::RRClass zone_class,
+ const dns::Name& zone_name,
+ ZoneData* content);
/// Find a zone that best matches the given name in the \c ZoneTable.
///
diff --git a/src/lib/datasrc/tests/memory/domaintree_unittest.cc b/src/lib/datasrc/tests/memory/domaintree_unittest.cc
index 81fa576..45e256a 100644
--- a/src/lib/datasrc/tests/memory/domaintree_unittest.cc
+++ b/src/lib/datasrc/tests/memory/domaintree_unittest.cc
@@ -411,10 +411,11 @@ performCallbackTest(TestDomainTree& dtree,
Name("example"),
&parentdtnode));
// the child/parent nodes shouldn't "inherit" the callback flag.
- // "cdtnode" may be invalid due to the insertion, so we need to re-find
- // it.
+ // "dtnode" should still validly point to "callback.example", but we
+ // explicitly confirm it.
EXPECT_EQ(TestDomainTree::EXACTMATCH, dtree.find(Name("callback.example"),
&cdtnode));
+ ASSERT_EQ(dtnode, cdtnode);
EXPECT_TRUE(cdtnode->getFlag(TestDomainTreeNode::FLAG_CALLBACK));
EXPECT_FALSE(subdtnode->getFlag(TestDomainTreeNode::FLAG_CALLBACK));
EXPECT_FALSE(parentdtnode->getFlag(TestDomainTreeNode::FLAG_CALLBACK));
diff --git a/src/lib/datasrc/tests/memory/zone_table_unittest.cc b/src/lib/datasrc/tests/memory/zone_table_unittest.cc
index b9ee3d9..9cf1b34 100644
--- a/src/lib/datasrc/tests/memory/zone_table_unittest.cc
+++ b/src/lib/datasrc/tests/memory/zone_table_unittest.cc
@@ -89,38 +89,56 @@ 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_);
+ const ZoneData* data1(holder1.get());
// Normal successful case.
- EXPECT_EQ(result::SUCCESS, zone_table->addZone(mem_sgmt_, zclass_,
- zname1, holder1));
+ const ZoneTable::AddResult result1(zone_table->addZone(mem_sgmt_, zclass_,
+ zname1,
+ holder1.release()));
+ EXPECT_EQ(result::SUCCESS, result1.code);
+ EXPECT_EQ(NULL, result1.zone_data);
// It got released by it
EXPECT_EQ(NULL, holder1.get());
// Duplicate add doesn't replace the existing data.
SegmentObjectHolder<ZoneData, RRClass> holder2(
mem_sgmt_, ZoneData::create(mem_sgmt_, zname1), zclass_);
- EXPECT_EQ(result::EXIST, zone_table->addZone(mem_sgmt_, zclass_,
- zname1, holder2));
+ const ZoneTable::AddResult result2(zone_table->addZone(mem_sgmt_, zclass_,
+ zname1,
+ holder2.release()));
+ EXPECT_EQ(result::EXIST, result2.code);
+ // The old one gets out
+ EXPECT_EQ(data1, result2.zone_data);
// It releases this one even when we replace the old zone
EXPECT_EQ(NULL, holder2.get());
+ // We need to release the old one manually
+ ZoneData::destroy(mem_sgmt_, result2.zone_data, zclass_);
SegmentObjectHolder<ZoneData, RRClass> holder3(
mem_sgmt_, ZoneData::create(mem_sgmt_, Name("EXAMPLE.COM")),
zclass_);
// names are compared in a case insensitive manner.
- EXPECT_EQ(result::EXIST, zone_table->addZone(mem_sgmt_, zclass_,
- Name("EXAMPLE.COM"),
- holder3));
+ const ZoneTable::AddResult result3(zone_table->addZone(mem_sgmt_, zclass_,
+ Name("EXAMPLE.COM"),
+ holder3.release()));
+ EXPECT_EQ(result::EXIST, result3.code);
+ ZoneData::destroy(mem_sgmt_, result3.zone_data, zclass_);
// 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()).code);
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()).code);
// 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 +147,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 +156,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()).code);
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()).code);
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()).code);
const ZoneTable::FindResult find_result1 =
zone_table->findZone(Name("example.com"));
@@ -170,7 +190,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()).code);
EXPECT_EQ(zone_data,
zone_table->findZone(Name("www.example.com")).zone_data);
}
More information about the bind10-changes
mailing list