Use std::unique_ptr for DHTBucketTree's left and right pointers

src/DHTBucket.cc

@@ -179,7 +179,7 @@ bool DHTBucket::splitAllowed() const
   return prefixLength_ < DHT_ID_LENGTH*8-1 && isInRange(localNode_);
 }
 
-std::shared_ptr<DHTBucket> DHTBucket::split()
+std::unique_ptr<DHTBucket> DHTBucket::split()
 {
   assert(splitAllowed());
 
@@ -192,10 +192,9 @@ std::shared_ptr<DHTBucket> DHTBucket::split()
   bitfield::flipBit(min_, DHT_ID_LENGTH, prefixLength_);
 
   ++prefixLength_;
-  auto rBucket = std::make_shared<DHTBucket>(prefixLength_, rMax, rMin,
-                                             localNode_);
+  auto rBucket = make_unique<DHTBucket>(prefixLength_, rMax, rMin, localNode_);
 
-  std::deque<std::shared_ptr<DHTNode> > lNodes;
+  std::deque<std::shared_ptr<DHTNode>> lNodes;
   for(auto & elem : nodes_) {
     if(rBucket->isInRange(elem)) {
       assert(rBucket->addNode(elem));

src/DHTBucket.h

@@ -86,7 +86,7 @@ public:
 
   void getRandomNodeID(unsigned char* nodeID) const;
 
-  std::shared_ptr<DHTBucket> split();
+  std::unique_ptr<DHTBucket> split();
 
   bool isInRange(const std::shared_ptr<DHTNode>& node) const;
 

src/DHTBucketTree.cc

@@ -39,34 +39,30 @@
 
 #include "DHTBucket.h"
 #include "DHTNode.h"
+#include "a2functional.h"
 
 namespace aria2 {
 
 DHTBucketTreeNode::DHTBucketTreeNode
-(DHTBucketTreeNode* left,
- DHTBucketTreeNode* right)
+(std::unique_ptr<DHTBucketTreeNode> left,
+ std::unique_ptr<DHTBucketTreeNode> right)
   : parent_(nullptr),
-    left_(left),
-    right_(right)
+    left_(std::move(left)),
+    right_(std::move(right))
 {
   resetRelation();
 }
 
-DHTBucketTreeNode::DHTBucketTreeNode(const std::shared_ptr<DHTBucket>& bucket)
+DHTBucketTreeNode::DHTBucketTreeNode(std::shared_ptr<DHTBucket> bucket)
   : parent_(nullptr),
-    left_(nullptr),
-    right_(nullptr),
-    bucket_(bucket)
+    bucket_(std::move(bucket))
 {
   memcpy(minId_, bucket_->getMinID(), DHT_ID_LENGTH);
   memcpy(maxId_, bucket_->getMaxID(), DHT_ID_LENGTH);
 }
 
 DHTBucketTreeNode::~DHTBucketTreeNode()
-{
-  delete left_;
-  delete right_;
-}
+{}
 
 void DHTBucketTreeNode::resetRelation()
 {
@@ -82,9 +78,9 @@ DHTBucketTreeNode* DHTBucketTreeNode::dig(const unsigned char* key)
     return nullptr;
   }
   if(left_->isInRange(key)) {
-    return left_;
+    return left_.get();
   } else {
-    return right_;
+    return right_.get();
   }
 }
 
@@ -99,9 +95,8 @@ bool DHTBucketTreeNode::isInRange(const unsigned char* key) const
 
 void DHTBucketTreeNode::split()
 {
-  std::shared_ptr<DHTBucket> leftBucket = bucket_->split();
-  left_ = new DHTBucketTreeNode(leftBucket);
-  right_ = new DHTBucketTreeNode(bucket_);
+  left_ = make_unique<DHTBucketTreeNode>(bucket_->split());
+  right_ = make_unique<DHTBucketTreeNode>(bucket_);
   bucket_.reset();
   resetRelation();
 }

src/DHTBucketTree.h

@@ -53,9 +53,10 @@ class DHTNode;
 class DHTBucketTreeNode {
 public:
   // Ctor for internal node
-  DHTBucketTreeNode(DHTBucketTreeNode* left, DHTBucketTreeNode* right);
+  DHTBucketTreeNode(std::unique_ptr<DHTBucketTreeNode> left,
+                    std::unique_ptr<DHTBucketTreeNode> right);
   // Ctor for leaf node
-  DHTBucketTreeNode(const std::shared_ptr<DHTBucket>& bucket);
+  DHTBucketTreeNode(std::shared_ptr<DHTBucket> bucket);
   ~DHTBucketTreeNode();
   // Returns child node, left or right, which contains key.  If dig is
   // called against leaf node, then returns 0.
@@ -66,8 +67,8 @@ public:
   const unsigned char* getMaxId() const { return maxId_; }
   const unsigned char* getMinId() const { return minId_; }
   DHTBucketTreeNode* getParent() const { return parent_; }
-  DHTBucketTreeNode* getLeft() const { return left_; }
-  DHTBucketTreeNode* getRight() const { return right_; }
+  DHTBucketTreeNode* getLeft() const { return left_.get(); }
+  DHTBucketTreeNode* getRight() const { return right_.get(); }
   const std::shared_ptr<DHTBucket>& getBucket() const { return bucket_; }
   // Splits this object's bucket using DHTBucket::split() and create
   // left and right child node to hold buckets. The bucket of current
@@ -83,8 +84,8 @@ private:
   void resetRelation();
   void setParent(DHTBucketTreeNode* parent) { parent_ = parent; }
   DHTBucketTreeNode* parent_;
-  DHTBucketTreeNode* left_;
-  DHTBucketTreeNode* right_;
+  std::unique_ptr<DHTBucketTreeNode> left_;
+  std::unique_ptr<DHTBucketTreeNode> right_;
   std::shared_ptr<DHTBucket> bucket_;
   unsigned char minId_[DHT_ID_LENGTH];
   unsigned char maxId_[DHT_ID_LENGTH];

test/DHTBucketTreeTest.cc

@@ -33,11 +33,11 @@ void DHTBucketTreeTest::testDig()
   unsigned char localNodeID[DHT_ID_LENGTH];
   memset(localNodeID, 0xff, DHT_ID_LENGTH);
 
-  std::shared_ptr<DHTNode> localNode(new DHTNode(localNodeID));
+  auto localNode = std::make_shared<DHTNode>(localNodeID);
 
-  std::shared_ptr<DHTBucket> bucket1(new DHTBucket(localNode));
-  std::shared_ptr<DHTBucket> bucket2 = bucket1->split();
-  std::shared_ptr<DHTBucket> bucket3 = bucket1->split();
+  auto bucket1 = std::make_shared<DHTBucket>(localNode);
+  auto bucket2 = std::shared_ptr<DHTBucket>(bucket1->split());
+  auto bucket3 = std::shared_ptr<DHTBucket>(bucket1->split());
   // Tree: number is prefix
   //
   //           +
@@ -53,10 +53,9 @@ void DHTBucketTreeTest::testDig()
     CPPUNIT_ASSERT(!b.dig(localNode->getID()));
   }
   {
-    DHTBucketTreeNode* left = new DHTBucketTreeNode(bucket3);
-    DHTBucketTreeNode* right = new DHTBucketTreeNode(bucket1);
-    DHTBucketTreeNode b(left, right);
-    CPPUNIT_ASSERT(b.dig(localNode->getID()) == right);
+    DHTBucketTreeNode b(make_unique<DHTBucketTreeNode>(bucket3),
+                        make_unique<DHTBucketTreeNode>(bucket1));
+    CPPUNIT_ASSERT(b.dig(localNode->getID()) == b.getRight());
   }
 }
 
@@ -65,13 +64,13 @@ void DHTBucketTreeTest::testFindBucketFor()
   unsigned char localNodeID[DHT_ID_LENGTH];
   memset(localNodeID, 0xaa, DHT_ID_LENGTH);
 
-  std::shared_ptr<DHTNode> localNode(new DHTNode(localNodeID));
+  auto localNode = std::make_shared<DHTNode>(localNodeID);
 
-  std::shared_ptr<DHTBucket> bucket1(new DHTBucket(localNode));
-  std::shared_ptr<DHTBucket> bucket2 = bucket1->split();
-  std::shared_ptr<DHTBucket> bucket3 = bucket1->split();
-  std::shared_ptr<DHTBucket> bucket4 = bucket3->split();
-  std::shared_ptr<DHTBucket> bucket5 = bucket3->split();
+  auto bucket1 = std::make_shared<DHTBucket>(localNode);
+  auto bucket2 = std::shared_ptr<DHTBucket>(bucket1->split());
+  auto bucket3 = std::shared_ptr<DHTBucket>(bucket1->split());
+  auto bucket4 = std::shared_ptr<DHTBucket>(bucket3->split());
+  auto bucket5 = std::shared_ptr<DHTBucket>(bucket3->split());
 
   {
     DHTBucketTreeNode b(bucket5);
@@ -92,16 +91,16 @@ void DHTBucketTreeTest::testFindBucketFor()
     //          1010         1011
     //           |
     //    localNode is here
-    DHTBucketTreeNode* b1 = new DHTBucketTreeNode(bucket1);
-    DHTBucketTreeNode* b2 = new DHTBucketTreeNode(bucket2);
-    DHTBucketTreeNode* b3 = new DHTBucketTreeNode(bucket3);
-    DHTBucketTreeNode* b4 = new DHTBucketTreeNode(bucket4);
-    DHTBucketTreeNode* b5 = new DHTBucketTreeNode(bucket5);
+    auto b1 = make_unique<DHTBucketTreeNode>(bucket1);
+    auto b2 = make_unique<DHTBucketTreeNode>(bucket2);
+    auto b3 = make_unique<DHTBucketTreeNode>(bucket3);
+    auto b4 = make_unique<DHTBucketTreeNode>(bucket4);
+    auto b5 = make_unique<DHTBucketTreeNode>(bucket5);
 
-    DHTBucketTreeNode* bp1 = new DHTBucketTreeNode(b5, b3);
-    DHTBucketTreeNode* bp2 = new DHTBucketTreeNode(b4, bp1);
-    DHTBucketTreeNode* bp3 = new DHTBucketTreeNode(bp2, b1);
-    DHTBucketTreeNode bp4(b2, bp3);
+    auto bp1 = make_unique<DHTBucketTreeNode>(std::move(b5), std::move(b3));
+    auto bp2 = make_unique<DHTBucketTreeNode>(std::move(b4), std::move(bp1));
+    auto bp3 = make_unique<DHTBucketTreeNode>(std::move(bp2), std::move(b1));
+    DHTBucketTreeNode bp4(std::move(b2), std::move(bp3));
 
     CPPUNIT_ASSERT(*bucket5 == *dht::findBucketFor(&bp4, localNode->getID()));
   }
@@ -112,43 +111,43 @@ void DHTBucketTreeTest::testFindClosestKNodes()
   unsigned char localNodeID[DHT_ID_LENGTH];
   memset(localNodeID, 0xaa, DHT_ID_LENGTH);
 
-  std::shared_ptr<DHTNode> localNode(new DHTNode(localNodeID));
+  auto localNode = std::make_shared<DHTNode>(localNodeID);
 
-  std::shared_ptr<DHTBucket> bucket1(new DHTBucket(localNode));
-  std::shared_ptr<DHTBucket> bucket2 = bucket1->split();
-  std::shared_ptr<DHTBucket> bucket3 = bucket1->split();
-  std::shared_ptr<DHTBucket> bucket4 = bucket3->split();
-  std::shared_ptr<DHTBucket> bucket5 = bucket3->split();
+  auto bucket1 = std::make_shared<DHTBucket>(localNode);
+  auto bucket2 = std::shared_ptr<DHTBucket>(bucket1->split());
+  auto bucket3 = std::shared_ptr<DHTBucket>(bucket1->split());
+  auto bucket4 = std::shared_ptr<DHTBucket>(bucket3->split());
+  auto bucket5 = std::shared_ptr<DHTBucket>(bucket3->split());
 
   unsigned char id[DHT_ID_LENGTH];
   {
-    DHTBucketTreeNode* b1 = new DHTBucketTreeNode(bucket1);
-    DHTBucketTreeNode* b2 = new DHTBucketTreeNode(bucket2);
-    DHTBucketTreeNode* b3 = new DHTBucketTreeNode(bucket3);
-    DHTBucketTreeNode* b4 = new DHTBucketTreeNode(bucket4);
-    DHTBucketTreeNode* b5 = new DHTBucketTreeNode(bucket5);
+    auto b1 = make_unique<DHTBucketTreeNode>(bucket1);
+    auto b2 = make_unique<DHTBucketTreeNode>(bucket2);
+    auto b3 = make_unique<DHTBucketTreeNode>(bucket3);
+    auto b4 = make_unique<DHTBucketTreeNode>(bucket4);
+    auto b5 = make_unique<DHTBucketTreeNode>(bucket5);
 
-    DHTBucketTreeNode* bp1 = new DHTBucketTreeNode(b5, b3);
-    DHTBucketTreeNode* bp2 = new DHTBucketTreeNode(b4, bp1);
-    DHTBucketTreeNode* bp3 = new DHTBucketTreeNode(bp2, b1);
-    DHTBucketTreeNode bp4(b2, bp3);
+    auto bp1 = make_unique<DHTBucketTreeNode>(std::move(b5), std::move(b3));
+    auto bp2 = make_unique<DHTBucketTreeNode>(std::move(b4), std::move(bp1));
+    auto bp3 = make_unique<DHTBucketTreeNode>(std::move(bp2), std::move(b1));
+    DHTBucketTreeNode bp4(std::move(b2), std::move(bp3));
 
     for(size_t i = 0; i < 2; ++i) {
       bucket1->getRandomNodeID(id);
-      bucket1->addNode(std::shared_ptr<DHTNode>(new DHTNode(id)));
+      bucket1->addNode(std::make_shared<DHTNode>(id));
       bucket2->getRandomNodeID(id);
-      bucket2->addNode(std::shared_ptr<DHTNode>(new DHTNode(id)));
+      bucket2->addNode(std::make_shared<DHTNode>(id));
       bucket3->getRandomNodeID(id);
-      bucket3->addNode(std::shared_ptr<DHTNode>(new DHTNode(id)));
+      bucket3->addNode(std::make_shared<DHTNode>(id));
       bucket4->getRandomNodeID(id);
-      bucket4->addNode(std::shared_ptr<DHTNode>(new DHTNode(id)));
+      bucket4->addNode(std::make_shared<DHTNode>(id));
       bucket5->getRandomNodeID(id);
-      bucket5->addNode(std::shared_ptr<DHTNode>(new DHTNode(id)));
+      bucket5->addNode(std::make_shared<DHTNode>(id));
     }
     {
       unsigned char targetID[DHT_ID_LENGTH];
       memset(targetID, 0x80, DHT_ID_LENGTH);
-      std::vector<std::shared_ptr<DHTNode> > nodes;
+      std::vector<std::shared_ptr<DHTNode>> nodes;
       dht::findClosestKNodes(nodes, &bp4, targetID);
       CPPUNIT_ASSERT_EQUAL((size_t)8, nodes.size());
       CPPUNIT_ASSERT(bucket4->isInRange(nodes[0]));
@@ -163,7 +162,7 @@ void DHTBucketTreeTest::testFindClosestKNodes()
     {
       unsigned char targetID[DHT_ID_LENGTH];
       memset(targetID, 0xf0, DHT_ID_LENGTH);
-      std::vector<std::shared_ptr<DHTNode> > nodes;
+      std::vector<std::shared_ptr<DHTNode>> nodes;
       dht::findClosestKNodes(nodes, &bp4, targetID);
       CPPUNIT_ASSERT_EQUAL((size_t)8, nodes.size());
       CPPUNIT_ASSERT(bucket1->isInRange(nodes[0]));
@@ -178,11 +177,11 @@ void DHTBucketTreeTest::testFindClosestKNodes()
     {
       for(size_t i = 0; i < 6; ++i) {
         bucket4->getRandomNodeID(id);
-        bucket4->addNode(std::shared_ptr<DHTNode>(new DHTNode(id)));
+        bucket4->addNode(std::make_shared<DHTNode>(id));
       }
       unsigned char targetID[DHT_ID_LENGTH];
       memset(targetID, 0x80, DHT_ID_LENGTH);
-      std::vector<std::shared_ptr<DHTNode> > nodes;
+      std::vector<std::shared_ptr<DHTNode>> nodes;
       dht::findClosestKNodes(nodes, &bp4, targetID);
       CPPUNIT_ASSERT_EQUAL((size_t)8, nodes.size());
       for(size_t i = 0; i < DHTBucket::K; ++i) {
@@ -198,34 +197,34 @@ void DHTBucketTreeTest::testEnumerateBucket()
   unsigned char localNodeID[DHT_ID_LENGTH];
   memset(localNodeID, 0xaa, DHT_ID_LENGTH);
 
-  std::shared_ptr<DHTNode> localNode(new DHTNode(localNodeID));
+  auto localNode = std::make_shared<DHTNode>(localNodeID);
 
-  std::shared_ptr<DHTBucket> bucket1(new DHTBucket(localNode));
-  std::shared_ptr<DHTBucket> bucket2 = bucket1->split();
-  std::shared_ptr<DHTBucket> bucket3 = bucket1->split();
-  std::shared_ptr<DHTBucket> bucket4 = bucket3->split();
-  std::shared_ptr<DHTBucket> bucket5 = bucket3->split();
+  auto bucket1 = std::make_shared<DHTBucket>(localNode);
+  auto bucket2 = std::shared_ptr<DHTBucket>(bucket1->split());
+  auto bucket3 = std::shared_ptr<DHTBucket>(bucket1->split());
+  auto bucket4 = std::shared_ptr<DHTBucket>(bucket3->split());
+  auto bucket5 = std::shared_ptr<DHTBucket>(bucket3->split());
 
   {
     DHTBucketTreeNode b(bucket1);
-    std::vector<std::shared_ptr<DHTBucket> > buckets;
+    std::vector<std::shared_ptr<DHTBucket>> buckets;
     dht::enumerateBucket(buckets, &b);
     CPPUNIT_ASSERT_EQUAL((size_t)1, buckets.size());
     CPPUNIT_ASSERT(*bucket1 == *buckets[0]);
   }
   {
-    DHTBucketTreeNode* b1 = new DHTBucketTreeNode(bucket1);
-    DHTBucketTreeNode* b2 = new DHTBucketTreeNode(bucket2);
-    DHTBucketTreeNode* b3 = new DHTBucketTreeNode(bucket3);
-    DHTBucketTreeNode* b4 = new DHTBucketTreeNode(bucket4);
-    DHTBucketTreeNode* b5 = new DHTBucketTreeNode(bucket5);
-
-    DHTBucketTreeNode* bp1 = new DHTBucketTreeNode(b5, b3);
-    DHTBucketTreeNode* bp2 = new DHTBucketTreeNode(b4, bp1);
-    DHTBucketTreeNode* bp3 = new DHTBucketTreeNode(bp2, b1);
-    DHTBucketTreeNode bp4(b2, bp3);
-
-    std::vector<std::shared_ptr<DHTBucket> > buckets;
+    auto b1 = make_unique<DHTBucketTreeNode>(bucket1);
+    auto b2 = make_unique<DHTBucketTreeNode>(bucket2);
+    auto b3 = make_unique<DHTBucketTreeNode>(bucket3);
+    auto b4 = make_unique<DHTBucketTreeNode>(bucket4);
+    auto b5 = make_unique<DHTBucketTreeNode>(bucket5);
+
+    auto bp1 = make_unique<DHTBucketTreeNode>(std::move(b5), std::move(b3));
+    auto bp2 = make_unique<DHTBucketTreeNode>(std::move(b4), std::move(bp1));
+    auto bp3 = make_unique<DHTBucketTreeNode>(std::move(bp2), std::move(b1));
+    DHTBucketTreeNode bp4(std::move(b2), std::move(bp3));
+
+    std::vector<std::shared_ptr<DHTBucket>> buckets;
     dht::enumerateBucket(buckets, &bp4);
     CPPUNIT_ASSERT_EQUAL((size_t)5, buckets.size());
     CPPUNIT_ASSERT(*bucket2 == *buckets[0]);