BIND 10 trac2107, updated. 4bd4bf928475f7fe9181fc71f0d87546b7b8a760 [2107] clarified the ownership of DomainTree data.
BIND 10 source code commits
bind10-changes at lists.isc.org
Mon Aug 27 23:04:23 UTC 2012
The branch, trac2107 has been updated
via 4bd4bf928475f7fe9181fc71f0d87546b7b8a760 (commit)
via 8c9a5e0ce95663f27736cacf432c5ff6ece35a06 (commit)
via 5e463e914a5442a5ab72c1a7aed0d8eb103a7dda (commit)
via dd15647393e777419c3f0783ed4416cdfe907a82 (commit)
via 6e2de8ba7a4a1582802ba3a03b02e286ecaf50d1 (commit)
via 56de9af8c7a03c5daf92e7460191f6d95b1bf917 (commit)
from 4ecc4e3295bddf87f14f2e60fca98ceac05fbf1d (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 4bd4bf928475f7fe9181fc71f0d87546b7b8a760
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Mon Aug 27 15:57:52 2012 -0700
[2107] clarified the ownership of DomainTree data.
commit 8c9a5e0ce95663f27736cacf432c5ff6ece35a06
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Mon Aug 27 15:42:30 2012 -0700
[2107] fixed a use-after-free bug in rdataSetDeleter.
commit 5e463e914a5442a5ab72c1a7aed0d8eb103a7dda
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Mon Aug 27 15:28:41 2012 -0700
[2107] updated doc for MemorySegmentTest, with correction and clarification.
commit dd15647393e777419c3f0783ed4416cdfe907a82
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Mon Aug 27 15:15:18 2012 -0700
[2107] added explicit tests for SegmentObjectHolderTest.
commit 6e2de8ba7a4a1582802ba3a03b02e286ecaf50d1
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Mon Aug 27 14:46:13 2012 -0700
[2107] clearly separated mutable and immutable versions of getNext().
and tested for both versions.
commit 56de9af8c7a03c5daf92e7460191f6d95b1bf917
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Mon Aug 27 14:36:58 2012 -0700
[2107] add explicit tests for RdataSet::getNext()
-----------------------------------------------------------------------
Summary of changes:
src/lib/datasrc/memory/domaintree.h | 20 ++++--
src/lib/datasrc/memory/rdataset.h | 6 +-
src/lib/datasrc/memory/tests/Makefile.am | 1 +
src/lib/datasrc/memory/tests/memory_segment_test.h | 8 ++-
src/lib/datasrc/memory/tests/rdataset_unittest.cc | 19 ++++++
.../memory/tests/segment_object_holder_unittest.cc | 67 ++++++++++++++++++++
src/lib/datasrc/memory/zone_data.cc | 5 +-
7 files changed, 117 insertions(+), 9 deletions(-)
create mode 100644 src/lib/datasrc/memory/tests/segment_object_holder_unittest.cc
-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/memory/domaintree.h b/src/lib/datasrc/memory/domaintree.h
index bb7bbae..f5674dd 100644
--- a/src/lib/datasrc/memory/domaintree.h
+++ b/src/lib/datasrc/memory/domaintree.h
@@ -780,11 +780,21 @@ private:
* \brief \c DomainTree class represents all the domains with the same suffix.
* It can be used to store the domains in one zone, for example.
*
- * DomainTree is a generic map from domain names to any kind of
- * data. Internally, it uses a red-black tree. However, it isn't one
- * tree containing everything. Subdomains are trees, so this structure
- * is recursive - trees inside trees. But, from the interface point of
- * view, it is opaque data structure.
+ * DomainTree is a generic map from domain names to any kind of
+ * data. Internally, it uses a red-black tree. However, it isn't one
+ * tree containing everything. Subdomains are trees, so this structure
+ * is recursive - trees inside trees. But, from the interface point of
+ * view, it is opaque data structure.
+ *
+ * The data of DomainTree are set by the application via the
+ * DomainTreeNode::setData() method. The ownership of the data isn't
+ * transferred to the DomainTree; if the application replaces existing
+ * data for a specific name in DomainTree by setData(), the application is
+ * responsible for releasing any resource for the old data. When the
+ * application destroys the entire DomainTree by the \c destroy() method,
+ * it needs to pass a deleter object for any remained data in the DomainTree.
+ * The DomainTree will call that object with all the data in the tree so that
+ * the application complete the cleanup about the remaining data.
*
* \c DomainTree splits the domain space into hierarchy red black trees; nodes
* in one tree has the same base name. The benefit of this struct is that:
diff --git a/src/lib/datasrc/memory/rdataset.h b/src/lib/datasrc/memory/rdataset.h
index e7c15f6..69fb17d 100644
--- a/src/lib/datasrc/memory/rdataset.h
+++ b/src/lib/datasrc/memory/rdataset.h
@@ -237,7 +237,11 @@ public:
/// get the same result by directly calling get() on \c next, it would
/// help encourage the use of more efficient usage if we provide an
/// explicit accessor.
- RdataSet* getNext() const { return (next.get()); }
+ //const RdataSet* getNext() const { return (next.get()); }
+ const RdataSet* getNext() const { return (next.get()); }
+
+ /// \brief Return the bare pointer to the next node, mutable version.
+ RdataSet* getNext() { return (next.get()); }
/// \brief Return the number of RDATAs stored in the \c RdataSet.
size_t getRdataCount() const { return (rdata_count_); }
diff --git a/src/lib/datasrc/memory/tests/Makefile.am b/src/lib/datasrc/memory/tests/Makefile.am
index 6cc6de5..e210a4a 100644
--- a/src/lib/datasrc/memory/tests/Makefile.am
+++ b/src/lib/datasrc/memory/tests/Makefile.am
@@ -23,6 +23,7 @@ run_unittests_SOURCES += rdataset_unittest.cc
run_unittests_SOURCES += domaintree_unittest.cc
run_unittests_SOURCES += zone_data_unittest.cc
run_unittests_SOURCES += memory_segment_test.h
+run_unittests_SOURCES += segment_object_holder_unittest.cc
run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)
diff --git a/src/lib/datasrc/memory/tests/memory_segment_test.h b/src/lib/datasrc/memory/tests/memory_segment_test.h
index 78f4182..3195a9b 100644
--- a/src/lib/datasrc/memory/tests/memory_segment_test.h
+++ b/src/lib/datasrc/memory/tests/memory_segment_test.h
@@ -27,8 +27,12 @@ namespace test {
// A special memory segment that can be used for tests. It normally behaves
// like a "local" memory segment. If "throw count" is set to non 0 via
-// setThrowCount(), it continues the normal behavior up to the specified
-// number of calls to allocate(), and throws an exception at the next call.
+// setThrowCount(), it continues the normal behavior until the specified
+// number of calls to allocate(), exclusive, and throws an exception at the
+// next call. For example, if count is set to 3, the next two calls to
+// allocate() will succeed, and the 3rd call will fail with an exception.
+// This segment object can be used after the exception is thrown, and the
+// count is internally reset to 0.
class MemorySegmentTest : public isc::util::MemorySegmentLocal {
public:
MemorySegmentTest() : throw_count_(0) {}
diff --git a/src/lib/datasrc/memory/tests/rdataset_unittest.cc b/src/lib/datasrc/memory/tests/rdataset_unittest.cc
index 5e7d4b9..07a240c 100644
--- a/src/lib/datasrc/memory/tests/rdataset_unittest.cc
+++ b/src/lib/datasrc/memory/tests/rdataset_unittest.cc
@@ -116,6 +116,25 @@ TEST_F(RdataSetTest, create) {
RdataSet::destroy(mem_sgmt_, RRClass::IN(), rdataset);
}
+TEST_F(RdataSetTest, getNext) {
+ RdataSet* rdataset = RdataSet::create(mem_sgmt_, encoder_, a_rrset_,
+ ConstRRsetPtr());
+
+ // By default, the next pointer should be NULL (already tested in other
+ // test cases), which should be the case with getNext(). We test both
+ // mutable and immutable versions of getNext().
+ EXPECT_EQ(static_cast<RdataSet*>(NULL), rdataset->getNext());
+ EXPECT_EQ(static_cast<const RdataSet*>(NULL),
+ static_cast<const RdataSet*>(rdataset)->getNext());
+
+ // making a link (it would form an infinite loop, but it doesn't matter
+ // in this test), and check the pointer returned by getNext().
+ rdataset->next = rdataset;
+ EXPECT_EQ(rdataset, static_cast<const RdataSet*>(rdataset)->getNext());
+
+ RdataSet::destroy(mem_sgmt_, RRClass::IN(), rdataset);
+}
+
// A helper function to create an RRset containing the given number of
// unique RDATAs.
ConstRRsetPtr
diff --git a/src/lib/datasrc/memory/tests/segment_object_holder_unittest.cc b/src/lib/datasrc/memory/tests/segment_object_holder_unittest.cc
new file mode 100644
index 0000000..d27e364
--- /dev/null
+++ b/src/lib/datasrc/memory/tests/segment_object_holder_unittest.cc
@@ -0,0 +1,67 @@
+// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// 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 <datasrc/memory/segment_object_holder.h>
+
+#include <gtest/gtest.h>
+
+using namespace isc::util;
+using namespace isc::datasrc::memory;
+using namespace isc::datasrc::memory::detail;
+
+namespace {
+const int TEST_ARG_VAL = 42; // arbitrary chosen magic number
+
+class TestObject {
+public:
+ static void destroy(MemorySegment& sgmt, TestObject* obj, int arg) {
+ sgmt.deallocate(obj, sizeof(*obj));
+ EXPECT_EQ(TEST_ARG_VAL, arg);
+ }
+};
+
+void
+useHolder(MemorySegment& sgmt, TestObject* obj, bool release) {
+ // Create a holder object, check the return value of get(), and,
+ // if requested, release the held object. At the end of function
+ // the holder is destructed, and if the object hasn't been released by
+ // then, it should be deallocated. Passed argument is checked in its
+ // deallocate().
+
+ typedef SegmentObjectHolder<TestObject, int> HolderType;
+ HolderType holder(sgmt, obj, TEST_ARG_VAL);
+ EXPECT_EQ(obj, holder.get());
+ if (release) {
+ EXPECT_EQ(obj, holder.release());
+ }
+}
+
+TEST(SegmentObjectHolderTest, foo) {
+ MemorySegmentLocal sgmt;
+ void* p = sgmt.allocate(sizeof(TestObject));
+ TestObject* obj = new(p) TestObject;
+
+ // Use holder, and release the content. The memory shouldn't be
+ // deallocated.
+ useHolder(sgmt, obj, true);
+ EXPECT_FALSE(sgmt.allMemoryDeallocated());
+
+ // Use holder, and let it deallocate the object. The memory segment
+ // should now be empty.
+ useHolder(sgmt, obj, false);
+ EXPECT_TRUE(sgmt.allMemoryDeallocated());
+}
+}
diff --git a/src/lib/datasrc/memory/zone_data.cc b/src/lib/datasrc/memory/zone_data.cc
index 03eab04..3b083a4 100644
--- a/src/lib/datasrc/memory/zone_data.cc
+++ b/src/lib/datasrc/memory/zone_data.cc
@@ -42,9 +42,12 @@ void
rdataSetDeleter(RRClass rrclass, util::MemorySegment* mem_sgmt,
RdataSet* rdataset_head)
{
+ RdataSet* rdataset_next;
for (RdataSet* rdataset = rdataset_head;
rdataset != NULL;
- rdataset = rdataset->getNext()) {
+ rdataset = rdataset_next)
+ {
+ rdataset_next = rdataset->getNext();
RdataSet::destroy(*mem_sgmt, rrclass, rdataset);
}
}
More information about the bind10-changes
mailing list