2008-09-18 Tatsuhiro Tsujikawa <tujikawa at rednoah dot com>

	Fixed the bug that prefixLength of split bucket which doesn't 
include
	local node ID is incremented.
	* src/DHTBucket.cc
	* src/DHTBucket.h
	* test/DHTBucketTest.cc

ChangeLog

@@ -1,3 +1,11 @@
+2008-09-18  Tatsuhiro Tsujikawa  <tujikawa at rednoah dot com>
+
+	Fixed the bug that prefixLength of split bucket which doesn't include
+	local node ID is incremented.
+	* src/DHTBucket.cc
+	* src/DHTBucket.h
+	* test/DHTBucketTest.cc
+
 2008-09-17  Tatsuhiro Tsujikawa  <tujikawa at rednoah dot com>
 
 	Added cookies.sqlite to EXTRA_DIST

src/DHTBucket.cc

@@ -81,22 +81,29 @@ void DHTBucket::getRandomNodeID(unsigned char* nodeID) const
 
 bool DHTBucket::isInRange(const SharedHandle<DHTNode>& node) const
 {
-  return isInRange(node->getID());
+  return isInRange(node->getID(), _max, _min);
 }
 
 bool DHTBucket::isInRange(const unsigned char* nodeID) const
+{
+  return isInRange(nodeID, _max, _min);
+}
+
+bool DHTBucket::isInRange(const unsigned char* nodeID,
+			  const unsigned char* max,
+			  const unsigned char* min) const
 {
   for(size_t i = 0; i < DHT_ID_LENGTH; ++i) {
-    if(nodeID[i] < _min[i]) {
+    if(nodeID[i] < min[i]) {
       return false;
-    } else if(_min[i] < nodeID[i]) {
+    } else if(min[i] < nodeID[i]) {
       break;
     }
   }
   for(size_t i = 0; i < DHT_ID_LENGTH; ++i) {
-    if(_max[i] < nodeID[i]) {
+    if(max[i] < nodeID[i]) {
       return false;
-    } else if(nodeID[i] < _max[i]) {
+    } else if(nodeID[i] < max[i]) {
       break;
     }
   }
@@ -176,14 +183,30 @@ bool DHTBucket::splitAllowed() const
 SharedHandle<DHTBucket> DHTBucket::split()
 {
   assert(splitAllowed());
-  size_t newPrefixLength = _prefixLength+1;
 
   unsigned char rMax[DHT_ID_LENGTH];
   memcpy(rMax, _max, DHT_ID_LENGTH);
   DHTUtil::flipBit(rMax, DHT_ID_LENGTH, _prefixLength);
+  unsigned char rMin[DHT_ID_LENGTH];
+  memcpy(rMin, _min, DHT_ID_LENGTH);
+
+  DHTUtil::flipBit(_min, DHT_ID_LENGTH, _prefixLength);
 
-  SharedHandle<DHTBucket> rBucket(new DHTBucket(newPrefixLength,
-						rMax, _min, _localNode));
+  size_t rPrefixLength;
+  size_t lPrefixLength;
+  if(isInRange(_localNode->getID(), rMax, rMin)) {
+    rPrefixLength = _prefixLength+1;
+    lPrefixLength = _prefixLength;
+  } else if(isInRange(_localNode->getID(), _max, _min)) {
+    rPrefixLength = _prefixLength;
+    lPrefixLength = _prefixLength+1;
+  } else {
+    rPrefixLength = _prefixLength;
+    lPrefixLength = _prefixLength;
+  }
+
+  SharedHandle<DHTBucket> rBucket(new DHTBucket(rPrefixLength,
+						rMax, rMin, _localNode));
   std::deque<SharedHandle<DHTNode> > tempNodes = _nodes;
   for(std::deque<SharedHandle<DHTNode> >::iterator i = tempNodes.begin();
       i != tempNodes.end();) {
@@ -194,8 +217,7 @@ SharedHandle<DHTBucket> DHTBucket::split()
       ++i;
     }
   }
-  DHTUtil::flipBit(_min, DHT_ID_LENGTH, _prefixLength);
-  _prefixLength = newPrefixLength;
+  _prefixLength = lPrefixLength;
   _nodes = tempNodes;
   // TODO create toString() and use it.
   _logger->debug("New bucket. Range:%s-%s",

src/DHTBucket.h

@@ -68,6 +68,9 @@ private:
   Time _lastUpdated;
 
   Logger* _logger;
+
+  bool isInRange(const unsigned char* nodeID,
+		 const unsigned char* max, const unsigned char* min) const;
 public:
   DHTBucket(const SharedHandle<DHTNode>& localNode);
 

test/DHTBucketTest.cc

@@ -193,10 +193,68 @@ void DHTBucketTest::testSplitAllowed()
 
 void DHTBucketTest::testSplit()
 {
-  unsigned char localNodeID[DHT_ID_LENGTH];
-  memset(localNodeID, 0, DHT_ID_LENGTH);
-  SharedHandle<DHTNode> localNode(new DHTNode(localNodeID));
   {
+    unsigned char localNodeID[DHT_ID_LENGTH];
+    memset(localNodeID, 0, DHT_ID_LENGTH);
+    SharedHandle<DHTNode> localNode(new DHTNode(localNodeID));
+    {
+      DHTBucket bucket(localNode);
+      SharedHandle<DHTBucket> r = bucket.split();
+      {
+	unsigned char expectedRMax[] = { 0x7f, 0xff, 0xff, 0xff, 0xff,
+					 0xff, 0xff, 0xff, 0xff, 0xff,
+					 0xff, 0xff, 0xff, 0xff, 0xff,
+					 0xff, 0xff, 0xff, 0xff, 0xff };
+	unsigned char expectedRMin[DHT_ID_LENGTH];
+	memset(expectedRMin, 0, DHT_ID_LENGTH);
+	CPPUNIT_ASSERT_EQUAL(Util::toHex(expectedRMax, DHT_ID_LENGTH),
+			     Util::toHex(r->getMaxID(), DHT_ID_LENGTH));
+	CPPUNIT_ASSERT_EQUAL(Util::toHex(expectedRMin, DHT_ID_LENGTH),
+			     Util::toHex(r->getMinID(), DHT_ID_LENGTH));
+	CPPUNIT_ASSERT_EQUAL((size_t)1, r->getPrefixLength());
+      }
+      {
+	unsigned char expectedLMax[DHT_ID_LENGTH];
+	memset(expectedLMax, 0xff, DHT_ID_LENGTH);
+	unsigned char expectedLMin[] = { 0x80, 0x00, 0x00, 0x00, 0x00,
+					 0x00, 0x00, 0x00, 0x00, 0x00,
+					 0x00, 0x00, 0x00, 0x00, 0x00,
+					 0x00, 0x00, 0x00, 0x00, 0x00 };
+	CPPUNIT_ASSERT_EQUAL(Util::toHex(expectedLMax, DHT_ID_LENGTH),
+			     Util::toHex(bucket.getMaxID(), DHT_ID_LENGTH));
+	CPPUNIT_ASSERT_EQUAL(Util::toHex(expectedLMin, DHT_ID_LENGTH),
+			     Util::toHex(bucket.getMinID(), DHT_ID_LENGTH));
+	// prefixLength is not changed because bucket cannot include localNode
+	CPPUNIT_ASSERT_EQUAL((size_t)0, bucket.getPrefixLength());
+      }
+    }
+    {
+      SharedHandle<DHTBucket> bucket(new DHTBucket(localNode));
+      for(int i = 0; i < 159; ++i) {
+	CPPUNIT_ASSERT(bucket->splitAllowed());
+	SharedHandle<DHTBucket> t = bucket;
+	bucket = bucket->split();
+	CPPUNIT_ASSERT(!t->splitAllowed());
+      }
+      CPPUNIT_ASSERT(!bucket->splitAllowed());
+      unsigned char expectedMax[] = { 0x00, 0x00, 0x00, 0x00, 0x00,
+				      0x00, 0x00, 0x00, 0x00, 0x00,
+				      0x00, 0x00, 0x00, 0x00, 0x00,
+				      0x00, 0x00, 0x00, 0x00, 0x01 };
+      unsigned char expectedMin[DHT_ID_LENGTH];
+      memset(expectedMin, 0, DHT_ID_LENGTH);
+      CPPUNIT_ASSERT_EQUAL(Util::toHex(expectedMax, DHT_ID_LENGTH),
+			   Util::toHex(bucket->getMaxID(), DHT_ID_LENGTH));
+      CPPUNIT_ASSERT_EQUAL(Util::toHex(expectedMin, DHT_ID_LENGTH),
+			   Util::toHex(bucket->getMinID(), DHT_ID_LENGTH));
+      CPPUNIT_ASSERT_EQUAL((size_t)159, bucket->getPrefixLength());
+    }
+  }
+  {
+    unsigned char localNodeID[DHT_ID_LENGTH];
+    memset(localNodeID, 0, DHT_ID_LENGTH);
+    localNodeID[0] = 0x80;
+    SharedHandle<DHTNode> localNode(new DHTNode(localNodeID));
     DHTBucket bucket(localNode);
     SharedHandle<DHTBucket> r = bucket.split();
     {
@@ -210,7 +268,8 @@ void DHTBucketTest::testSplit()
 			   Util::toHex(r->getMaxID(), DHT_ID_LENGTH));
       CPPUNIT_ASSERT_EQUAL(Util::toHex(expectedRMin, DHT_ID_LENGTH),
 			   Util::toHex(r->getMinID(), DHT_ID_LENGTH));
-      CPPUNIT_ASSERT_EQUAL((size_t)1, r->getPrefixLength());
+      // prefixLength is not changed because bucket cannot include localNode
+      CPPUNIT_ASSERT_EQUAL((size_t)0, r->getPrefixLength());
     }
     {
       unsigned char expectedLMax[DHT_ID_LENGTH];
@@ -226,27 +285,7 @@ void DHTBucketTest::testSplit()
       CPPUNIT_ASSERT_EQUAL((size_t)1, bucket.getPrefixLength());
     }
   }
-  {
-    SharedHandle<DHTBucket> bucket(new DHTBucket(localNode));
-    for(int i = 0; i < 159; ++i) {
-      CPPUNIT_ASSERT(bucket->splitAllowed());
-      SharedHandle<DHTBucket> t = bucket;
-      bucket = bucket->split();
-      CPPUNIT_ASSERT(!t->splitAllowed());
-    }
-    CPPUNIT_ASSERT(!bucket->splitAllowed());
-    unsigned char expectedMax[] = { 0x00, 0x00, 0x00, 0x00, 0x00,
-				    0x00, 0x00, 0x00, 0x00, 0x00,
-				    0x00, 0x00, 0x00, 0x00, 0x00,
-				    0x00, 0x00, 0x00, 0x00, 0x01 };
-    unsigned char expectedMin[DHT_ID_LENGTH];
-    memset(expectedMin, 0, DHT_ID_LENGTH);
-    CPPUNIT_ASSERT_EQUAL(Util::toHex(expectedMax, DHT_ID_LENGTH),
-			 Util::toHex(bucket->getMaxID(), DHT_ID_LENGTH));
-    CPPUNIT_ASSERT_EQUAL(Util::toHex(expectedMin, DHT_ID_LENGTH),
-			 Util::toHex(bucket->getMinID(), DHT_ID_LENGTH));
-    CPPUNIT_ASSERT_EQUAL((size_t)159, bucket->getPrefixLength());
-  }
+
 }
 
 static void createID(unsigned char* id, unsigned char firstChar, unsigned char lastChar)