BIND 10 trac2107, updated. 558b2dbe02c6de2cac63ef44e1eeab12600aa99a [2107] have DomainTreeNode::setData() return pointer to old data.
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Aug 29 00:10:51 UTC 2012
The branch, trac2107 has been updated
via 558b2dbe02c6de2cac63ef44e1eeab12600aa99a (commit)
via 7985ae6830540ce6f6f6c3d6d71b2876c4e7315e (commit)
via 44f92aeb7817fec9534698ac659d898d1a35acbb (commit)
via 9ebbd0cc6b9d20b1b67bf8d7161ba4b3239d5482 (commit)
via 870aa95f8b1c608468021d7666f24078cc78d5be (commit)
via 78eeb8706770caf3f25c166b0413c9a324be1de3 (commit)
via ee81b663d85fbc1a8f74e8206a1da8e0a54d604a (commit)
via 8052c97a801dd4d46241ce48744b0f2a69edd801 (commit)
from 7443b1d25391782d488ae94f3d36dd65f54d948f (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 558b2dbe02c6de2cac63ef44e1eeab12600aa99a
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Tue Aug 28 15:20:40 2012 -0700
[2107] have DomainTreeNode::setData() return pointer to old data.
added some test cases to confirm the behavior; update the doc.
commit 7985ae6830540ce6f6f6c3d6d71b2876c4e7315e
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Tue Aug 28 15:01:36 2012 -0700
[2107] define "find rdataset" method as a static method of the class.
with notes about the intent.
commit 44f92aeb7817fec9534698ac659d898d1a35acbb
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Tue Aug 28 14:34:20 2012 -0700
[2107] renamed exception-safety tests for ZoneDataTest with more comments.
hopefully the new name better represents the intent.
commit 9ebbd0cc6b9d20b1b67bf8d7161ba4b3239d5482
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Tue Aug 28 14:32:01 2012 -0700
[2107] clarified getter/for-modify methods in ZoneData doc.
commit 870aa95f8b1c608468021d7666f24078cc78d5be
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Tue Aug 28 14:15:48 2012 -0700
[2107] doc update: more explicit about modify method of NSEC3Data.
commit 78eeb8706770caf3f25c166b0413c9a324be1de3
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Tue Aug 28 14:10:30 2012 -0700
[2107] s/WILD_NODE/WILDCARD_NODE/ as suggested in the review
commit ee81b663d85fbc1a8f74e8206a1da8e0a54d604a
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Tue Aug 28 14:08:17 2012 -0700
[2107] editorial cleanup: removed a leftover comment-out'ed line
commit 8052c97a801dd4d46241ce48744b0f2a69edd801
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Tue Aug 28 14:06:51 2012 -0700
[2107] minor editorial update: make sure \c is in the same line as its target.
not sure if it's absolute necessary for doxygen to process it, but I thought
it's more readable anyway.
-----------------------------------------------------------------------
Summary of changes:
src/lib/datasrc/memory/domaintree.h | 21 +++--
src/lib/datasrc/memory/rdataset.h | 92 +++++++++++---------
.../datasrc/memory/tests/domaintree_unittest.cc | 30 +++++--
src/lib/datasrc/memory/tests/zone_data_unittest.cc | 8 +-
src/lib/datasrc/memory/zone_data.h | 32 ++++---
5 files changed, 115 insertions(+), 68 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/memory/domaintree.h b/src/lib/datasrc/memory/domaintree.h
index e7170e5..ae2f1b0 100644
--- a/src/lib/datasrc/memory/domaintree.h
+++ b/src/lib/datasrc/memory/domaintree.h
@@ -248,13 +248,22 @@ public:
/// \brief Set the data stored in the node.
///
- /// If there is old data, it will be simply dropped; unless the data
- /// is managed outside the node and its resource is released (if needed),
- /// it will leak.
- ///
- /// \param data The new data to set.
- void setData(T* data) {
+ /// If there is old data, a pointer to the data will be returned;
+ /// otherwise NULL will be returned. The caller is responsible for
+ /// releasing any resource for the old data if it's not needed any more.
+ /// See also the note about data ownership in the \c DomainTree
+ /// description.
+ ///
+ /// \c data can be NULL, in which case it effectively clears any existing
+ /// old data.
+ ///
+ /// \param data The new data to set. It can be NULL.
+ /// \return A pointer to the old data or NULL if the node doesn't have
+ /// data.
+ T* setData(T* data) {
+ T* olddata = data_.get();
data_ = data;
+ return (olddata);
}
//@}
diff --git a/src/lib/datasrc/memory/rdataset.h b/src/lib/datasrc/memory/rdataset.h
index 69fb17d..b0b3b48 100644
--- a/src/lib/datasrc/memory/rdataset.h
+++ b/src/lib/datasrc/memory/rdataset.h
@@ -194,6 +194,43 @@ public:
static void destroy(util::MemorySegment& mem_sgmt, dns::RRClass rrclass,
RdataSet* rdataset);
+ /// \brief Find \c RdataSet of given RR type from a list (const version).
+ ///
+ /// This function is a convenient shortcut for commonly used operation of
+ /// finding a given type of \c RdataSet from a linked list of them.
+ ///
+ /// It follows the linked list of \c RdataSet objects (via their \c next
+ /// member) starting the given head, until it finds an object of the
+ /// given RR type. If found, it returns a (bare) pointer to the object;
+ /// if not found in the entire list, it returns NULL. The head pointer
+ /// can be NULL, in which case this function will simply return NULL.
+ ///
+ /// \note This function is defined as a (static) class method to
+ /// clarify its an operation for \c RdataSet objects and to make the
+ /// name shorter. But its implementation does not depend on private
+ /// members of the class, and it should be kept if and when this method
+ /// needs to be extended, unless there's a reason other than simply
+ /// because it's already a member function.
+ ///
+ /// \param rdata_head A pointer to \c RdataSet from which the search
+ /// starts. It can be NULL.
+ /// \param type The RRType of \c RdataSet to find.
+ /// \return A pointer to the found \c RdataSet or NULL if none found.
+ static const RdataSet*
+ find(const RdataSet* rdataset_head, const dns::RRType& type) {
+ return (find<const RdataSet>(rdataset_head, type));
+ }
+
+ /// \brief Find \c RdataSet of given RR type from a list (non const
+ /// version).
+ ///
+ /// This is similar to the const version, except it takes and returns non
+ /// const pointers.
+ static RdataSet*
+ find(RdataSet* rdataset_head, const dns::RRType& type) {
+ return (find<RdataSet>(rdataset_head, type));
+ }
+
typedef boost::interprocess::offset_ptr<RdataSet> RdataSetPtr;
typedef boost::interprocess::offset_ptr<const RdataSet> ConstRdataSetPtr;
@@ -237,7 +274,6 @@ 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.
- //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.
@@ -308,6 +344,21 @@ private:
return (reinterpret_cast<uint16_t*>(this + 1));
}
+ // Shared by both mutable and immutable versions of find()
+ template <typename RdataSetType>
+ static RdataSetType*
+ find(RdataSetType* rdataset_head, const dns::RRType& type) {
+ for (RdataSetType* rdataset = rdataset_head;
+ rdataset != NULL;
+ rdataset = rdataset->getNext()) // use getNext() for efficiency
+ {
+ if (rdataset->type == type) {
+ return (rdataset);
+ }
+ }
+ return (NULL);
+ }
+
/// \brief The constructor.
///
/// An object of this class is always expected to be created by the
@@ -327,45 +378,6 @@ private:
~RdataSet() {}
};
-// Shared by both mutable and immutable versions below
-template <typename RdataSetType>
-RdataSetType*
-findRdataSetOfType(RdataSetType* rdataset_head, dns::RRType type) {
- for (RdataSetType* rdataset = rdataset_head;
- rdataset != NULL;
- rdataset = rdataset->getNext()) // use getNext() for efficiency
- {
- if (rdataset->type == type) {
- return (rdataset);
- }
- }
- return (NULL);
-}
-
-/// \brief Find \c RdataSet of given RR type from a list (const version).
-///
-/// This function is a convenient shortcut for commonly used operation of
-/// finding a given type of \c RdataSet from a linked list of them.
-///
-/// It follows the linked list of \c RdataSet objects (via their \c next
-/// member) starting the given head, until it finds an object of the
-/// given RR type. If found, it returns a (bare) pointer to the object;
-/// if not found in the entire list, it returns NULL. The head pointer
-/// can be NULL, in which case this function will simply return NULL.
-inline const RdataSet*
-findRdataSetOfType(const RdataSet* rdataset_head, dns::RRType type) {
- return (findRdataSetOfType<const RdataSet>(rdataset_head, type));
-}
-
-/// \brief Find \c RdataSet of given RR type from a list (non const version).
-///
-/// This is similar to the const version, except it takes and returns non
-/// const pointers.
-inline RdataSet*
-findRdataSetOfType(RdataSet* rdataset_head, dns::RRType type) {
- return (findRdataSetOfType<RdataSet>(rdataset_head, type));
-}
-
} // namespace memory
} // namespace datasrc
} // namespace isc
diff --git a/src/lib/datasrc/memory/tests/domaintree_unittest.cc b/src/lib/datasrc/memory/tests/domaintree_unittest.cc
index 21d6dbb..3a0fdd7 100644
--- a/src/lib/datasrc/memory/tests/domaintree_unittest.cc
+++ b/src/lib/datasrc/memory/tests/domaintree_unittest.cc
@@ -97,12 +97,13 @@ protected:
int name_count = sizeof(domain_names) / sizeof(domain_names[0]);
for (int i = 0; i < name_count; ++i) {
dtree.insert(mem_sgmt_, Name(domain_names[i]), &dtnode);
- dtnode->setData(new int(i + 1));
+ // Check the node doesn't have any data initially.
+ EXPECT_EQ(static_cast<int*>(NULL),
+ dtnode->setData(new int(i + 1)));
dtree_expose_empty_node.insert(mem_sgmt_, Name(domain_names[i]),
&dtnode);
- dtnode->setData(new int(i + 1));
-
+ EXPECT_EQ(static_cast<int*>(NULL), dtnode->setData(new int(i + 1)));
}
}
@@ -125,8 +126,17 @@ TEST_F(DomainTreeTest, nodeCount) {
}
TEST_F(DomainTreeTest, setGetData) {
- dtnode->setData(new int(11));
+ // set new data to an existing node. It should have some data.
+ int* newdata = new int(11);
+ int* olddata = dtnode->setData(newdata);
+ EXPECT_NE(static_cast<int*>(NULL), olddata);
+ deleteData(olddata);
EXPECT_EQ(11, *(dtnode->getData()));
+
+ // clear the node. we should get the new data back we just passed.
+ olddata = dtnode->setData(NULL);
+ EXPECT_EQ(newdata, olddata);
+ deleteData(olddata);
}
TEST_F(DomainTreeTest, insertNames) {
@@ -146,7 +156,9 @@ TEST_F(DomainTreeTest, insertNames) {
Name("example.com"),
&dtnode));
EXPECT_EQ(17, dtree.getNodeCount());
- dtnode->setData(new int(12));
+ // ad data to it; also make sure it doesn't have data right now
+ // (otherwise it would leak)
+ EXPECT_EQ(static_cast<int*>(NULL), dtnode->setData(new int(12)));
// return ALREADYEXISTS, since node "example.com" already has
// been explicitly inserted
@@ -376,7 +388,7 @@ performCallbackTest(TestDomainTree& dtree,
EXPECT_EQ(TestDomainTree::SUCCESS, dtree.insert(mem_sgmt,
Name("callback.example"),
&dtnode));
- dtnode->setData(new int(1));
+ EXPECT_EQ(static_cast<int*>(NULL), dtnode->setData(new int(1)));
EXPECT_FALSE(dtnode->getFlag(TestDomainTreeNode::FLAG_CALLBACK));
// enable/re-disable callback
@@ -392,7 +404,7 @@ performCallbackTest(TestDomainTree& dtree,
EXPECT_EQ(TestDomainTree::SUCCESS, dtree.insert(mem_sgmt,
Name("sub.callback.example"),
&subdtnode));
- subdtnode->setData(new int(2));
+ EXPECT_EQ(static_cast<int*>(NULL), subdtnode->setData(new int(2)));
TestDomainTreeNode* parentdtnode;
EXPECT_EQ(TestDomainTree::ALREADYEXISTS, dtree.insert(mem_sgmt,
Name("example"),
@@ -992,7 +1004,7 @@ TEST_F(DomainTreeTest, root) {
TreeHolder tree_holder(mem_sgmt_, TestDomainTree::create(mem_sgmt_));
TestDomainTree& root(*tree_holder.get());
root.insert(mem_sgmt_, Name::ROOT_NAME(), &dtnode);
- dtnode->setData(new int(1));
+ EXPECT_EQ(static_cast<int*>(NULL), dtnode->setData(new int(1)));
EXPECT_EQ(TestDomainTree::EXACTMATCH,
root.find(Name::ROOT_NAME(), &cdtnode));
@@ -1004,7 +1016,7 @@ TEST_F(DomainTreeTest, root) {
// Insert a new name that better matches the query name. find() should
// find the better one.
root.insert(mem_sgmt_, Name("com"), &dtnode);
- dtnode->setData(new int(2));
+ EXPECT_EQ(static_cast<int*>(NULL), dtnode->setData(new int(2)));
EXPECT_EQ(TestDomainTree::PARTIALMATCH,
root.find(Name("example.com"), &cdtnode));
EXPECT_EQ(dtnode, cdtnode);
diff --git a/src/lib/datasrc/memory/tests/zone_data_unittest.cc b/src/lib/datasrc/memory/tests/zone_data_unittest.cc
index c81801c..d2844ea 100644
--- a/src/lib/datasrc/memory/tests/zone_data_unittest.cc
+++ b/src/lib/datasrc/memory/tests/zone_data_unittest.cc
@@ -111,7 +111,7 @@ checkFindRdataSet(const ZoneTree& tree, const Name& name, RRType type,
ZoneNode* node = NULL;
tree.find(name, &node);
ASSERT_NE(static_cast<ZoneNode*>(NULL), node);
- EXPECT_EQ(expected_set, findRdataSetOfType(node->getData(), type));
+ EXPECT_EQ(expected_set, RdataSet::find(node->getData(), type));
}
TEST_F(ZoneDataTest, createNSEC3Data) {
@@ -150,10 +150,12 @@ TEST_F(ZoneDataTest, getOriginNode) {
EXPECT_EQ(LabelSequence(zname_), zone_data_->getOriginNode()->getLabels());
}
-TEST_F(ZoneDataTest, throwOnCreate) {
+TEST_F(ZoneDataTest, exceptionSafetyOnCreate) {
// Note: below, we use our knowledge of how memory allocation happens
// within the NSEC3Data, the zone data and the underlying domain tree
- // implementation.
+ // implementation. We'll emulate rare situations where allocate() fails
+ // with an exception, and confirm it doesn't cause any harsh disruption
+ // or leak.
// Creating internal NSEC3 tree will succeed, but allocation of NSEC3Data
// will fail due to bad_alloc. It shouldn't cause memory leak
diff --git a/src/lib/datasrc/memory/zone_data.h b/src/lib/datasrc/memory/zone_data.h
index 044289d..f592680 100644
--- a/src/lib/datasrc/memory/zone_data.h
+++ b/src/lib/datasrc/memory/zone_data.h
@@ -61,10 +61,12 @@ typedef DomainTreeNode<RdataSet> ZoneNode;
/// this condition. It's the caller's responsibility.
///
/// Read-only access to the tree is possible via the \c getNSEC3Tree() method.
-/// Modifying the tree must be done by specific method; the application
-/// cannot directly change the content of the tree in an arbitrary way.
-/// This class does not have a strong reason to be that strict, but is
-/// defined this way mainly to be consistent with the \c ZoneData class.
+/// Modifying the tree must be done by a specific method (in the initial
+/// implementation, it's \c insertName(). There may be some more as we
+/// see the need); the application cannot directly change the content of the
+/// tree in an arbitrary way. This class does not have a strong reason to be
+/// that strict, but is defined this way mainly to be consistent with the
+/// \c ZoneData class.
///
/// Most of the hash parameters are maintained in the form of straightforward
/// member variables, which can be directly referenced by the application.
@@ -117,9 +119,9 @@ public:
/// It's necessary to destroy the stored \c RdataSet objects
/// (see its class description). This class doesn't hold this information;
/// it's the caller's responsibility to associate an \c NSEC3Data
- /// class object with its expected RR class, and pass it to \c
- /// destroy(). (In practice, it will be passed via \c
- /// ZoneData::destroy().)
+ /// class object with its expected RR class, and pass it to
+ /// \c destroy(). (In practice, it will be passed via
+ /// \c ZoneData::destroy().)
///
/// \throw none
///
@@ -354,7 +356,7 @@ public:
/// \brief Node flag indicating it is at a "wildcard level"
///
/// This means one of the node's immediate children is a wildcard.
- static const ZoneNode::Flags WILD_NODE = ZoneNode::FLAG_USER2;
+ static const ZoneNode::Flags WILDCARD_NODE = ZoneNode::FLAG_USER2;
public:
/// \brief Allocate and construct \c ZoneData.
@@ -378,8 +380,8 @@ public:
/// Note that an \c RRClass object must be passed to this method.
/// It's used to destroy the stored \c RdataSet objects
/// (see its class description). This class doesn't hold this information;
- /// it's the caller's responsibility to associate a \c ZoneData class object
- /// with its expected RR class, and pass it to \c destroy().
+ /// it's the caller's responsibility to associate a \c ZoneData class
+ /// object with its expected RR class, and pass it to \c destroy().
///
/// \throw none
///
@@ -393,6 +395,10 @@ public:
static void destroy(util::MemorySegment& mem_sgmt, ZoneData* zone_data,
dns::RRClass zone_class);
+ ///
+ /// \name Getter methods
+ ///
+ //@{
/// \brief Return zone's origin node.
///
/// This is a convenience and efficient short cut to get access to the
@@ -443,7 +449,12 @@ public:
///
/// \throw none
const NSEC3Data* getNSEC3Data() const { return (nsec3_data_.get()); }
+ //@}
+ ///
+ /// \name Methods for modifying the tree
+ ///
+ //@{
/// \brief Insert a name to the zone.
///
/// It allocates resource for the given name in the internal storage
@@ -534,6 +545,7 @@ public:
nsec3_data_ = nsec3_data;
return (old);
}
+ //@}
private:
const boost::interprocess::offset_ptr<ZoneTree> zone_tree_;
More information about the bind10-changes
mailing list