From da84d3cf8304445a4344f38bc9fba7db6a092884 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 6 Jan 2008 16:37:25 +0000 Subject: [PATCH] 2008-01-07 Tatsuhiro Tsujikawa 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 | 10 ++++++++ src/RequestGroup.cc | 2 +- src/SegmentMan.cc | 52 ++++++++++++------------------------------ src/SegmentMan.h | 7 ------ test/SegmentManTest.cc | 31 +++++++++++++++++++++++++ 5 files changed, 57 insertions(+), 45 deletions(-) diff --git a/ChangeLog b/ChangeLog index 0a7fd4e7..e10798d0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +2008-01-07 Tatsuhiro Tsujikawa + + 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 Fixed the bug that SegmentMan::completeSegment() is not called diff --git a/src/RequestGroup.cc b/src/RequestGroup.cc index 4a3ac9cd..4dedd96e 100644 --- a/src/RequestGroup.cc +++ b/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) diff --git a/src/SegmentMan.cc b/src/SegmentMan.cc index 35221587..763ae924 100644 --- a/src/SegmentMan.cc +++ b/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) { diff --git a/src/SegmentMan.h b/src/SegmentMan.h index 14270a86..b06e6ea0 100644 --- a/src/SegmentMan.h +++ b/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, diff --git a/test/SegmentManTest.cc b/test/SegmentManTest.cc index e9149ee0..3f8082ba 100644 --- a/test/SegmentManTest.cc +++ b/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 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