2008-01-07 Tatsuhiro Tsujikawa <tujikawa at rednoah dot com>

Fixed the bug that always first found Segment is removed from
	usedSegmentEntries. Removed unused functions.
	* src/SegmentMan.{h, cc}
	* test/SegmentManTest.cc

	Fixed the bug that ServerHost is not removed.
	* src/RequestGroup.cc

ChangeLog

@@ -1,3 +1,13 @@
+2008-01-07  Tatsuhiro Tsujikawa  <tujikawa at rednoah dot com>
+
+	Fixed the bug that always first found Segment is removed from
+	usedSegmentEntries. Removed unused functions.
+	* src/SegmentMan.{h, cc}
+	* test/SegmentManTest.cc
+
+	Fixed the bug that ServerHost is not removed.
+	* src/RequestGroup.cc
+	
 2008-01-07  Tatsuhiro Tsujikawa  <tujikawa at rednoah dot com>
 
 	Fixed the bug that SegmentMan::completeSegment() is not called

src/RequestGroup.cc

@@ -828,7 +828,7 @@ ServerHostHandle RequestGroup::searchServerHost(const string& hostname) const
 
 void RequestGroup::removeServerHost(int32_t cuid)
 {
-  remove_if(_serverHosts.begin(), _serverHosts.end(), FindServerHostByCUID(cuid));
+  _serverHosts.erase(remove_if(_serverHosts.begin(), _serverHosts.end(), FindServerHostByCUID(cuid)), _serverHosts.end());
 }
   
 void RequestGroup::removeURIWhoseHostnameIs(const string& hostname)

src/SegmentMan.cc

@@ -204,10 +204,24 @@ void SegmentMan::cancelSegment(int32_t cuid) {
   }
 }
 
