iputils.hh: NetmaskTree: Make tree cleanup mandatory.

Potentially leaving branches full of unassigned nodes unnecessarily complicates
tree algorithms. Disabling tree cleanup was not used anywhere, except for a unit
test. Note that, after this change, individual branch nodes can still be
unassigned, but not the whole branch. So, when e.g. the left sub-branch of a
node exists, algorithms can rely on the fact that there is at least one assigned
node in there.

diff --git a/pdns/iputils.hh b/pdns/iputils.hh
index ea9703076442f468aa699983226d7f7e16b676c6..9a66e13b84a88ce18a4899fb78d1377d31660708 100644 (file)
--- a/pdns/iputils.hh
+++ b/pdns/iputils.hh
@@ -790,13 +790,10 @@ private:
   }
 
 public:
-  NetmaskTree() noexcept : NetmaskTree(false) {
+  NetmaskTree() noexcept : d_root(new TreeNode()) {
   }
 
-  NetmaskTree(bool cleanup) noexcept : d_root(new TreeNode()), d_cleanup_tree(cleanup) {
-  }
-
-  NetmaskTree(const NetmaskTree& rhs): d_root(new TreeNode()), d_cleanup_tree(rhs.d_cleanup_tree) {
+  NetmaskTree(const NetmaskTree& rhs): d_root(new TreeNode()) {
     // it is easier to copy the nodes than tree.
     // also acts as handy compactor
     for(auto const& node: rhs._nodes)
@@ -808,7 +805,6 @@ public:
     // see above.
     for(auto const& node: rhs._nodes)
       insert(node->first).second = node->second;
-    d_cleanup_tree = rhs.d_cleanup_tree;
     return *this;
   }
 
@@ -1031,9 +1027,7 @@ public:
       _nodes.erase(node->node.get());
       node->assigned = false;
       node->node->second = value_type();
-
-      if (d_cleanup_tree)
-        cleanup_tree(node);
+      cleanup_tree(node);
     }
   }
 
@@ -1075,7 +1069,6 @@ public:
 private:
   unique_ptr<TreeNode> d_root; //<! Root of our tree
   std::set<node_type*> _nodes; //<! Container for actual values
-  bool d_cleanup_tree; //<! Whether or not to cleanup the tree on erase
 };
 
 /** This class represents a group of supplemental Netmask classes. An IP address matchs
@@ -1084,12 +1077,7 @@ private:
 class NetmaskGroup
 {
 public:
-  //! By default, initialise the tree to cleanup
-  NetmaskGroup() noexcept : NetmaskGroup(true) {
-  }
-
-  //! This allows control over whether to cleanup or not
-  NetmaskGroup(bool cleanup) noexcept : tree(cleanup) {
+  NetmaskGroup() noexcept {
   }
 
   //! If this IP address is matched by any of the classes within

diff --git a/pdns/recursor_cache.hh b/pdns/recursor_cache.hh
index d298671ba0b7fef35fb8b7bc41b85da637554356..00348c4b5744fddac03339d18a122c6a37435a27 100644 (file)
--- a/pdns/recursor_cache.hh
+++ b/pdns/recursor_cache.hh
@@ -107,7 +107,7 @@ private:
   class ECSIndexEntry
   {
   public:
-    ECSIndexEntry(const DNSName& qname, uint16_t qtype): d_nmt(true), d_qname(qname), d_qtype(qtype)
+    ECSIndexEntry(const DNSName& qname, uint16_t qtype): d_nmt(), d_qname(qname), d_qtype(qtype)
     {
     }
 

diff --git a/pdns/test-iputils_hh.cc b/pdns/test-iputils_hh.cc
index 5820c87d6faecc103ad0c1ef08ab6278cc3877a3..dc14bf558d3b232fa85458c58304a2df02e59062 100644 (file)
--- a/pdns/test-iputils_hh.cc
+++ b/pdns/test-iputils_hh.cc
@@ -478,7 +478,7 @@ BOOST_AUTO_TEST_CASE(test_scale) {
 
 BOOST_AUTO_TEST_CASE(test_removal) {
   std::string prefix = "192.";
-  NetmaskTree<int> nmt(true);
+  NetmaskTree<int> nmt;
   BOOST_CHECK(nmt.empty());
   BOOST_CHECK_EQUAL(nmt.size(), 0);