[2091b] Merge branch 'trac2091a' into trac2091b with fixing conflicts.

src/lib/datasrc/memory/rdata_encoder.cc

@@ -308,18 +308,10 @@ encodeRdata(const rdata::Rdata& rdata, RRClass rrclass, RRType rrtype,
         {
             const Name name(ibuffer);
             const LabelSequence labels(name);
-            size_t nlen;
-            const uint8_t* ndata = labels.getData(&nlen);
-            assert(nlen < 256); // nlen should fit in 8 bits
-            size_t olen;
-            uint8_t offset_holder[Name::MAX_LABELS];
-            labels.getOffsetData(&olen, offset_holder);
-            assert(olen < 256); // olen should fit in 8 bits
-            data_result.push_back(nlen);
-            data_result.push_back(olen);
-            data_result.insert(data_result.end(), ndata, ndata + nlen);
-            data_result.insert(data_result.end(), offset_holder,
-                               offset_holder + olen);
+            uint8_t labels_holder[LabelSequence::MAX_SERIALIZED_LENGTH];
+            labels.serialize(labels_holder, sizeof(labels_holder));
+            data_result.insert(data_result.end(), labels_holder,
+                               labels_holder + labels.getSerializedLength());
             break;
         }
         }
@@ -359,15 +351,11 @@ foreachRdataField(RRClass rrclass, RRType rrtype,
         case RdataFieldSpec::DOMAIN_NAME:
         {
             ++name_count;
-            const uint8_t nlen = encoded_data.at(off);
-            const uint8_t olen = encoded_data.at(off + 1);
+            const LabelSequence labels(&encoded_data.at(off));
             if (name_callback) {
-                const uint8_t* ndata = &encoded_data.at(off + 2);
-                const uint8_t* odata = &encoded_data.at(off + 2 + nlen);
-                name_callback(LabelSequence(ndata, odata, olen),
-                              field_spec.name_attributes);
+                name_callback(labels, field_spec.name_attributes);
             }
-            off += (2 + nlen + olen);
+            off += labels.getSerializedLength();
             break;
         }
         }

src/lib/datasrc/memory_datasrc.cc

@@ -14,6 +14,8 @@
 
 #include <exceptions/exceptions.h>
 
+#include <util/memory_segment_local.h>
+
 #include <dns/name.h>
 #include <dns/nsec3hash.h>
 #include <dns/rdataclass.h>