+class FindSegmentEntry {
+private:
+  SegmentHandle _segment;
+public:
+  FindSegmentEntry(const SegmentHandle& segment):_segment(segment) {}
+
+  bool operator()(const SegmentEntryHandle& segmentEntry) const
+  {
+    return segmentEntry->segment->getIndex() == _segment->getIndex();
+  }
+};
+
 bool SegmentMan::completeSegment(int32_t cuid, const SegmentHandle& segment) {
   _pieceStorage->completePiece(segment->getPiece());
   _pieceStorage->advertisePiece(cuid, segment->getPiece()->getIndex());
-  SegmentEntries::iterator itr = getSegmentEntryIteratorByCuid(cuid);
+  SegmentEntries::iterator itr = find_if(usedSegmentEntries.begin(),
+					 usedSegmentEntries.end(),
+					 FindSegmentEntry(segment));
   if(itr == usedSegmentEntries.end()) {
     return false;
   } else {
@@ -258,42 +272,6 @@ int32_t SegmentMan::calculateDownloadSpeed() const {
   return speed;
 }
 
-SegmentEntryHandle SegmentMan::getSegmentEntryByIndex(int32_t index)
-{
-  for(SegmentEntries::const_iterator itr = usedSegmentEntries.begin();
-      itr != usedSegmentEntries.end(); ++itr) {
-    const SegmentEntryHandle& segmentEntry = *itr;
-    if(segmentEntry->segment->getIndex() == index) {
-      return segmentEntry;
-    }
-  }
-  return 0;
-}
-  
-SegmentEntryHandle SegmentMan::getSegmentEntryByCuid(int32_t cuid)
-{
-  for(SegmentEntries::const_iterator itr = usedSegmentEntries.begin();
-      itr != usedSegmentEntries.end(); ++itr) {
-    const SegmentEntryHandle& segmentEntry = *itr;
-    if(segmentEntry->cuid == cuid) {
-      return segmentEntry;
-    }
-  }
-  return 0;    
-}
-
-SegmentEntries::iterator SegmentMan::getSegmentEntryIteratorByCuid(int32_t cuid)
-{
-  for(SegmentEntries::iterator itr = usedSegmentEntries.begin();
-      itr != usedSegmentEntries.end(); ++itr) {
-    const SegmentEntryHandle& segmentEntry = *itr;
-    if(segmentEntry->cuid == cuid) {
-      return itr;
-    }
-  }
-  return usedSegmentEntries.end();    
-}
-
 int32_t SegmentMan::countFreePieceFrom(int32_t index) const
 {
   for(int32_t i = index; i < _downloadContext->getNumPieces(); ++i) {

src/SegmentMan.h

@@ -87,13 +87,6 @@ private:
   SegmentHandle checkoutSegment(int32_t cuid, const PieceHandle& piece);
 
   SegmentEntryHandle findSlowerSegmentEntry(const PeerStatHandle& peerStat) const;
-
-  SegmentEntryHandle getSegmentEntryByIndex(int32_t index);
-  
-  SegmentEntryHandle getSegmentEntryByCuid(int32_t cuid);
-
-  SegmentEntries::iterator getSegmentEntryIteratorByCuid(int32_t cuid);
-
 public:
   SegmentMan(const Option* option,
 	     const DownloadContextHandle& downloadContext = 0,

test/SegmentManTest.cc

@@ -4,8 +4,10 @@
 #include "Util.h"
 #include "SingleFileDownloadContext.h"
 #include "UnknownLengthPieceStorage.h"
+#include "DefaultPieceStorage.h"
 #include "Segment.h"
 #include "Option.h"
+#include "MockBtContext.h"
 #include <cppunit/extensions/HelperMacros.h>
 
 using namespace std;
@@ -14,6 +16,7 @@ class SegmentManTest:public CppUnit::TestFixture {
 
   CPPUNIT_TEST_SUITE(SegmentManTest);
   CPPUNIT_TEST(testNullBitfield);
+  CPPUNIT_TEST(testCompleteSegment);
   CPPUNIT_TEST(testMarkPieceDone_usedSegment);
   CPPUNIT_TEST_SUITE_END();
 private:
@@ -23,6 +26,7 @@ public:
   }
 
   void testNullBitfield();
+  void testCompleteSegment();
   void testMarkPieceDone_usedSegment();
 };
 
@@ -50,6 +54,33 @@ void SegmentManTest::testNullBitfield()
   CPPUNIT_ASSERT(!segmentMan.getSegment(2).isNull());
 }
 
+void SegmentManTest::testCompleteSegment()
+{
+  Option op;
+  int32_t pieceLength = 1024*1024;
+  int64_t totalLength = 64*1024*1024;
+  MockBtContextHandle dctx = new MockBtContext();
+  dctx->setPieceLength(pieceLength);
+  dctx->setTotalLength(totalLength);
+  dctx->setNumPieces((totalLength+pieceLength-1)/pieceLength);
+  DefaultPieceStorageHandle ps = new DefaultPieceStorage(dctx, &op);
+
+  SegmentMan segmentMan(&op, dctx, ps);
+
+  CPPUNIT_ASSERT(!segmentMan.getSegment(1, 0).isNull());
+  SegmentHandle seg = segmentMan.getSegment(1, 1);
+  CPPUNIT_ASSERT(!seg.isNull());
+  CPPUNIT_ASSERT(!segmentMan.getSegment(1, 2).isNull());
+
+  seg->updateWrittenLength(pieceLength);
+  segmentMan.completeSegment(1, seg);
+  
+  Segments segments = segmentMan.getInFlightSegment(1);
+  CPPUNIT_ASSERT_EQUAL((size_t)2, segments.size());
+  CPPUNIT_ASSERT_EQUAL(0, segments[0]->getIndex());
+  CPPUNIT_ASSERT_EQUAL(2, segments[1]->getIndex());
+}
+
 void SegmentManTest::testMarkPieceDone_usedSegment()
 {
   // TODO implement this later