@@ -120,19 +122,48 @@ typedef NSEC3Map::value_type NSEC3Pair;
 // Actual zone data: Essentially a set of zone's RRs.  This is defined as
 // a separate structure so that it'll be replaceable on reload.
 struct ZoneData {
+    // Note: this code is not entirely exception safe; domains_storage_ could
+    // leak if the constructor throws.  But since it's an intermediate version
+    // toward a full revision and the actual risk of leak should be very small
+    // in practice, we leave it open for now.
     ZoneData(const Name& origin) :
-        domains_(true),
+        domains_storage_(DomainTree::create(local_mem_sgmt_, true)),
+        domains_(*domains_storage_),
+        aux_wild_domains_(NULL),
         origin_data_(NULL),
         nsec_signed_(false)
     {
         // We create the node for origin (it needs to exist anyway in future)
-        domains_.insert(origin, &origin_data_);
+        domains_.insert(local_mem_sgmt_, origin, &origin_data_);
         DomainPtr origin_domain(new Domain);
         origin_data_->setData(origin_domain);
     }
 
-    // The main data (name + RRsets)
-    DomainTree domains_;
+    ~ZoneData() {
+        DomainTree::destroy(local_mem_sgmt_, domains_storage_);
+        if (aux_wild_domains_ != NULL) {
+            DomainTree::destroy(local_mem_sgmt_, aux_wild_domains_);
+        }
+
+        // The assert may be too harsh, but we assume we'll discard (rewrite)
+        // this code soon enough.  Until then this would be a good way to
+        // detect any memory leak.  Also, at that point we shouldn't use
+        // a single separate memory segment for each zone tree; normally
+        // zone data for multiple zones will be managed in a single segment.
+        assert(local_mem_sgmt_.allMemoryDeallocated());
+    }
+
+    // Memory segment to allocate/deallocate memory for the tree and the nodes.
+    // (This will eventually have to be abstract; for now we hardcode the
+    // specific derived segment class).
+    util::MemorySegmentLocal local_mem_sgmt_;
+
+    // The main data (name + RRsets).  We use domains_ as a reference to
+    // domains_storage_ so we don't have to update the rest of the code;
+    // it will eventually have to be revised substantially, at which point
+    // we should clean this up, too.
+    DomainTree* domains_storage_;
+    DomainTree& domains_;
 
     // An auxiliary tree for wildcard expanded data used in additional data
     // processing.  It contains names like "ns.wild.example" in the following
@@ -150,11 +181,11 @@ struct ZoneData {
     // should be even empty, and even if it has content it should be very
     // small.
 private:
-    scoped_ptr<DomainTree> aux_wild_domains_;
+    DomainTree* aux_wild_domains_;
 public:
     DomainTree& getAuxWildDomains() {
-        if (!aux_wild_domains_) {
-            aux_wild_domains_.reset(new DomainTree);
+        if (aux_wild_domains_ == NULL) {
+            aux_wild_domains_ = DomainTree::create(local_mem_sgmt_);
         }
         return (*aux_wild_domains_);
     }
@@ -857,7 +888,9 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
     //
     // We also perform the same trick for empty wild card names possibly
     // contained in 'name' (e.g., '*.foo.example' in 'bar.*.foo.example').
-    void addWildcards(DomainTree& domains, const Name& name) {
+    void addWildcards(util::MemorySegment& mem_sgmt, DomainTree& domains,
+                      const Name& name)
+    {
         Name wname(name);
         const unsigned int labels(wname.getLabelCount());
         const unsigned int origin_labels(origin_.getLabelCount());
@@ -870,7 +903,8 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
                 // Ensure a separate level exists for the "wildcarding" name,
                 // and mark the node as "wild".
                 DomainNode* node;
-                DomainTree::Result result(domains.insert(wname.split(1),
+                DomainTree::Result result(domains.insert(mem_sgmt,
+                                                         wname.split(1),
                                                          &node));
                 assert(result == DomainTree::SUCCESS ||
                        result == DomainTree::ALREADYEXISTS);
@@ -880,7 +914,7 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
                 // Note: for 'name' itself we do this later anyway, but the
                 // overhead should be marginal because wildcard names should
                 // be rare.
-                result = domains.insert(wname, &node);
+                result = domains.insert(mem_sgmt, wname, &node);
                 assert(result == DomainTree::SUCCESS ||
                        result == DomainTree::ALREADYEXISTS);
             }
@@ -1167,12 +1201,14 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
         // tree.
         // Note: this can throw an exception, breaking strong exception
         // guarantee.  (see also the note for contextCheck() below).
-        addWildcards(zone_data.domains_, rrset->getName());
+        addWildcards(zone_data.local_mem_sgmt_, zone_data.domains_,
+                     rrset->getName());
 
         // Get the node
         DomainNode* node;
-        DomainTree::Result result = zone_data.domains_.insert(rrset->getName(),
-                                                              &node);
+        DomainTree::Result result =
+            zone_data.domains_.insert(zone_data.local_mem_sgmt_,
+                                      rrset->getName(), &node);
         // Just check it returns reasonable results
         assert((result == DomainTree::SUCCESS ||
                 result == DomainTree::ALREADYEXISTS) && node!= NULL);
@@ -1605,7 +1641,8 @@ addAdditional(RBNodeRRset* rrset, ZoneData* zone_data,
             // Wildcard and glue shouldn't coexist.  Make it sure here.
             assert(!node->getFlag(domain_flag::GLUE));
 
-            if (zone_data->getAuxWildDomains().insert(name, &wildnode)
+            if (zone_data->getAuxWildDomains().insert(
+                    zone_data->local_mem_sgmt_, name, &wildnode)
                 == DomainTree::SUCCESS) {
                 // If we first insert the node, copy the RRsets.  If the
                 // original node was empty, we add empty data so
@@ -1782,9 +1819,22 @@ InMemoryZoneFinder::getFileName() const {
 /// member variables later for new features.
 class InMemoryClient::InMemoryClientImpl {
 public:
-    InMemoryClientImpl() : zone_count(0) {}
+    InMemoryClientImpl() : zone_count(0),
+                           zone_table(ZoneTable::create(local_mem_sgmt))
+    {}
+    ~InMemoryClientImpl() {
+        ZoneTable::destroy(local_mem_sgmt, zone_table);
+
+        // see above for the assert().
+        assert(local_mem_sgmt.allMemoryDeallocated());
+    }
+
+    // Memory segment to allocate/deallocate memory for the zone table.
+    // (This will eventually have to be abstract; for now we hardcode the
+    // specific derived segment class).
+    util::MemorySegmentLocal local_mem_sgmt;
     unsigned int zone_count;
-    ZoneTable zone_table;
+    ZoneTable* zone_table;
 };
 
 InMemoryClient::InMemoryClient() : impl_(new InMemoryClientImpl)
@@ -1809,7 +1859,8 @@ InMemoryClient::addZone(ZoneFinderPtr zone_finder) {
     LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEM_ADD_ZONE).
         arg(zone_finder->getOrigin()).arg(zone_finder->getClass().toText());
 
-    const result::Result result = impl_->zone_table.addZone(zone_finder);
+    const result::Result result =
+        impl_->zone_table->addZone(impl_->local_mem_sgmt, zone_finder);
     if (result == result::SUCCESS) {
         ++impl_->zone_count;
     }
@@ -1819,7 +1870,7 @@ InMemoryClient::addZone(ZoneFinderPtr zone_finder) {
 InMemoryClient::FindResult
 InMemoryClient::findZone(const isc::dns::Name& name) const {
     LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_FIND_ZONE).arg(name);
-    ZoneTable::FindResult result(impl_->zone_table.findZone(name));
+    ZoneTable::FindResult result(impl_->zone_table->findZone(name));
     return (FindResult(result.code, result.zone));
 }
 
@@ -1925,7 +1976,7 @@ public:
 
 ZoneIteratorPtr
 InMemoryClient::getIterator(const Name& name, bool separate_rrs) const {
-    ZoneTable::FindResult result(impl_->zone_table.findZone(name));
+    ZoneTable::FindResult result(impl_->zone_table->findZone(name));
     if (result.code != result::SUCCESS) {
         isc_throw(DataSourceError, "No such zone: " + name.toText());
     }

src/lib/datasrc/rbtree.h

@@ -24,12 +24,13 @@
 ///     to be used as a base data structure by other modules.
 
 #include <exceptions/exceptions.h>
-
+#include <util/memory_segment.h>
 #include <dns/name.h>
+
 #include <boost/utility.hpp>
 #include <boost/shared_ptr.hpp>
 #include <boost/interprocess/offset_ptr.hpp>
-#include <exceptions/exceptions.h>
+
 #include <ostream>
 #include <algorithm>
 #include <cassert>
@@ -113,8 +114,42 @@ private:
     /// \param name The *relative* domain name (if this will live inside
     ///     a.b.c and is called d.e.a.b.c, then you pass d.e).
     RBNode(const isc::dns::Name& name);
+
+    /// \brief Destructor
+    ~RBNode();
     //@}
 
+    /// \brief Allocate and construct \c RBNode
+    ///
+    /// This static method allocates memory for a new \c RBNode object
+    /// from the given memory segment, constructs the object, and returns
+    /// a pointer to it.
+    ///
+    /// \throw std::bad_alloc Memory allocation fails.
+    ///
+    /// \param mem_sgmt A \c MemorySegment from which memory for the new
+    /// \c RBNode is allocated.
+    static RBNode<T>* create(util::MemorySegment& mem_sgmt,
+                             const dns::Name& name)
+    {
+        void* p = mem_sgmt.allocate(sizeof(RBNode<T>));
+        return (new(p) RBNode<T>(name));
+    }
+
+    /// \brief Destruct and deallocate \c RBNode
+    ///
+    /// \throw none
+    ///
+    /// \param mem_sgmt The \c MemorySegment that allocated memory for
+    /// \c rbnode.
+    /// \param ztable A non NULL pointer to a valid \c RBNode object
+    /// that was originally created by the \c create() method (the behavior
+    /// is undefined if this condition isn't met).
+    static void destroy(util::MemorySegment& mem_sgmt, RBNode<T>* rbnode) {
+        rbnode->~RBNode<T>();
+        mem_sgmt.deallocate(rbnode, sizeof(RBNode<T>));
+    }
+
 public:
     /// \brief Alias for shared pointer to the data.
     typedef boost::shared_ptr<T> NodeDataPtr;
@@ -147,17 +182,6 @@ private:
 
 public:
 
-    /// \brief Destructor
-    ///
-    /// It might seem strange that constructors are private and destructor
-    /// public, but this is needed because of shared pointers need access
-    /// to the destructor.
-    ///
-    /// You should never call anything like:
-    /// \code delete pointer_to_node; \endcode
-    /// The RBTree handles both creation and destructoion of nodes.
-    ~RBNode();
-
     /// \name Getter functions.
     //@{
     /// \brief Return the name of current node.
@@ -759,18 +783,77 @@ public:
         ALREADYEXISTS,
     };
 
+    /// \brief Allocate and construct \c RBTree
+    ///
+    /// This static method allocates memory for a new \c RBTree object
+    /// from the given memory segment, constructs the object, and returns
+    /// a pointer to it.
+    ///
+    /// \throw std::bad_alloc Memory allocation fails.
+    ///
+    /// \param mem_sgmt A \c MemorySegment from which memory for the new
+    /// \c RBTree is allocated.
+    static RBTree* create(util::MemorySegment& mem_sgmt,
+                          bool return_empty_node = false)
+    {
+        void* p = mem_sgmt.allocate(sizeof(RBTree<T>));
+        return (new(p) RBTree<T>(return_empty_node));
+    }
+
+    /// \brief Destruct and deallocate \c RBTree
+    ///
+    /// This method also destroys and deallocates all nodes inserted to the
+    /// tree.
+    ///
+    /// \note The memory segment (\c mem_sgmt) must be the same one that
+    /// was originally used to allocate memory for the tree (and for all
+    /// nodes inserted to the tree, due to the requirement of \c insert()),
+    /// since the tree itself doesn't maintain a reference to the segment.
+    /// This is not a robust interface, but since we plan to share the tree
+    /// structure by multiple processes via shared memory or possibly allow
+    /// the memory image to be dumped to a file for later reload, there
+    /// doesn't seem to be an easy way to store such reference in the data
+    /// itself.  We should probably consider a wrapper interface that
+    /// encapsulates the corresponding segment and always use it for any
+    /// allocation/deallocation of tree related data (the tree itself, their
+    /// nodes, and node data) to keep the usage as safe as possible.
+    ///
+    /// \throw none
+    ///
+    /// \param mem_sgmt The \c MemorySegment that allocated memory for
+    /// \c rbtree and for all nodes inserted to the tree.
+    /// \param rbtree A non NULL pointer to a valid \c RBTree object
+    /// that was originally created by the \c create() method (the behavior
+    /// is undefined if this condition isn't met).
+    static void destroy(util::MemorySegment& mem_sgmt, RBTree<T>* rbtree) {
+        rbtree->deleteAllNodes(mem_sgmt);
+        rbtree->~RBTree<T>();
+        mem_sgmt.deallocate(rbtree, sizeof(RBTree<T>));
+    }
+
+private:
     /// \name Constructor and Destructor
     //@{
-    /// The constructor.
+    /// \brief The constructor.
+    ///
+    /// An object of this class is always expected to be created by the
+    /// allocator (\c create()), so the constructor is hidden as private.
     ///
     /// It never throws an exception.
     explicit RBTree(bool returnEmptyNode = false);
 
-    /// \b Note: RBTree is not intended to be inherited so the destructor
+    /// \brief The destructor.
+    ///
+    /// An object of this class is always expected to be destroyed explicitly
+    /// by \c destroy(), so the constructor is hidden as private.
+    ///
+    /// \note RBTree is not intended to be inherited so the destructor
     /// is not virtual
     ~RBTree();
     //@}
 
+public:
+
     /// \name Find methods
     ///
     /// \brief Find the node that gives a longest match against the given name.
@@ -1045,6 +1128,9 @@ public:
     /// the same.  This method provides the weak exception guarantee in its
     /// normal sense.
     ///
+    /// \param mem_sgmt A \c MemorySegment object for allocating memory of
+    /// a new node to be inserted.  Must be the same segment as that used
+    /// for creating the tree itself.
     /// \param name The name to be inserted into the tree.
     /// \param inserted_node This is an output parameter and is set to the
     ///     node.
@@ -1053,10 +1139,24 @@ public:
     ///  - SUCCESS The node was added.
     ///  - ALREADYEXISTS There was already a node of that name, so it was not
     ///     added.
-    Result insert(const isc::dns::Name& name, RBNode<T>** inserted_node);
+    Result insert(util::MemorySegment& mem_sgmt, const isc::dns::Name& name,
+                  RBNode<T>** inserted_node);
+
+    /// \brief Delete all tree nodes.
+    ///
+    /// \throw none.
+    ///
+    /// \param mem_sgmt The \c MemorySegment object used to insert the nodes
+    /// (which was also used for creating the tree due to the requirement of
+    /// \c inert()).
+    void deleteAllNodes(util::MemorySegment& mem_sgmt);
 
     /// \brief Swaps two tree's contents.
     ///
+    /// This and \c other trees must have been created with the same
+    /// memory segment (see the discussion in \c create()); otherwise the
+    /// behavior is undefined.
+    ///
     /// This acts the same as many std::*.swap functions, exchanges the
     /// contents. This doesn't throw anything.
     void swap(RBTree<T>& other) {
@@ -1079,7 +1179,7 @@ private:
     /// \name Helper functions
     //@{
     /// \brief delete tree whose root is equal to node
-    void deleteHelper(RBNode<T> *node);
+    void deleteHelper(util::MemorySegment& mem_sgmt, RBNode<T> *node);
 
     /// \brief Print the information of given RBNode.
     void dumpTreeHelper(std::ostream& os, const RBNode<T>* node,
@@ -1091,8 +1191,10 @@ private:
     /// Split one node into two nodes, keep the old node and create one new
     /// node, old node will hold the base name, new node will be the down node
     /// of old node, new node will hold the sub_name, the data
-    /// of old node will be move into new node, and old node became non-terminal
-    void nodeFission(RBNode<T>& node, const isc::dns::Name& sub_name);
+    /// of old node will be move into new node, and old node became
+    /// non-terminal
+    void nodeFission(util::MemorySegment& mem_sgmt, RBNode<T>& node,
+                     const isc::dns::Name& sub_name);
     //@}
 
     RBNode<T>* NULLNODE;
@@ -1114,13 +1216,12 @@ RBTree<T>::RBTree(bool returnEmptyNode) :
 
 template <typename T>
 RBTree<T>::~RBTree() {
-    deleteHelper(root_.get());
     assert(node_count_ == 0);
 }
 
 template <typename T>
 void
-RBTree<T>::deleteHelper(RBNode<T>* root) {
+RBTree<T>::deleteHelper(util::MemorySegment& mem_sgmt, RBNode<T>* root) {
     if (root == NULLNODE) {
         return;
     }
@@ -1141,14 +1242,14 @@ RBTree<T>::deleteHelper(RBNode<T>* root) {
             parent->right_ = NULLNODE;
         }
 
-        deleteHelper(node->getDown());
-        delete node;
+        deleteHelper(mem_sgmt, node->getDown());
+        RBNode<T>::destroy(mem_sgmt, node);
         --node_count_;
         node = parent;
     }
 
-    deleteHelper(root->getDown());
-    delete root;
+    deleteHelper(mem_sgmt, root->getDown());
+    RBNode<T>::destroy(mem_sgmt, root);
     --node_count_;
 }
 
@@ -1405,7 +1506,9 @@ RBTree<T>::previousNode(RBTreeNodeChain<T>& node_path) const {
 
 template <typename T>
 typename RBTree<T>::Result
-RBTree<T>::insert(const isc::dns::Name& target_name, RBNode<T>** new_node) {
+RBTree<T>::insert(util::MemorySegment& mem_sgmt,
+                  const isc::dns::Name& target_name, RBNode<T>** new_node)
+{
     using namespace helper;
     RBNode<T>* parent = NULLNODE;
     RBNode<T>* current = root_.get();
@@ -1446,7 +1549,7 @@ RBTree<T>::insert(const isc::dns::Name& target_name, RBNode<T>** new_node) {
                     const isc::dns::Name common_ancestor = name.split(
                         name.getLabelCount() - common_label_count,
                         common_label_count);
-                    nodeFission(*current, common_ancestor);
+                    nodeFission(mem_sgmt, *current, common_ancestor);
                 }
             }
         }
@@ -1454,34 +1557,37 @@ RBTree<T>::insert(const isc::dns::Name& target_name, RBNode<T>** new_node) {
 
     typename RBNode<T>::RBNodePtr* current_root = (up_node != NULLNODE) ?
         &(up_node->down_) : &root_;
-    // using auto_ptr here is avoid memory leak in case of exceptoin raised
-    // after the RBNode creation, if we can make sure no exception will be
-    // raised until the end of the function, we can remove it for optimization
-    std::auto_ptr<RBNode<T> > node(new RBNode<T>(name));
+    // Once a new node is created, no exception will be thrown until the end
+    // of the function, so we can simply create and hold a new node pointer.
+    RBNode<T>* node = RBNode<T>::create(mem_sgmt, name);
     node->parent_ = parent;
     if (parent == NULLNODE) {
-        *current_root = node.get();
-        //node is the new root of sub tree, so its init color
-        // is BLACK
+        *current_root = node;
+        // node is the new root of sub tree, so its init color is BLACK
         node->setColor(RBNode<T>::BLACK);
         node->setSubTreeRoot(true);
     } else if (order < 0) {
         node->setSubTreeRoot(false);
-        parent->left_ = node.get();
+        parent->left_ = node;
     } else {
         node->setSubTreeRoot(false);
-        parent->right_ = node.get();
+        parent->right_ = node;
     }
-    insertRebalance(current_root, node.get());
+    insertRebalance(current_root, node);
     if (new_node != NULL) {
-        *new_node = node.get();
+        *new_node = node;
     }
 
     ++node_count_;
-    node.release();
     return (SUCCESS);
 }
 
+template <typename T>
+void
+RBTree<T>::deleteAllNodes(util::MemorySegment& mem_sgmt) {
+    deleteHelper(mem_sgmt, root_.get());
+    root_ = NULLNODE;
+}
 
 // Note: when we redesign this (still keeping the basic concept), we should
 // change this part so the newly created node will be used for the inserted
@@ -1489,12 +1595,14 @@ RBTree<T>::insert(const isc::dns::Name& target_name, RBNode<T>** new_node) {
 // Otherwise, things like shortcut links between nodes won't work.
 template <typename T>
 void
-RBTree<T>::nodeFission(RBNode<T>& node, const isc::dns::Name& base_name) {
+RBTree<T>::nodeFission(util::MemorySegment& mem_sgmt, RBNode<T>& node,
+                       const isc::dns::Name& base_name)
+{
     using namespace helper;
     const isc::dns::Name sub_name = node.name_ - base_name;
-    // using auto_ptr here is to avoid memory leak in case of exception raised
-    // after the RBNode creation
-    std::auto_ptr<RBNode<T> > down_node(new RBNode<T>(sub_name));
+    // Note: the following code is not entirely exception safe (name copy
+    // can result in exception), but will soon be so within Trac #2091.
+    RBNode<T>* down_node = RBNode<T>::create(mem_sgmt, sub_name);
     node.name_ = base_name;
     // the rest of this function should be exception free so that it keeps
     // consistent behavior (i.e., a weak form of strong exception guarantee)
@@ -1503,10 +1611,10 @@ RBTree<T>::nodeFission(RBNode<T>& node, const isc::dns::Name& base_name) {
     std::swap(node.flags_, down_node->flags_);
 
     down_node->down_ = node.getDown();
+    node.down_ = down_node;
 
-    node.down_ = down_node.get();
-
-    // Restore the color of the node (may have gotten changed by the flags swap)
+    // Restore the color of the node (may have gotten changed by the flags
+    // swap)
     node.setColor(down_node->getColor());
 
     // root node of sub tree, the initial color is BLACK
@@ -1516,7 +1624,6 @@ RBTree<T>::nodeFission(RBNode<T>& node, const isc::dns::Name& base_name) {
     down_node->setSubTreeRoot(true);
 
     ++node_count_;
-    down_node.release();
 }
 
 

src/lib/datasrc/tests/Makefile.am

@@ -60,6 +60,7 @@ run_unittests_SOURCES += sqlite3_unittest.cc
 run_unittests_SOURCES += sqlite3_accessor_unittest.cc
 run_unittests_SOURCES += memory_datasrc_unittest.cc
 run_unittests_SOURCES += rbnode_rrset_unittest.cc
+run_unittests_SOURCES += zonetable_unittest.cc
 run_unittests_SOURCES += zone_finder_context_unittest.cc
 run_unittests_SOURCES += faked_nsec3.h faked_nsec3.cc
 run_unittests_SOURCES += client_list_unittest.cc

src/lib/datasrc/tests/rbtree_unittest.cc

@@ -16,6 +16,8 @@
 
 #include <exceptions/exceptions.h>
 
+#include <util/memory_segment_local.h>
+
 #include <dns/name.h>
 #include <dns/rrclass.h>
 #include <dns/rrset.h>
@@ -54,31 +56,61 @@ const size_t Name::MAX_LABELS;
  */
 
 namespace {
+class TreeHolder {
+public:
+    TreeHolder(util::MemorySegment& mem_sgmt, RBTree<int>* tree) :
+        mem_sgmt_(mem_sgmt), tree_(tree)
+    {}
+    ~TreeHolder() {
+        RBTree<int>::destroy(mem_sgmt_, tree_);
+    }
+    RBTree<int>* get() { return (tree_); }
+private:
+    util::MemorySegment& mem_sgmt_;
+    RBTree<int>* tree_;
+};
+
 class RBTreeTest : public::testing::Test {
 protected:
-    RBTreeTest() : rbtree_expose_empty_node(true), crbtnode(NULL) {
+    RBTreeTest() :
+        rbtree_holder_(mem_sgmt_, RBTree<int>::create(mem_sgmt_)),
+        rbtree_expose_empty_node_holder_(mem_sgmt_,
+                                         RBTree<int>::create(mem_sgmt_, true)),
+        rbtree(*rbtree_holder_.get()),
+        rbtree_expose_empty_node(*rbtree_expose_empty_node_holder_.get()),
+        crbtnode(NULL)
+    {
         const char* const domain_names[] = {
             "c", "b", "a", "x.d.e.f", "z.d.e.f", "g.h", "i.g.h", "o.w.y.d.e.f",
             "j.z.d.e.f", "p.w.y.d.e.f", "q.w.y.d.e.f", "k.g.h"};
         int name_count = sizeof(domain_names) / sizeof(domain_names[0]);
         for (int i = 0; i < name_count; ++i) {
-            rbtree.insert(Name(domain_names[i]), &rbtnode);
+            rbtree.insert(mem_sgmt_, Name(domain_names[i]), &rbtnode);
             rbtnode->setData(RBNode<int>::NodeDataPtr(new int(i + 1)));
 
-            rbtree_expose_empty_node.insert(Name(domain_names[i]), &rbtnode);
+            rbtree_expose_empty_node.insert(mem_sgmt_, Name(domain_names[i]),
+                                            &rbtnode);
             rbtnode->setData(RBNode<int>::NodeDataPtr(new int(i + 1)));
 
         }
     }
 
-    RBTree<int> rbtree;
-    RBTree<int> rbtree_expose_empty_node;
+    util::MemorySegmentLocal mem_sgmt_;
+    TreeHolder rbtree_holder_;
+    TreeHolder rbtree_expose_empty_node_holder_;
+    RBTree<int>& rbtree;
+    RBTree<int>& rbtree_expose_empty_node;
     RBNode<int>* rbtnode;
     const RBNode<int>* crbtnode;
 };
 
-TEST_F(RBTreeTest, getNodeCount) {
+TEST_F(RBTreeTest, nodeCount) {
     EXPECT_EQ(14, rbtree.getNodeCount());
+
+    // Delete all nodes, then the count should be set to 0.  This also tests
+    // the behavior of deleteAllNodes().
+    rbtree.deleteAllNodes(mem_sgmt_);
+    EXPECT_EQ(0, rbtree.getNodeCount());
 }
 
 TEST_F(RBTreeTest, setGetData) {
@@ -87,64 +119,92 @@ TEST_F(RBTreeTest, setGetData) {
 }
 
 TEST_F(RBTreeTest, insertNames) {
-    EXPECT_EQ(RBTree<int>::ALREADYEXISTS, rbtree.insert(Name("d.e.f"),
+    EXPECT_EQ(RBTree<int>::ALREADYEXISTS, rbtree.insert(mem_sgmt_,
+                                                        Name("d.e.f"),
                                                         &rbtnode));
     EXPECT_EQ(Name("d.e.f"), rbtnode->getName());
     EXPECT_EQ(14, rbtree.getNodeCount());
 
     //insert not exist node
-    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("."), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(mem_sgmt_, Name("."),
+                                                  &rbtnode));
     EXPECT_EQ(Name("."), rbtnode->getName());
     EXPECT_EQ(15, rbtree.getNodeCount());
 
-    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("example.com"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(mem_sgmt_,
+                                                  Name("example.com"),
+                                                  &rbtnode));
     EXPECT_EQ(16, rbtree.getNodeCount());
     rbtnode->setData(RBNode<int>::NodeDataPtr(new int(12)));
 
     // return ALREADYEXISTS, since node "example.com" already has been explicitly inserted
-    EXPECT_EQ(RBTree<int>::ALREADYEXISTS, rbtree.insert(Name("example.com"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::ALREADYEXISTS, rbtree.insert(mem_sgmt_,
+                                                        Name("example.com"),
+                                                        &rbtnode));
     EXPECT_EQ(16, rbtree.getNodeCount());
 
     // split the node "d.e.f"
-    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("k.e.f"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(mem_sgmt_, Name("k.e.f"),
+                                                  &rbtnode));
     EXPECT_EQ(Name("k"), rbtnode->getName());
     EXPECT_EQ(18, rbtree.getNodeCount());
 
     // split the node "g.h"
-    EXPECT_EQ(RBTree<int>::ALREADYEXISTS, rbtree.insert(Name("h"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::ALREADYEXISTS, rbtree.insert(mem_sgmt_, Name("h"),
+                                                        &rbtnode));
     EXPECT_EQ(Name("h"), rbtnode->getName());
     EXPECT_EQ(19, rbtree.getNodeCount());
 
     // add child domain
-    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("m.p.w.y.d.e.f"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(mem_sgmt_,
+                                                  Name("m.p.w.y.d.e.f"),
+                                                  &rbtnode));
     EXPECT_EQ(Name("m"), rbtnode->getName());
     EXPECT_EQ(20, rbtree.getNodeCount());
-    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("n.p.w.y.d.e.f"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(mem_sgmt_,
+                                                  Name("n.p.w.y.d.e.f"),
+                                                  &rbtnode));
     EXPECT_EQ(Name("n"), rbtnode->getName());
     EXPECT_EQ(21, rbtree.getNodeCount());
 
-    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("l.a"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(mem_sgmt_, Name("l.a"),
+                                                  &rbtnode));
     EXPECT_EQ(Name("l"), rbtnode->getName());
     EXPECT_EQ(22, rbtree.getNodeCount());
 
-    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("r.d.e.f"), &rbtnode));
-    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("s.d.e.f"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(mem_sgmt_, Name("r.d.e.f"),
+                                                  &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(mem_sgmt_, Name("s.d.e.f"),
+                                                  &rbtnode));
     EXPECT_EQ(24, rbtree.getNodeCount());
 
-    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("h.w.y.d.e.f"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(mem_sgmt_,
+                                                  Name("h.w.y.d.e.f"),
+                                                  &rbtnode));
 
     // add more nodes one by one to cover leftRotate and rightRotate
-    EXPECT_EQ(RBTree<int>::ALREADYEXISTS, rbtree.insert(Name("f"), &rbtnode));
-    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("m"), &rbtnode));
-    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("nm"), &rbtnode));
-    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("om"), &rbtnode));
-    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("k"), &rbtnode));
-    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("l"), &rbtnode));
-    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("fe"), &rbtnode));
-    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("ge"), &rbtnode));
-    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("i"), &rbtnode));
-    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("ae"), &rbtnode));
-    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("n"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::ALREADYEXISTS, rbtree.insert(mem_sgmt_, Name("f"),
+                                                        &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(mem_sgmt_, Name("m"),
+                                                  &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(mem_sgmt_, Name("nm"),
+                                                  &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(mem_sgmt_, Name("om"),
+                                                  &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(mem_sgmt_, Name("k"),
+                                                  &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(mem_sgmt_, Name("l"),
+                                                  &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(mem_sgmt_, Name("fe"),
+                                                  &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(mem_sgmt_, Name("ge"),
+                                                  &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(mem_sgmt_, Name("i"),
+                                                  &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(mem_sgmt_, Name("ae"),
+                                                  &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(mem_sgmt_, Name("n"),
+                                                  &rbtnode));
 }
 
 TEST_F(RBTreeTest, findName) {
@@ -187,7 +247,8 @@ TEST_F(RBTreeTest, findError) {
 }
 
 TEST_F(RBTreeTest, flags) {
-    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("flags.example"),
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(mem_sgmt_,
+                                                  Name("flags.example"),
                                                   &rbtnode));
 
     // by default, flags are all off
@@ -218,7 +279,8 @@ testCallback(const RBNode<int>&, bool* callack_checker) {
 
 TEST_F(RBTreeTest, callback) {
     // by default callback isn't enabled
-    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("callback.example"),
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(mem_sgmt_,
+                                                  Name("callback.example"),
                                                   &rbtnode));
     rbtnode->setData(RBNode<int>::NodeDataPtr(new int(1)));
     EXPECT_FALSE(rbtnode->getFlag(RBNode<int>::FLAG_CALLBACK));
@@ -233,12 +295,14 @@ TEST_F(RBTreeTest, callback) {
     rbtnode->setFlag(RBNode<int>::FLAG_CALLBACK);
     // add more levels below and above the callback node for partial match.
     RBNode<int>* subrbtnode;
-    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("sub.callback.example"),
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(mem_sgmt_,
+                                                  Name("sub.callback.example"),
                                                   &subrbtnode));
     subrbtnode->setData(RBNode<int>::NodeDataPtr(new int(2)));
     RBNode<int>* parentrbtnode;
-    EXPECT_EQ(RBTree<int>::ALREADYEXISTS, rbtree.insert(Name("example"),
-                                                       &parentrbtnode));
+    EXPECT_EQ(RBTree<int>::ALREADYEXISTS, rbtree.insert(mem_sgmt_,
+                                                        Name("example"),
+                                                        &parentrbtnode));
     //  the chilld/parent nodes shouldn't "inherit" the callback flag.
     // "rbtnode" may be invalid due to the insertion, so we need to re-find
     // it.
@@ -275,9 +339,11 @@ TEST_F(RBTreeTest, chainLevel) {
 
     // insert one node to the tree and find it.  there should be exactly
     // one level in the chain.
-    RBTree<int> tree(true);
+    TreeHolder tree_holder(mem_sgmt_, RBTree<int>::create(mem_sgmt_, true));
+    RBTree<int>& tree(*tree_holder.get());
     Name node_name(Name::ROOT_NAME());
-    EXPECT_EQ(RBTree<int>::SUCCESS, tree.insert(node_name, &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, tree.insert(mem_sgmt_, node_name,
+                                                &rbtnode));
     EXPECT_EQ(RBTree<int>::EXACTMATCH,
               tree.find(node_name, &crbtnode, chain));
     EXPECT_EQ(1, chain.getLevelCount());
@@ -299,7 +365,8 @@ TEST_F(RBTreeTest, chainLevel) {
      */
     for (unsigned int i = 2; i <= Name::MAX_LABELS; ++i) {
         node_name = Name("a.").concatenate(node_name);
-        EXPECT_EQ(RBTree<int>::SUCCESS, tree.insert(node_name, &rbtnode));
+        EXPECT_EQ(RBTree<int>::SUCCESS, tree.insert(mem_sgmt_, node_name,
+                                                    &rbtnode));
         RBTreeNodeChain<int> found_chain;
         EXPECT_EQ(RBTree<int>::EXACTMATCH,
                   tree.find(node_name, &crbtnode, found_chain));
@@ -542,7 +609,8 @@ TEST_F(RBTreeTest, previousNode) {
     {
         SCOPED_TRACE("Lookup in empty tree");
         // Just check it doesn't crash, etc.
-        RBTree<int> empty_tree;
+        TreeHolder tree_holder(mem_sgmt_, RBTree<int>::create(mem_sgmt_));
+        RBTree<int>& empty_tree(*tree_holder.get());
         EXPECT_EQ(RBTree<int>::NOTFOUND,
                   empty_tree.find(Name("x"), &node, node_path));
         EXPECT_EQ(static_cast<void*>(NULL), node);
@@ -586,7 +654,8 @@ TEST_F(RBTreeTest, getLastComparedNode) {
     EXPECT_EQ(static_cast<void*>(NULL), chain.getLastComparedNode());
 
     // A search for an empty tree should result in no 'last compared', too.
-    RBTree<int> empty_tree;
+    TreeHolder tree_holder(mem_sgmt_, RBTree<int>::create(mem_sgmt_));
+    RBTree<int>& empty_tree(*tree_holder.get());
     EXPECT_EQ(RBTree<int>::NOTFOUND,
               empty_tree.find(Name("a"), &crbtnode, chain));
     EXPECT_EQ(static_cast<void*>(NULL), chain.getLastComparedNode());
@@ -731,9 +800,10 @@ TEST_F(RBTreeTest, swap) {
     size_t count1(rbtree.getNodeCount());
 
     // Create second one and store state
-    RBTree<int> tree2;
+    TreeHolder tree_holder(mem_sgmt_, RBTree<int>::create(mem_sgmt_));
+    RBTree<int>& tree2(*tree_holder.get());
     RBNode<int>* node;
-    tree2.insert(Name("second"), &node);
+    tree2.insert(mem_sgmt_, Name("second"), &node);
     std::ostringstream str2;
     tree2.dumpTree(str2);
 
@@ -757,8 +827,9 @@ TEST_F(RBTreeTest, swap) {
 // any domain names should be considered a subdomain of it), so it makes
 // sense to test cases with the root zone explicitly.
 TEST_F(RBTreeTest, root) {
-    RBTree<int> root;
-    root.insert(Name::ROOT_NAME(), &rbtnode);
+    TreeHolder tree_holder(mem_sgmt_, RBTree<int>::create(mem_sgmt_));
+    RBTree<int>& root(*tree_holder.get());
+    root.insert(mem_sgmt_, Name::ROOT_NAME(), &rbtnode);
     rbtnode->setData(RBNode<int>::NodeDataPtr(new int(1)));
 
     EXPECT_EQ(RBTree<int>::EXACTMATCH,
@@ -770,7 +841,7 @@ TEST_F(RBTreeTest, root) {
 
     // Insert a new name that better matches the query name.  find() should
     // find the better one.
-    root.insert(Name("com"), &rbtnode);
+    root.insert(mem_sgmt_, Name("com"), &rbtnode);
     rbtnode->setData(RBNode<int>::NodeDataPtr(new int(2)));
     EXPECT_EQ(RBTree<int>::PARTIALMATCH,
               root.find(Name("example.com"), &crbtnode));
@@ -778,8 +849,10 @@ TEST_F(RBTreeTest, root) {
 
     // Perform the same tests for the tree that allows matching against empty
     // nodes.
-    RBTree<int> root_emptyok(true);
-    root_emptyok.insert(Name::ROOT_NAME(), &rbtnode);
+    TreeHolder tree_holder_emptyok(mem_sgmt_,
+                                   RBTree<int>::create(mem_sgmt_, true));
+    RBTree<int>& root_emptyok(*tree_holder_emptyok.get());
+    root_emptyok.insert(mem_sgmt_, Name::ROOT_NAME(), &rbtnode);
     EXPECT_EQ(RBTree<int>::EXACTMATCH,
               root_emptyok.find(Name::ROOT_NAME(), &crbtnode));
     EXPECT_EQ(rbtnode, crbtnode);
@@ -787,7 +860,7 @@ TEST_F(RBTreeTest, root) {
               root_emptyok.find(Name("example.com"), &crbtnode));
     EXPECT_EQ(rbtnode, crbtnode);
 
-    root.insert(Name("com"), &rbtnode);
+    root.insert(mem_sgmt_, Name("com"), &rbtnode);
     EXPECT_EQ(RBTree<int>::PARTIALMATCH,
               root.find(Name("example.com"), &crbtnode));
     EXPECT_EQ(rbtnode, crbtnode);

src/lib/datasrc/tests/zonetable_unittest.cc

@@ -14,6 +14,8 @@
 
 #include <exceptions/exceptions.h>
 
+#include <util/memory_segment_local.h>
+
 #include <dns/name.h>
 #include <dns/rrclass.h>
 
@@ -40,7 +42,7 @@ TEST(ZoneTest, init) {
 TEST(ZoneTest, find) {
     InMemoryZoneFinder zone(RRClass::IN(), Name("example.com"));
     EXPECT_EQ(ZoneFinder::NXDOMAIN,
-              zone.find(Name("www.example.com"), RRType::A()).code);
+              zone.find(Name("www.example.com"), RRType::A())->code);
 }
 
 class ZoneTableTest : public ::testing::Test {
@@ -50,68 +52,77 @@ protected:
                       zone2(new InMemoryZoneFinder(RRClass::IN(),
                                                    Name("example.net"))),
                       zone3(new InMemoryZoneFinder(RRClass::IN(),
-                                                   Name("example")))
+                                                   Name("example"))),
+                      zone_table(ZoneTable::create(mem_sgmt_))
     {}
-    ZoneTable zone_table;
+
+    ~ZoneTableTest() {
+        ZoneTable::destroy(mem_sgmt_, zone_table);
+    }
     ZoneFinderPtr zone1, zone2, zone3;
+    isc::util::MemorySegmentLocal mem_sgmt_;
+    ZoneTable* zone_table;
 };
 
 TEST_F(ZoneTableTest, addZone) {
-    EXPECT_EQ(result::SUCCESS, zone_table.addZone(zone1));
-    EXPECT_EQ(result::EXIST, zone_table.addZone(zone1));
+    EXPECT_EQ(result::SUCCESS, zone_table->addZone(mem_sgmt_, zone1));
+    EXPECT_EQ(result::EXIST, zone_table->addZone(mem_sgmt_, zone1));
     // names are compared in a case insensitive manner.
-    EXPECT_EQ(result::EXIST, zone_table.addZone(
+    EXPECT_EQ(result::EXIST, zone_table->addZone(
+                  mem_sgmt_,
                   ZoneFinderPtr(new InMemoryZoneFinder(RRClass::IN(),
                                                        Name("EXAMPLE.COM")))));
 
-    EXPECT_EQ(result::SUCCESS, zone_table.addZone(zone2));
-    EXPECT_EQ(result::SUCCESS, zone_table.addZone(zone3));
+    EXPECT_EQ(result::SUCCESS, zone_table->addZone(mem_sgmt_, zone2));
+    EXPECT_EQ(result::SUCCESS, zone_table->addZone(mem_sgmt_, zone3));
 
     // Zone table is indexed only by name.  Duplicate origin name with
     // different zone class isn't allowed.
-    EXPECT_EQ(result::EXIST, zone_table.addZone(
+    EXPECT_EQ(result::EXIST, zone_table->addZone(
+                  mem_sgmt_,
                   ZoneFinderPtr(new InMemoryZoneFinder(RRClass::CH(),
                                                        Name("example.com")))));
 
     /// Bogus zone (NULL)
-    EXPECT_THROW(zone_table.addZone(ZoneFinderPtr()), isc::InvalidParameter);
+    EXPECT_THROW(zone_table->addZone(mem_sgmt_, ZoneFinderPtr()),
+                 isc::InvalidParameter);
 }
 
 TEST_F(ZoneTableTest, DISABLED_removeZone) {
-    EXPECT_EQ(result::SUCCESS, zone_table.addZone(zone1));
-    EXPECT_EQ(result::SUCCESS, zone_table.addZone(zone2));
-    EXPECT_EQ(result::SUCCESS, zone_table.addZone(zone3));
+    EXPECT_EQ(result::SUCCESS, zone_table->addZone(mem_sgmt_, zone1));
+    EXPECT_EQ(result::SUCCESS, zone_table->addZone(mem_sgmt_, zone2));
+    EXPECT_EQ(result::SUCCESS, zone_table->addZone(mem_sgmt_, zone3));
 
-    EXPECT_EQ(result::SUCCESS, zone_table.removeZone(Name("example.net")));
-    EXPECT_EQ(result::NOTFOUND, zone_table.removeZone(Name("example.net")));
+    EXPECT_EQ(result::SUCCESS, zone_table->removeZone(Name("example.net")));
+    EXPECT_EQ(result::NOTFOUND, zone_table->removeZone(Name("example.net")));
 }
 
 TEST_F(ZoneTableTest, findZone) {
-    EXPECT_EQ(result::SUCCESS, zone_table.addZone(zone1));
-    EXPECT_EQ(result::SUCCESS, zone_table.addZone(zone2));
-    EXPECT_EQ(result::SUCCESS, zone_table.addZone(zone3));
+    EXPECT_EQ(result::SUCCESS, zone_table->addZone(mem_sgmt_, zone1));
+    EXPECT_EQ(result::SUCCESS, zone_table->addZone(mem_sgmt_, zone2));
+    EXPECT_EQ(result::SUCCESS, zone_table->addZone(mem_sgmt_, zone3));
 
-    EXPECT_EQ(result::SUCCESS, zone_table.findZone(Name("example.com")).code);
+    EXPECT_EQ(result::SUCCESS, zone_table->findZone(Name("example.com")).code);
     EXPECT_EQ(Name("example.com"),
-              zone_table.findZone(Name("example.com")).zone->getOrigin());
+              zone_table->findZone(Name("example.com")).zone->getOrigin());
 
     EXPECT_EQ(result::NOTFOUND,
-              zone_table.findZone(Name("example.org")).code);
+              zone_table->findZone(Name("example.org")).code);
     EXPECT_EQ(ConstZoneFinderPtr(),
-              zone_table.findZone(Name("example.org")).zone);
+              zone_table->findZone(Name("example.org")).zone);
 
     // there's no exact match.  the result should be the longest match,
     // and the code should be PARTIALMATCH.
     EXPECT_EQ(result::PARTIALMATCH,
-              zone_table.findZone(Name("www.example.com")).code);
+              zone_table->findZone(Name("www.example.com")).code);
     EXPECT_EQ(Name("example.com"),
-              zone_table.findZone(Name("www.example.com")).zone->getOrigin());
+              zone_table->findZone(Name("www.example.com")).zone->getOrigin());
 
     // make sure the partial match is indeed the longest match by adding
     // a zone with a shorter origin and query again.
     ZoneFinderPtr zone_com(new InMemoryZoneFinder(RRClass::IN(), Name("com")));
-    EXPECT_EQ(result::SUCCESS, zone_table.addZone(zone_com));
+    EXPECT_EQ(result::SUCCESS, zone_table->addZone(mem_sgmt_, zone_com));
     EXPECT_EQ(Name("example.com"),
-              zone_table.findZone(Name("www.example.com")).zone->getOrigin());
+              zone_table->findZone(Name("www.example.com")).zone->getOrigin());
 }
 }

src/lib/datasrc/zonetable.cc

@@ -12,13 +12,15 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <cassert>
+#include <util/memory_segment.h>
 
 #include <dns/name.h>
 
 #include <datasrc/zonetable.h>
 #include <datasrc/rbtree.h>
 
+#include <cassert>
+
 using namespace std;
 using namespace isc::dns;
 
@@ -30,8 +32,14 @@ struct ZoneTable::ZoneTableImpl {
     // Type aliases to make it shorter
     typedef RBTree<ZoneFinder> ZoneTree;
     typedef RBNode<ZoneFinder> ZoneNode;
+
     // The actual storage
-    ZoneTree zones_;
+    ZoneTree* zones_;
+
+    // Constructor
+    ZoneTableImpl(util::MemorySegment& mem_sgmt) :
+        zones_(ZoneTree::create(mem_sgmt))
+    {}
 
     /*
      * The implementation methods are here and just wrap-called in the
@@ -40,7 +48,7 @@ struct ZoneTable::ZoneTableImpl {
      */
 
     // Implementation of ZoneTable::addZone
-    result::Result addZone(ZoneFinderPtr zone) {
+    result::Result addZone(util::MemorySegment& mem_sgmt, ZoneFinderPtr zone) {
         // Sanity check
         if (!zone) {
             isc_throw(InvalidParameter,
@@ -49,7 +57,7 @@ struct ZoneTable::ZoneTableImpl {
 
         // Get the node where we put the zone
         ZoneNode* node(NULL);
-        switch (zones_.insert(zone->getOrigin(), &node)) {
+        switch (zones_->insert(mem_sgmt, zone->getOrigin(), &node)) {
             // This is OK
             case ZoneTree::SUCCESS:
             case ZoneTree::ALREADYEXISTS:
@@ -76,7 +84,7 @@ struct ZoneTable::ZoneTableImpl {
         result::Result my_result;
 
         // Translate the return codes
-        switch (zones_.find(name, &node)) {
+        switch (zones_->find(name, &node)) {
             case ZoneTree::EXACTMATCH:
                 my_result = result::SUCCESS;
                 break;
@@ -100,16 +108,39 @@ struct ZoneTable::ZoneTableImpl {
     }
 };
 
-ZoneTable::ZoneTable() : impl_(new ZoneTableImpl)
+ZoneTable::ZoneTable(util::MemorySegment& mem_sgmt) :
+    impl_(new ZoneTableImpl(mem_sgmt))
 {}
 
 ZoneTable::~ZoneTable() {
     delete impl_;
 }
 
+ZoneTable*
+ZoneTable::create(util::MemorySegment& mem_sgmt) {
+    // The ZoneTable constructor can throw, so we need to prevent memory leak.
+    // This is ugly, but for now this seems to be the only place we need
+    // this, and since we'll substantially revise this code soon, so we don't
+    // work around it by this hack at the moment.
+    void* p = mem_sgmt.allocate(sizeof(ZoneTable));
+    try {
+        return (new(p) ZoneTable(mem_sgmt));
+    } catch (...) {
+        mem_sgmt.deallocate(p, sizeof(ZoneTable));
+        throw;
+    }
+}
+
+void
+ZoneTable::destroy(util::MemorySegment& mem_sgmt, ZoneTable* ztable) {
+    ZoneTableImpl::ZoneTree::destroy(mem_sgmt, ztable->impl_->zones_);
+    ztable->~ZoneTable();
+    mem_sgmt.deallocate(ztable, sizeof(ZoneTable));
+}
+
 result::Result
-ZoneTable::addZone(ZoneFinderPtr zone) {
-    return (impl_->addZone(zone));
+ZoneTable::addZone(util::MemorySegment& mem_sgmt, ZoneFinderPtr zone) {
+    return (impl_->addZone(mem_sgmt, zone));
 }
 
 result::Result

src/lib/datasrc/zonetable.h

@@ -15,12 +15,14 @@
 #ifndef __ZONETABLE_H
 #define __ZONETABLE_H 1
 
-#include <boost/shared_ptr.hpp>
+#include <util/memory_segment.h>
 
 #include <dns/rrset.h>
 
 #include <datasrc/zone.h>
 
+#include <boost/shared_ptr.hpp>
+
 namespace isc {
 namespace dns {
 class Name;
@@ -58,18 +60,47 @@ private:
     ZoneTable(const ZoneTable& source);
     ZoneTable& operator=(const ZoneTable& source);
 
-public:
-    /// Default constructor.
+    /// Constructor.
+    ///
+    /// An object of this class is always expected to be created by the
+    /// allocator (\c create()), so the constructor is hidden as private.
     ///
     /// This constructor internally involves resource allocation, and if
     /// it fails, a corresponding standard exception will be thrown.
     /// It never throws an exception otherwise.
-    ZoneTable();
+    ZoneTable(util::MemorySegment& mem_sgmt);
 
     /// The destructor.
+    ///
+    /// An object of this class is always expected to be destroyed explicitly
+    /// by \c destroy(), so the constructor is hidden as private.
     ~ZoneTable();
     //@}
 
+public:
+    /// \brief Allocate and construct \c ZoneTable
+    ///
+    /// This static method allocates memory for a new \c ZoneTable object
+    /// from the given memory segment, constructs the object, and returns
+    /// a pointer to it.
+    ///
+    /// \throw std::bad_alloc Memory allocation fails.
+    ///
+    /// \param mem_sgmt A \c MemorySegment from which memory for the new
+    /// \c ZoneTable is allocated.
+    static ZoneTable* create(util::MemorySegment& mem_sgmt);
+
+    /// \brief Destruct and deallocate \c ZoneTable
+    ///
+    /// \throw none
+    ///
+    /// \param mem_sgmt The \c MemorySegment that allocated memory for
+    /// \c ztable.
+    /// \param ztable A non NULL pointer to a valid \c ZoneTable object
+    /// that was originally created by the \c create() method (the behavior
+    /// is undefined if this condition isn't met).
+    static void destroy(util::MemorySegment& mem_sgmt, ZoneTable* ztable);
+
     /// Add a \c Zone to the \c ZoneTable.
     ///
     /// \c Zone must not be associated with a NULL pointer; otherwise
@@ -83,7 +114,7 @@ public:
     /// added to the zone table.
     /// \return \c result::EXIST The zone table already contains
     /// zone of the same origin.
-    result::Result addZone(ZoneFinderPtr zone);
+    result::Result addZone(util::MemorySegment& mem_sgmt, ZoneFinderPtr zone);
 
     /// Remove a \c Zone of the given origin name from the \c ZoneTable.
     ///

src/lib/dns/labelsequence.cc

@@ -23,62 +23,75 @@
 namespace isc {
 namespace dns {
 
-LabelSequence::LabelSequence(const uint8_t* data,
-                             const uint8_t* offsets,
-                             size_t offsets_size) : data_(data),
-                                                    offsets_(offsets),
-                                                    offsets_size_(offsets_size),
-                                                    first_label_(0),
-                                                    last_label_(offsets_size_)
-{
-    if (data == NULL || offsets == NULL) {
-        isc_throw(BadValue, "Null pointer passed to LabelSequence constructor");
-    }
-    if (offsets_size == 0) {
-        isc_throw(BadValue, "Zero offsets to LabelSequence constructor");
+LabelSequence::LabelSequence(const void* buf) {
+    if (buf == NULL) {
+        isc_throw(BadValue,
+                  "Null pointer passed to LabelSequence constructor");
     }
-    if (offsets_size > Name::MAX_LABELS) {
-        isc_throw(BadValue, "MAX_LABELS exceeded");
+
+    const uint8_t* bp = reinterpret_cast<const uint8_t*>(buf);
+
+    first_label_ = 0;
+    const uint8_t offsets_len = *bp++;
+    if (offsets_len == 0 || offsets_len > Name::MAX_LABELS) {
+        isc_throw(BadValue,
+                  "Bad offsets len in serialized LabelSequence data: "
+                  << static_cast<unsigned int>(offsets_len));
     }
-    for (size_t cur_offset = 0; cur_offset < offsets_size; ++cur_offset) {
-        if (offsets[cur_offset] > Name::MAX_LABELLEN) {
-            isc_throw(BadValue, "MAX_LABEL_LEN exceeded");
-        }
-        if (cur_offset > 0 && offsets[cur_offset] <= offsets[cur_offset - 1]) {
-            isc_throw(BadValue, "Offset smaller than previous offset");
+    last_label_ = offsets_len - 1;
+    offsets_ = bp;
+    data_ = bp + offsets_len;
+
+    // Check the integrity on the offsets and the name data
+    const uint8_t* dp = data_;
+    for (size_t cur_offset = 0; cur_offset < offsets_len; ++cur_offset) {
+        if (offsets_[cur_offset] > Name::MAX_LABELLEN ||
+            dp - data_ != offsets_[cur_offset]) {
+            isc_throw(BadValue,
+                      "Broken offset or name data in serialized "
+                      "LabelSequence data");
         }
+        dp += (1 + *dp);
     }
 }
 
-
 const uint8_t*
 LabelSequence::getData(size_t *len) const {
     *len = getDataLength();
     return (&data_[offsets_[first_label_]]);
 }
 
-void
-LabelSequence::getOffsetData(size_t* len,
-                             uint8_t placeholder[Name::MAX_LABELS]) const {
-  *len = last_label_ - first_label_;
-  for (size_t i = 0; i < *len; i++) {
-      placeholder[i] = offsets_[first_label_ + i] - offsets_[first_label_];
-  }
+size_t
+LabelSequence::getDataLength() const {
+    const size_t last_label_len = data_[offsets_[last_label_]] + 1;
+    return (offsets_[last_label_] - offsets_[first_label_] + last_label_len);
 }
 
 size_t
-LabelSequence::getDataLength() const {
-    // If the labelsequence is absolute, the current last_label_ falls
-    // out of the vector (since it points to the 'label' after the
-    // root label, which doesn't exist; in that case, return
-    // the length for the 'previous' label (the root label) plus
-    // one (for the root label zero octet)
-    if (isAbsolute()) {
-        return (offsets_[last_label_ - 1] -
-                offsets_[first_label_] + 1);
-    } else {
-        return (offsets_[last_label_] - offsets_[first_label_]);
+LabelSequence::getSerializedLength() const {
+    return (1 + getLabelCount() + getDataLength());
+}
+
+void
+LabelSequence::serialize(void* buf, size_t buf_len) const {
+    const size_t expected_size = getSerializedLength();
+    if (expected_size > buf_len) {
+        isc_throw(BadValue, "buffer too short for LabelSequence::serialize");
+    }
+
+    const size_t offsets_len = getLabelCount();
+    assert(offsets_len < 256);  // should be in the 8-bit range
+
+    uint8_t* bp = reinterpret_cast<uint8_t*>(buf);
+    *bp++ = offsets_len;
+    for (size_t i = 0; i < offsets_len; ++i) {
+        *bp++ = offsets_[first_label_ + i] - offsets_[first_label_];
     }
+    const size_t ndata_len = getDataLength();
+    memcpy(bp, &data_[offsets_[first_label_]], ndata_len);
+    bp += ndata_len;
+
+    assert(bp - reinterpret_cast<const uint8_t*>(buf) == expected_size);
 }
 
 bool
@@ -110,19 +123,16 @@ LabelSequence::equals(const LabelSequence& other, bool case_sensitive) const {
 
 NameComparisonResult
 LabelSequence::compare(const LabelSequence& other,
-                       bool case_sensitive) const {
-    if (isAbsolute() ^ other.isAbsolute()) {
-        return (NameComparisonResult(0, 0, NameComparisonResult::NONE));
-    }
-
+                       bool case_sensitive) const
+{
     // Determine the relative ordering under the DNSSEC order relation of
     // 'this' and 'other', and also determine the hierarchical relationship
-    // of the names.
+    // of the labels.
 
     unsigned int nlabels = 0;
-    int l1 = last_label_ - first_label_;
-    int l2 = other.last_label_ - other.first_label_;
-    int ldiff = (int)l1 - (int)l2;
+    int l1 = getLabelCount();
+    int l2 = other.getLabelCount();
+    const int ldiff = static_cast<int>(l1) - static_cast<int>(l2);
     unsigned int l = (ldiff < 0) ? l1 : l2;
 
     while (l > 0) {
@@ -138,47 +148,38 @@ LabelSequence::compare(const LabelSequence& other,
         // bitstring labels.
         assert(count1 <= Name::MAX_LABELLEN && count2 <= Name::MAX_LABELLEN);
 
-        int cdiff = (int)count1 - (int)count2;
+        const int cdiff = static_cast<int>(count1) - static_cast<int>(count2);
         unsigned int count = (cdiff < 0) ? count1 : count2;
 
         while (count > 0) {
-            uint8_t label1 = data_[pos1];
-            uint8_t label2 = other.data_[pos2];
+            const uint8_t label1 = data_[pos1];
+            const uint8_t label2 = other.data_[pos2];
             int chdiff;
 
             if (case_sensitive) {
-                chdiff = (int)label1 - (int)label2;
+                chdiff = static_cast<int>(label1) - static_cast<int>(label2);
             } else {
-                chdiff = (int)isc::dns::name::internal::maptolower[label1] -
-                         (int)isc::dns::name::internal::maptolower[label2];
+                chdiff = static_cast<int>(
+                    isc::dns::name::internal::maptolower[label1]) -
+                    static_cast<int>(
+                        isc::dns::name::internal::maptolower[label2]);
             }
 
             if (chdiff != 0) {
-                if ((nlabels == 0) &&
-                     (!isAbsolute() ||
-                     ((last_label_ < getLabelCount()) ||
-                      (other.last_label_ < other.getLabelCount())))) {
-                    return (NameComparisonResult(0, 0,
-                                                 NameComparisonResult::NONE));
-                } else {
-                    return (NameComparisonResult(chdiff, nlabels,
-                                                 NameComparisonResult::COMMONANCESTOR));
-                }
+                return (NameComparisonResult(
+                            chdiff, nlabels,
+                            nlabels == 0 ? NameComparisonResult::NONE :
+                            NameComparisonResult::COMMONANCESTOR));
             }
             --count;
             ++pos1;
             ++pos2;
         }
         if (cdiff != 0) {
-            if ((nlabels == 0) &&
-                ((last_label_ < getLabelCount()) ||
-                 (other.last_label_ < other.getLabelCount()))) {
-                return (NameComparisonResult(0, 0,
-                                             NameComparisonResult::NONE));
-            } else {
-                return (NameComparisonResult(cdiff, nlabels,
-                                             NameComparisonResult::COMMONANCESTOR));
-            }
+            return (NameComparisonResult(
+                        cdiff, nlabels,
+                        nlabels == 0 ? NameComparisonResult::NONE :
+                        NameComparisonResult::COMMONANCESTOR));
         }
         ++nlabels;
     }
@@ -214,7 +215,7 @@ LabelSequence::stripRight(size_t i) {
 
 bool
 LabelSequence::isAbsolute() const {
-    return (last_label_ == offsets_size_);
+    return (data_[offsets_[last_label_]] == 0);
 }
 
 size_t
@@ -241,7 +242,7 @@ LabelSequence::toText(bool omit_final_dot) const {
     const uint8_t* np_end = np + getDataLength();
 
     // use for integrity check
-    unsigned int labels = last_label_ - first_label_;
+    unsigned int labels = getLabelCount();
     // init with an impossible value to catch error cases in the end:
     unsigned int count = Name::MAX_LABELLEN + 1;
 

src/lib/dns/labelsequence.h

@@ -21,7 +21,7 @@
 namespace isc {
 namespace dns {
 
-/// \brief Light-weight Accessor to data of Name object
+/// \brief Light-weight Accessor to Name data.
 ///
 /// The purpose of this class is to easily match Names and parts of Names,
 /// without needing to copy the underlying data on each label strip.
@@ -40,7 +40,6 @@ namespace dns {
 /// LabelSequences can be compared to other LabelSequences, and their
 /// data can be requested (which then points to part of the original
 /// data of the original Name object).
-///
 class LabelSequence {
     // Name calls the private toText(bool) method of LabelSequence.
     friend std::string Name::toText(bool) const;
@@ -55,48 +54,29 @@ public:
     ///
     /// \param name The Name to construct a LabelSequence for
     explicit LabelSequence(const Name& name):
-                                     data_(&name.ndata_[0]),
-                                     offsets_(&name.offsets_[0]),
-                                     offsets_size_(name.offsets_.size()),
-                                     first_label_(0),
-                                     last_label_(name.getLabelCount())
+        data_(&name.ndata_[0]),
+        offsets_(&name.offsets_[0]),
+        first_label_(0),
+        last_label_(name.getLabelCount() - 1)
     {}
 
-    /// \brief Constructs a LabelSequence for the given data
+    /// \brief Constructor from serialized image.
     ///
-    /// \note The associated data MUST remain in scope during the lifetime
-    /// of this LabelSequence, since only the pointers are copied.
+    /// This constructor restores a \c LabelSequence object from a serialized
+    /// binary image previously generated by \c serialize().  Any other input
+    /// to this constructor will result in undefined behavior.
     ///
-    /// \note No validation is done on the given data upon construction;
-    ///       use with care.
+    /// The binary data passed to this constructor MUST remain in scope and
+    /// MUST NOT be modified during the lifetime of this LabelSequence.
     ///
-    /// \exception isc::BadValue if basic checks for the input data, or
-    ///            offsets fails.
+    /// As long as the data were previously generated by a call to
+    /// \c serialize() on a valid \c LabelSequence object, this constructor
+    /// should succeed.  While any other case is undefined, this constructor
+    /// may perform some validity checks internally for safety.  Nevertheless,
+    /// applications must not rely on such checks.
     ///
-    /// \param data The raw data for the domain name, in wire format
-    /// \param offsets The offsets of the labels in the domain name data,
-    ///        as given by a Name object or another LabelSequence
-    /// \param offsets_size The size of the offsets data
-    LabelSequence(const uint8_t* data,
-                  const uint8_t* offsets,
-                  size_t offsets_size);
-
-    /// \brief Copy constructor.
-    ///
-    /// \note The associated data MUST remain in scope during the lifetime
-    /// of this LabelSequence, since only the pointers are copied.
-    ///
-    /// \note No validation is done on the given data upon construction;
-    ///       use with care.
-    ///
-    /// \param ls The LabelSequence to construct a LabelSequence from
-    LabelSequence(const LabelSequence& ls):
-                                     data_(ls.data_),
-                                     offsets_(ls.offsets_),
-                                     offsets_size_(ls.offsets_size_),
-                                     first_label_(ls.first_label_),
-                                     last_label_(ls.last_label_)
-    {}
+    /// \param buf Pointer to the serialized image generated by \c serialize().
+    explicit LabelSequence(const void* buf);
 
     /// \brief Return the wire-format data for this LabelSequence
     ///
@@ -113,16 +93,6 @@ public:
     /// \return Pointer to the wire-format data of this label sequence
     const uint8_t* getData(size_t* len) const;
 
-    /// \brief Return the offset data for this LabelSequence
-    ///
-    /// The offsets are returned in the <code>placeholder</code> array.
-    ///
-    /// \param len Pointer to a size_t where the number of offsets
-    ///        will be stored
-    /// \param placeholder Array where the offset data will be returned
-    void getOffsetData(size_t* len,
-                       uint8_t placeholder[Name::MAX_LABELS]) const;
-
     /// \brief Return the length of the wire-format data of this LabelSequence
     ///
     /// This method returns the number of octets for the data that would
@@ -139,6 +109,52 @@ public:
     /// \return The length of the data of the label sequence in octets.
     size_t getDataLength() const;
 
+    /// \brief Max possible size of serialized image generated by \c serialize
+    ///
+    /// A fixed length buffer of this size can be always passed to
+    /// \c serialize() safely.  (But the application shouldn't use the
+    /// specific size value; it must use this constant variable).
+    static const size_t MAX_SERIALIZED_LENGTH =
+        Name::MAX_WIRE + Name::MAX_LABELS + 1;
+
+    /// \brief Return the size of serialized image of the \c LabelSequence.
+    ///
+    /// This method calculates the size of necessary storage to store
+    /// serialized image of this \c LabelSequence (which would be dumped by
+    /// \c serialize()) and returns it.  The size is in bytes.
+    ///
+    /// \throw none.
+    ///
+    /// \return The size of serialized image of the \c LabelSequence.
+    size_t getSerializedLength() const;
+
+    /// \brief Serialize the \c LabelSequence object in to a buffer.
+    ///
+    /// This method dumps a serialized image of this \c LabelSequence
+    /// that would be restored by the corresponding constructor into the
+    /// given buffer.  The buffer size must be at least equal to
+    /// the value returned by getSerializedLength() (it can be larger than
+    /// that).
+    ///
+    /// The serialized image would be as follows:
+    /// - olen: number of offsets (1 byte)
+    /// - binary sequence of offsets (olen bytes, verbatim copy of offsets_
+    ///   of this size)
+    /// - binary sequence of name data (length determined by itself, verbatim
+    ///   copy of data_ of the corresponding size)
+    ///
+    /// Applications must use the resulting image opaque value and must not
+    /// use it for other purposes than input to the corresponding constructor
+    /// to restore it.  Application behavior that assumes the specific
+    /// organization of the image is not guaranteed.
+    ///
+    /// \throw isc::BadValue buf_len is too short (this method never throws
+    /// otherwise)
+    ///
+    /// \param buf Pointer to the placeholder to dump the serialized image
+    /// \param buf_len The size of available region in \c buf
+    void serialize(void* buf, size_t buf_len) const;
+
     /// \brief Compares two label sequences for equality.
     ///
     /// Performs a (optionally case-insensitive) comparison between this
@@ -187,7 +203,9 @@ public:
     /// \brief Returns the current number of labels for this LabelSequence
     ///
     /// \return The number of labels
-    size_t getLabelCount() const { return (last_label_ - first_label_); }
+    size_t getLabelCount() const {
+        return (last_label_ - first_label_ + 1);
+    }
 
     /// \brief Convert the LabelSequence to a string.
     ///
@@ -252,11 +270,12 @@ public:
     bool isAbsolute() const;
 
 private:
-    const uint8_t* data_;
-    const uint8_t* offsets_;
-    size_t offsets_size_;
-    size_t first_label_;
-    size_t last_label_;
+    const uint8_t* data_;       // wire-format name data
+    const uint8_t* offsets_;    // an array of offsets in data_ for the labels
+    size_t first_label_;        // index of offsets_ for the first label
+    size_t last_label_;         // index of offsets_ for the last label.
+                                // can be equal to first_label_, but must not
+                                // be smaller (the class ensures that)
 };
 
 
@@ -281,3 +300,7 @@ operator<<(std::ostream& os, const LabelSequence& label_sequence);
 } // end namespace isc
 
 #endif
+
+// Local Variables:
+// mode: c++
+// End:

src/lib/dns/name.h

@@ -106,38 +106,45 @@ public:
 };
 
 ///
-/// This is a supplemental class used only as a return value of Name::compare().
-/// It encapsulate a tuple of the comparison: ordering, number of common labels,
-/// and relationship as follows:
+/// This is a supplemental class used only as a return value of
+/// Name::compare() and LabelSequence::compare().
+/// It encapsulate a tuple of the comparison: ordering, number of common
+/// labels, and relationship as follows:
 /// - ordering: relative ordering under the DNSSEC order relation
-/// - labels: the number of common significant labels of the two names being
-///   compared
+/// - labels: the number of common significant labels of the two names or
+///   two label sequences being compared
 /// - relationship: see NameComparisonResult::NameRelation
 ///
+/// Note that the ordering is defined for two label sequences that have no
+/// hierarchical relationship (in which case the relationship will be NONE).
+/// For example, two non absolute (or "relative") sequences "example.com" and
+/// "example.org" have no hierarchical relationship, and the former should be
+/// sorted before (i.e. "smaller") than the latter.
 class NameComparisonResult {
 public:
     /// The relation of two names under comparison.
     /// Its semantics for the case of
     /// <code>name1->compare(name2)</code> (where name1 and name2 are instances
-    /// of the Name class) is as follows:
+    /// of the \c Name or \c LabelSequence class) is as follows:
     ///    - SUPERDOMAIN: name1 properly contains name2; name2 is a proper
     ///      subdomain of name1
     ///    - SUBDOMAIN: name1 is a proper subdomain of name2
     ///    - EQUAL: name1 and name2 are equal
     ///    - COMMONANCESTOR: name1 and name2 share a common ancestor
+    ///    - NONE: There's no hierarchical relationship between name1 and name2
     ///
-    /// Note that in our implementation there's always a hierarchical
-    /// relationship between any two names since all names are absolute and
+    /// Note that there's always a hierarchical relationship between any two
+    /// names since all names (not generic label sequences) are absolute and
     /// they at least share the trailing empty label.
     /// So, for example, the relationship between "com." and "net." is
-    /// "commonancestor".  This may be counter intuitive and inconvenient, but
-    /// we'll keep this design at the moment until we decide whether and how to
-    /// handle "non absolute" names (see the description of the \c Name class).
-    /// If we want to (re)introduce the notion of non absolute names, we'll
-    /// want to distinguish "com" and "com.", and the current definition would
-    /// be more compatible for that purpose.
-    /// If, on the other hand, we finally decide we really don't need that
-    /// notion, we'll probably reconsider the design here, too.
+    /// "commonancestor".  The relationship of "NONE" can only happen for
+    /// comparison between two label sequences (\c LabelSequence objects);
+    /// usually only SUPERDOMAIN, SUBDOMAIN or EQUAL are important relationship
+    /// between two names.
+    ///
+    /// When two \c LabelSequence objects are compared, it's generally expected
+    /// they are either both absolute or both non absolute; if one is absolute
+    /// and the other is not, the resulting relationship will be NONE.
     enum NameRelation {
         SUPERDOMAIN = 0,
         SUBDOMAIN = 1,

src/lib/dns/tests/labelsequence_unittest.cc

@@ -21,11 +21,17 @@
 #include <boost/functional/hash.hpp>
 
 #include <string>
+#include <vector>
+#include <utility>
 #include <set>
 
 using namespace isc::dns;
 using namespace std;
 
+// XXX: this is defined as class static constants, but some compilers
+// seemingly cannot find the symbols when used in the EXPECT_xxx macros.
+const size_t LabelSequence::MAX_SERIALIZED_LENGTH;
+
 namespace {
 
 class LabelSequenceTest : public ::testing::Test {
@@ -38,6 +44,13 @@ public:
                           n10("\\000xample.org"),
                           n11("\\000xample.com"),
                           n12("\\000xamplE.com"),
+                          n_maxlabel("0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9."
+                                     "0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9."
+                                     "0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9."
+                                     "0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9."
+                                     "0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9."
+                                     "0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9."
+                                     "0.1.2.3.4.5.6"),
                           ls1(n1), ls2(n2), ls3(n3), ls4(n4), ls5(n5),
                           ls6(n6), ls7(n7), ls8(n8),
                           ls9(n9), ls10(n10), ls11(n11), ls12(n12)
@@ -46,6 +59,7 @@ public:
     // the labelsequences
     Name n1, n2, n3, n4, n5, n6, n7, n8;
     Name n9, n10, n11, n12;
+    const Name n_maxlabel;
 
     LabelSequence ls1, ls2, ls3, ls4, ls5, ls6, ls7, ls8;
     LabelSequence ls9, ls10, ls11, ls12;
@@ -189,12 +203,11 @@ TEST_F(LabelSequenceTest, compare) {
     LabelSequence lsc(nc);
 
     // "g.f.e.d.c.example.org." and "b.example.org" (not absolute), case
-    // in-sensitive
+    // in-sensitive; the absolute one is always smaller.
     lsb.stripRight(1);
     result = lsc.compare(lsb);
-    EXPECT_EQ(isc::dns::NameComparisonResult::NONE,
-              result.getRelation());
-    EXPECT_EQ(0, result.getOrder());
+    EXPECT_EQ(isc::dns::NameComparisonResult::NONE, result.getRelation());
+    EXPECT_GT(0, result.getOrder());
     EXPECT_EQ(0, result.getCommonLabels());
 
     // "g.f.e.d.c.example.org." and "example.org.", case in-sensitive
@@ -270,13 +283,13 @@ TEST_F(LabelSequenceTest, compare) {
     Name ng("w.x.y.isc.EXAMPLE.org");
     LabelSequence lsg(ng);
 
-    // "a.b.c.isc.example.org." and "w.x.y.isc.EXAMPLE.org" (not
-    // absolute), case in-sensitive
+    // lsf: "a.b.c.isc.example.org."
+    // lsg: "w.x.y.isc.EXAMPLE.org" (not absolute), case in-sensitive.
+    // the absolute one is always smaller.
     lsg.stripRight(1);
-    result = lsg.compare(lsf);
-    EXPECT_EQ(isc::dns::NameComparisonResult::NONE,
-              result.getRelation());
-    EXPECT_EQ(0, result.getOrder());
+    result = lsg.compare(lsf);  // lsg > lsf
+    EXPECT_EQ(isc::dns::NameComparisonResult::NONE, result.getRelation());
+    EXPECT_LT(0, result.getOrder());
     EXPECT_EQ(0, result.getCommonLabels());
 
     // "a.b.c.isc.example.org" (not absolute) and
@@ -298,14 +311,23 @@ TEST_F(LabelSequenceTest, compare) {
     EXPECT_LT(0, result.getOrder());
     EXPECT_EQ(2, result.getCommonLabels());
 
-    // "a.b.c" (not absolute) and
-    // "w.x.y" (not absolute), case in-sensitive
+    // lsf: "a.b.c" (not absolute) and
+    // lsg: "w.x.y" (not absolute), case in-sensitive; a.b.c < w.x.y;
+    // no common labels.
     lsf.stripRight(2);
     lsg.stripRight(2);
-    result = lsg.compare(lsf);
-    EXPECT_EQ(isc::dns::NameComparisonResult::NONE,
-              result.getRelation());
-    EXPECT_EQ(0, result.getOrder());
+    result = lsf.compare(lsg);
+    EXPECT_EQ(isc::dns::NameComparisonResult::NONE, result.getRelation());
+    EXPECT_GT(0, result.getOrder());
+    EXPECT_EQ(0, result.getCommonLabels());
+
+    // lsf2: a.b.cc (not absolute); a.b.c < a.b.cc, no common labels.
+    const Name nf2("a.b.cc");
+    LabelSequence lsf2(nf2);
+    lsf2.stripRight(1);
+    result = lsf.compare(lsf2);
+    EXPECT_EQ(isc::dns::NameComparisonResult::NONE, result.getRelation());
+    EXPECT_GT(0, result.getOrder());
     EXPECT_EQ(0, result.getCommonLabels());
 
     Name nh("aexample.org");
@@ -324,13 +346,13 @@ TEST_F(LabelSequenceTest, compare) {
     EXPECT_EQ(1, result.getCommonLabels());
 
     // "aexample" (not absolute) and
-    // "bexample" (not absolute), case in-sensitive
+    // "bexample" (not absolute), case in-sensitive;
+    // aexample < bexample; no common labels.
     lsh.stripRight(1);
     lsi.stripRight(1);
     result = lsh.compare(lsi);
-    EXPECT_EQ(isc::dns::NameComparisonResult::NONE,
-              result.getRelation());
-    EXPECT_EQ(0, result.getOrder());
+    EXPECT_EQ(isc::dns::NameComparisonResult::NONE, result.getRelation());
+    EXPECT_GT(0, result.getOrder());
     EXPECT_EQ(0, result.getCommonLabels());
 
     Name nj("example.org");
@@ -400,93 +422,6 @@ TEST_F(LabelSequenceTest, getData) {
     getDataCheck("\000", 1, ls7);
 };
 
-TEST_F(LabelSequenceTest, getOffsetData) {
-    size_t len;
-    uint8_t placeholder[Name::MAX_LABELS];
-
-    Name nx("x.isc.example.org");
-    LabelSequence lsx(nx);
-
-    // x.isc.example.org.
-    lsx.getOffsetData(&len, placeholder);
-    EXPECT_EQ(5, len);
-    EXPECT_EQ(0, placeholder[0]);
-    EXPECT_EQ(2, placeholder[1]);
-    EXPECT_EQ(6, placeholder[2]);
-    EXPECT_EQ(14, placeholder[3]);
-    EXPECT_EQ(18, placeholder[4]);
-
-    lsx.stripLeft(2);
-
-    // example.org.
-    lsx.getOffsetData(&len, placeholder);
-    EXPECT_EQ(3, len);
-    EXPECT_EQ(0, placeholder[0]);
-    EXPECT_EQ(8, placeholder[1]);
-    EXPECT_EQ(12, placeholder[2]);
-
-    lsx.stripLeft(1);
-
-    // org.
-    lsx.getOffsetData(&len, placeholder);
-    EXPECT_EQ(2, len);
-    EXPECT_EQ(0, placeholder[0]);
-    EXPECT_EQ(4, placeholder[1]);
-
-    lsx.stripLeft(1);
-
-    // .
-    lsx.getOffsetData(&len, placeholder);
-    EXPECT_EQ(1, len);
-    EXPECT_EQ(0, placeholder[0]);
-
-    Name ny("y.isc.example.org");
-    LabelSequence lsy(ny);
-
-    // y.isc.example.org.
-    lsy.getOffsetData(&len, placeholder);
-    EXPECT_EQ(5, len);
-    EXPECT_EQ(0, placeholder[0]);
-    EXPECT_EQ(2, placeholder[1]);
-    EXPECT_EQ(6, placeholder[2]);
-    EXPECT_EQ(14, placeholder[3]);
-    EXPECT_EQ(18, placeholder[4]);
-
-    lsy.stripRight(1);
-
-    // y.isc.example.org
-    lsy.getOffsetData(&len, placeholder);
-    EXPECT_EQ(4, len);
-    EXPECT_EQ(0, placeholder[0]);
-    EXPECT_EQ(2, placeholder[1]);
-    EXPECT_EQ(6, placeholder[2]);
-    EXPECT_EQ(14, placeholder[3]);
-
-    lsy.stripRight(1);
-
-    // y.isc.example
-    lsy.getOffsetData(&len, placeholder);
-    EXPECT_EQ(3, len);
-    EXPECT_EQ(0, placeholder[0]);
-    EXPECT_EQ(2, placeholder[1]);
-    EXPECT_EQ(6, placeholder[2]);
-
-    lsy.stripLeft(1);
-
-    // isc.example
-    lsy.getOffsetData(&len, placeholder);
-    EXPECT_EQ(2, len);
-    EXPECT_EQ(0, placeholder[0]);
-    EXPECT_EQ(4, placeholder[1]);
-
-    lsy.stripLeft(1);
-
-    // example
-    lsy.getOffsetData(&len, placeholder);
-    EXPECT_EQ(1, len);
-    EXPECT_EQ(0, placeholder[0]);
-};
-
 TEST_F(LabelSequenceTest, stripLeft) {
     EXPECT_TRUE(ls1.equals(ls3));
     ls1.stripLeft(0);
@@ -646,14 +581,7 @@ TEST_F(LabelSequenceTest, toText) {
               "012345678901234567890123456789"
               "0123456789012345678901234567890", ls_long1.toText());
 
-    Name n_long2("0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9."
-                 "0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9."
-                 "0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9."
-                 "0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9."
-                 "0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9."
-                 "0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9."
-                 "0.1.2.3.4.5.6");
-    LabelSequence ls_long2(n_long2);
+    LabelSequence ls_long2(n_maxlabel);
 
     EXPECT_EQ("0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9."
               "0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9."
@@ -762,78 +690,98 @@ TEST_F(LabelSequenceTest, LeftShiftOperator) {
     EXPECT_EQ(ls1.toText(), oss.str());
 }
 
-// Test different ways of construction, and see if they compare
-TEST(LabelSequence, rawConstruction) {
-    Name n("example.org");
-
-    uint8_t data[] = { 0x07, 'e', 'x', 'a', 'm', 'p', 'l', 'e',
-                       0x03, 'o', 'r', 'g',
-                       0x00 };
-    uint8_t offsets[] = { 0, 8, 12 };
-    size_t offsets_size = 3;
-
-    LabelSequence s1(n);
-    LabelSequence s2(s1);
-    LabelSequence s3(data, offsets, offsets_size);
-
-    // Assuming equality is transitive, so only comparing 1 to 2 and 1 to 3
-    NameComparisonResult result = s1.compare(s2);
-    EXPECT_EQ(isc::dns::NameComparisonResult::EQUAL,
-              result.getRelation());
-    EXPECT_EQ(0, result.getOrder());
-    EXPECT_EQ(3, result.getCommonLabels());
-
-    result = s1.compare(s3);
-    EXPECT_EQ(isc::dns::NameComparisonResult::EQUAL,
-              result.getRelation());
-    EXPECT_EQ(0, result.getOrder());
-    EXPECT_EQ(3, result.getCommonLabels());
-
-    // Modify the data and make sure it's not equal anymore
-    data[2] = 'f';
-    result = s1.compare(s3);
-    EXPECT_EQ(isc::dns::NameComparisonResult::COMMONANCESTOR,
-              result.getRelation());
-    EXPECT_EQ(2, result.getCommonLabels());
-
-    s1.stripRight(1);
-    s3.stripRight(1);
-
-    result = s1.compare(s3);
-    EXPECT_EQ(isc::dns::NameComparisonResult::COMMONANCESTOR,
-              result.getRelation());
-    EXPECT_EQ(1, result.getCommonLabels());
+TEST_F(LabelSequenceTest, serialize) {
+    // placeholder for serialized data
+    uint8_t labels_buf[LabelSequence::MAX_SERIALIZED_LENGTH];
+
+    // vector to store expected and actual data
+    vector<LabelSequence> actual_labelseqs;
+    typedef pair<size_t, const uint8_t*> DataPair;
+    vector<DataPair> expected;
+
+    // An absolute sequence directly constructed from a valid name.
+    // labels = 3, offset sequence = 0, 8, 12, data = "example.com."
+    actual_labelseqs.push_back(ls1);
+    const uint8_t expected_data1[] = {
+        3, 0, 8, 12, 7, 'e', 'x', 'a', 'm', 'p', 'l', 'e',
+        3, 'o', 'r', 'g', 0 };
+    expected.push_back(DataPair(sizeof(expected_data1), expected_data1));
+
+    // Strip the original one from right.
+    // labels = 2, offset sequence = 0, 8, data = "example.com" (non absolute)
+    LabelSequence ls_rstripped = ls1;
+    ls_rstripped.stripRight(1);
+    actual_labelseqs.push_back(ls_rstripped);
+    const uint8_t expected_data2[] = {
+        2, 0, 8, 7, 'e', 'x', 'a', 'm', 'p', 'l', 'e',
+        3, 'o', 'r', 'g'};
+    expected.push_back(DataPair(sizeof(expected_data2), expected_data2));
+
+    // Strip the original one from left.
+    // labels = 2, offset sequence = 0, 4, data = "com."
+    // Note that offsets are adjusted so that they begin with 0.
+    LabelSequence ls_lstripped = ls1;
+    ls_lstripped.stripLeft(1);
+    actual_labelseqs.push_back(ls_lstripped);
+    const uint8_t expected_data3[] = { 2, 0, 4, 3, 'o', 'r', 'g', 0 };
+    expected.push_back(DataPair(sizeof(expected_data3), expected_data3));
+
+    // Root label.
+    LabelSequence ls_root(Name::ROOT_NAME());
+    actual_labelseqs.push_back(ls_root);
+    const uint8_t expected_data4[] = { 1, 0, 0 };
+    expected.push_back(DataPair(sizeof(expected_data4), expected_data4));
+
+    // Non absolute single-label.
+    LabelSequence ls_single = ls_rstripped;
+    ls_single.stripRight(1);
+    actual_labelseqs.push_back(ls_single);
+    const uint8_t expected_data5[] = {
+        1, 0, 7, 'e', 'x', 'a', 'm', 'p', 'l', 'e' };
+    expected.push_back(DataPair(sizeof(expected_data5), expected_data5));
+
+    // For each data set, serialize the labels and compare the data to the
+    // expected one.
+    vector<DataPair>::const_iterator it = expected.begin();
+    vector<LabelSequence>::const_iterator itl = actual_labelseqs.begin();
+    for (; it != expected.end(); ++it, ++itl) {
+        SCOPED_TRACE(itl->toText());
+
+        const size_t serialized_len = itl->getSerializedLength();
+
+        ASSERT_GE(LabelSequence::MAX_SERIALIZED_LENGTH, serialized_len);
+        itl->serialize(labels_buf, serialized_len);
+        EXPECT_EQ(it->first, serialized_len);
+        EXPECT_EQ(0, memcmp(it->second, labels_buf, serialized_len));
+
+        EXPECT_EQ(NameComparisonResult::EQUAL,
+                  LabelSequence(labels_buf).compare(*itl).getRelation());
+    }
 
-    data[9] = 'f';
-    result = s1.compare(s3);
-    EXPECT_EQ(isc::dns::NameComparisonResult::NONE,
-              result.getRelation());
-    EXPECT_EQ(0, result.getCommonLabels());
+    EXPECT_THROW(ls1.serialize(labels_buf, ls1.getSerializedLength() - 1),
+                 isc::BadValue);
 }
 
-// Test with some data that exceeds limits (MAX_LABELS and MAX_LABEL_LEN)
-TEST(LabelSequence, badRawConstruction) {
-    uint8_t data[1] = { 0 };
-    uint8_t offsets[1] = { 0 };
-
-    EXPECT_THROW(LabelSequence(NULL, offsets, 1), isc::BadValue);
-    EXPECT_THROW(LabelSequence(data, NULL, 1), isc::BadValue);
-    EXPECT_THROW(LabelSequence(data, offsets, 0), isc::BadValue);
-
-    // exceed MAX_LABELS
-    EXPECT_THROW(LabelSequence(data, offsets, 127), isc::BadValue);
+TEST_F(LabelSequenceTest, badDeserialize) {
+    EXPECT_THROW(LabelSequence(NULL), isc::BadValue);
+    const uint8_t zero_offsets[] = { 0 };
+    EXPECT_THROW(LabelSequence ls(zero_offsets), isc::BadValue);
+    const uint8_t toomany_offsets[] = { Name::MAX_LABELS + 1 };
+    EXPECT_THROW(LabelSequence ls(toomany_offsets), isc::BadValue);
 
     // exceed MAX_LABEL_LEN
-    uint8_t offsets_toolonglabel[1] = { 64 };
-    EXPECT_THROW(LabelSequence(data, offsets_toolonglabel, 1), isc::BadValue);
-
-    // Add an offset that is lower than the previous offset
-    uint8_t offsets_lower[3] = { 0, 8, 4 };
-    EXPECT_THROW(LabelSequence(data, offsets_lower, 3), isc::BadValue);
-
-    // Add an offset that is equal to the previous offset
-    uint8_t offsets_noincrease[3] = { 0, 8, 8 };
-    EXPECT_THROW(LabelSequence(data, offsets_noincrease, 3), isc::BadValue);
+    const uint8_t offsets_toolonglabel[] = { 2, 0, 64 };
+    EXPECT_THROW(LabelSequence ls(offsets_toolonglabel), isc::BadValue);
+
+    // Inconsistent data: an offset is lower than the previous offset
+    const uint8_t offsets_lower[] = { 3, // # of offsets
+                                      0, 2, 1, // offsets
+                                      1, 'a', 1, 'b', 0};
+    EXPECT_THROW(LabelSequence ls(offsets_lower), isc::BadValue);
+
+    // Inconsistent data: an offset is equal to the previous offset
+    const uint8_t offsets_noincrease[] = { 2, 0, 0, 0, 0 };
+    EXPECT_THROW(LabelSequence ls(offsets_noincrease), isc::BadValue);
 }
 
 }