Use std::unique_ptr for PieceSelector instead of std::shared_ptr

src/DefaultPieceStorage.cc

@@ -83,7 +83,7 @@ DefaultPieceStorage::DefaultPieceStorage
    endGamePieceNum_(END_GAME_PIECE_NUM),
    option_(option),
    pieceStatMan_(new PieceStatMan(downloadContext->getNumPieces(), true)),
-   pieceSelector_(new RarestPieceSelector(pieceStatMan_)),
+   pieceSelector_(make_unique<RarestPieceSelector>(pieceStatMan_)),
    wrDiskCache_(0)
 {
   const std::string& pieceSelectorOpt =
@@ -866,4 +866,15 @@ void DefaultPieceStorage::onDownloadIncomplete()
   streamPieceSelector_->onBitfieldInit();
 }
 
+void DefaultPieceStorage::setPieceSelector
+(std::unique_ptr<PieceSelector> pieceSelector)
+{
+  pieceSelector_ = std::move(pieceSelector);
+}
+
+std::unique_ptr<PieceSelector> DefaultPieceStorage::popPieceSelector()
+{
+  return std::move(pieceSelector_);
+}
+
 } // namespace aria2

src/DefaultPieceStorage.h

@@ -90,7 +90,7 @@ private:
 
   std::shared_ptr<PieceStatMan> pieceStatMan_;
 
-  std::shared_ptr<PieceSelector> pieceSelector_;
+  std::unique_ptr<PieceSelector> pieceSelector_;
   std::shared_ptr<StreamPieceSelector> streamPieceSelector_;
 
   WrDiskCache* wrDiskCache_;
@@ -301,16 +301,15 @@ public:
     return pieceStatMan_;
   }
 
-  void setPieceSelector(const std::shared_ptr<PieceSelector>& pieceSelector)
-  {
-    pieceSelector_ = pieceSelector;
-  }
+  void setPieceSelector(std::unique_ptr<PieceSelector> pieceSelector);
 
-  const std::shared_ptr<PieceSelector>& getPieceSelector() const
+  const std::unique_ptr<PieceSelector>& getPieceSelector() const
   {
     return pieceSelector_;
   }
 
+  std::unique_ptr<PieceSelector> popPieceSelector();
+
   void setWrDiskCache(WrDiskCache* wrDiskCache)
   {
     wrDiskCache_ = wrDiskCache;

src/RequestGroup.cc

@@ -596,9 +596,7 @@ void RequestGroup::initPieceStorage()
         // integrated downloads. Currently multi-file integrated
         // download is not supported.
         A2_LOG_DEBUG("Using LongestSequencePieceSelector");
-        std::shared_ptr<PieceSelector> longestPieceSelector
-          (new LongestSequencePieceSelector());
-        ps->setPieceSelector(longestPieceSelector);
+        ps->setPieceSelector(make_unique<LongestSequencePieceSelector>());
       }
       if(option_->defined(PREF_BT_PRIORITIZE_PIECE)) {
         std::vector<size_t> result;
@@ -607,12 +605,12 @@ void RequestGroup::initPieceStorage()
            downloadContext_->getFileEntries(),
            downloadContext_->getPieceLength());
         if(!result.empty()) {
-          std::random_shuffle(result.begin(), result.end(),
+          std::random_shuffle(std::begin(result), std::end(result),
                               *SimpleRandomizer::getInstance());
-          std::shared_ptr<PriorityPieceSelector> priSelector
-            (new PriorityPieceSelector(ps->getPieceSelector()));
-          priSelector->setPriorityPiece(result.begin(), result.end());
-          ps->setPieceSelector(priSelector);
+          auto priSelector = make_unique<PriorityPieceSelector>
+            (ps->popPieceSelector());
+          priSelector->setPriorityPiece(std::begin(result), std::end(result));
+          ps->setPieceSelector(std::move(priSelector));
         }
       }
     }

test/DefaultPieceStorageTest.cc

@@ -44,17 +44,17 @@ private:
   std::shared_ptr<DownloadContext> dctx_;
   std::shared_ptr<Peer> peer;
   std::shared_ptr<Option> option_;
-  std::shared_ptr<PieceSelector> pieceSelector_;
+  std::unique_ptr<PieceSelector> pieceSelector_;
 public:
   void setUp() {
-    option_.reset(new Option());
+    option_ = std::make_shared<Option>();
     option_->put(PREF_DIR, ".");
-    dctx_.reset(new DownloadContext());
+    dctx_ = std::make_shared<DownloadContext>();
     bittorrent::load(A2_TEST_DIR"/test.torrent", dctx_, option_);
-    peer.reset(new Peer("192.168.0.1", 6889));
+    peer = std::make_shared<Peer>("192.168.0.1", 6889);
     peer->allocateSessionResource(dctx_->getPieceLength(),
                                   dctx_->getTotalLength());
-    pieceSelector_.reset(new InorderPieceSelector());
+    pieceSelector_ = make_unique<InorderPieceSelector>();
   }
 
   void testGetTotalLength();
@@ -87,10 +87,10 @@ void DefaultPieceStorageTest::testGetTotalLength() {
 
 void DefaultPieceStorageTest::testGetMissingPiece() {
   DefaultPieceStorage pss(dctx_, option_.get());
-  pss.setPieceSelector(pieceSelector_);
+  pss.setPieceSelector(std::move(pieceSelector_));
   peer->setAllBitfield();
 
-  std::shared_ptr<Piece> piece = pss.getMissingPiece(peer, 1);
+  auto piece = pss.getMissingPiece(peer, 1);
   CPPUNIT_ASSERT_EQUAL(std::string("piece: index=0, length=128"),
                        piece->toString());
   CPPUNIT_ASSERT(piece->usedBy(1));
@@ -106,9 +106,9 @@ void DefaultPieceStorageTest::testGetMissingPiece() {
 
 void DefaultPieceStorageTest::testGetMissingPiece_many() {
   DefaultPieceStorage pss(dctx_, option_.get());
-  pss.setPieceSelector(pieceSelector_);
+  pss.setPieceSelector(std::move(pieceSelector_));
   peer->setAllBitfield();
-  std::vector<std::shared_ptr<Piece> > pieces;
+  std::vector<std::shared_ptr<Piece>> pieces;
   pss.getMissingPiece(pieces, 2, peer, 1);
   CPPUNIT_ASSERT_EQUAL((size_t)2, pieces.size());
   CPPUNIT_ASSERT_EQUAL(std::string("piece: index=0, length=128"),
@@ -126,7 +126,7 @@ void DefaultPieceStorageTest::testGetMissingPiece_many() {
 void DefaultPieceStorageTest::testGetMissingPiece_excludedIndexes()
 {
   DefaultPieceStorage pss(dctx_, option_.get());
-  pss.setPieceSelector(pieceSelector_);
+  pss.setPieceSelector(std::move(pieceSelector_));
   pss.setEndGamePieceNum(0);
 
   peer->setAllBitfield();
@@ -134,7 +134,7 @@ void DefaultPieceStorageTest::testGetMissingPiece_excludedIndexes()
   std::vector<size_t> excludedIndexes;
   excludedIndexes.push_back(1);
 
-  std::shared_ptr<Piece> piece = pss.getMissingPiece(peer, excludedIndexes, 1);
+  auto piece = pss.getMissingPiece(peer, excludedIndexes, 1);
   CPPUNIT_ASSERT_EQUAL(std::string("piece: index=0, length=128"),
                        piece->toString());
 
@@ -148,11 +148,11 @@ void DefaultPieceStorageTest::testGetMissingPiece_excludedIndexes()
 
 void DefaultPieceStorageTest::testGetMissingPiece_manyWithExcludedIndexes() {
   DefaultPieceStorage pss(dctx_, option_.get());
-  pss.setPieceSelector(pieceSelector_);
+  pss.setPieceSelector(std::move(pieceSelector_));
   peer->setAllBitfield();
   std::vector<size_t> excludedIndexes;
   excludedIndexes.push_back(1);
-  std::vector<std::shared_ptr<Piece> > pieces;
+  std::vector<std::shared_ptr<Piece>> pieces;
   pss.getMissingPiece(pieces, 2, peer, excludedIndexes, 1);
   CPPUNIT_ASSERT_EQUAL((size_t)2, pieces.size());
   CPPUNIT_ASSERT_EQUAL(std::string("piece: index=0, length=128"),
@@ -166,14 +166,14 @@ void DefaultPieceStorageTest::testGetMissingPiece_manyWithExcludedIndexes() {
 
 void DefaultPieceStorageTest::testGetMissingFastPiece() {
   DefaultPieceStorage pss(dctx_, option_.get());
-  pss.setPieceSelector(pieceSelector_);
+  pss.setPieceSelector(std::move(pieceSelector_));
   pss.setEndGamePieceNum(0);
 
   peer->setAllBitfield();
   peer->setFastExtensionEnabled(true);
   peer->addPeerAllowedIndex(2);
 
-  std::shared_ptr<Piece> piece = pss.getMissingFastPiece(peer, 1);
+  auto piece = pss.getMissingFastPiece(peer, 1);
   CPPUNIT_ASSERT_EQUAL(std::string("piece: index=2, length=128"),
                        piece->toString());
 
@@ -183,7 +183,7 @@ void DefaultPieceStorageTest::testGetMissingFastPiece() {
 void DefaultPieceStorageTest::testGetMissingFastPiece_excludedIndexes()
 {
   DefaultPieceStorage pss(dctx_, option_.get());
-  pss.setPieceSelector(pieceSelector_);
+  pss.setPieceSelector(std::move(pieceSelector_));
   pss.setEndGamePieceNum(0);
 
   peer->setAllBitfield();
@@ -194,7 +194,7 @@ void DefaultPieceStorageTest::testGetMissingFastPiece_excludedIndexes()
   std::vector<size_t> excludedIndexes;
   excludedIndexes.push_back(2);
 
-  std::shared_ptr<Piece> piece = pss.getMissingFastPiece(peer, excludedIndexes, 1);
+  auto piece = pss.getMissingFastPiece(peer, excludedIndexes, 1);
   CPPUNIT_ASSERT_EQUAL(std::string("piece: index=1, length=128"),
                        piece->toString());
 
@@ -213,12 +213,12 @@ void DefaultPieceStorageTest::testHasMissingPiece() {
 
 void DefaultPieceStorageTest::testCompletePiece() {
   DefaultPieceStorage pss(dctx_, option_.get());
-  pss.setPieceSelector(pieceSelector_);
+  pss.setPieceSelector(std::move(pieceSelector_));
   pss.setEndGamePieceNum(0);
 
   peer->setAllBitfield();
 
-  std::shared_ptr<Piece> piece = pss.getMissingPiece(peer, 1);
+  auto piece = pss.getMissingPiece(peer, 1);
   CPPUNIT_ASSERT_EQUAL(std::string("piece: index=0, length=128"),
                        piece->toString());
 
@@ -228,7 +228,7 @@ void DefaultPieceStorageTest::testCompletePiece() {
 
   CPPUNIT_ASSERT_EQUAL((int64_t)128LL, pss.getCompletedLength());
 
-  std::shared_ptr<Piece> incompletePiece = pss.getMissingPiece(peer, 1);
+  auto incompletePiece = pss.getMissingPiece(peer, 1);
   incompletePiece->completeBlock(0);
   CPPUNIT_ASSERT_EQUAL((int64_t)256LL, pss.getCompletedLength());
 }
@@ -236,7 +236,7 @@ void DefaultPieceStorageTest::testCompletePiece() {
 void DefaultPieceStorageTest::testGetPiece() {
   DefaultPieceStorage pss(dctx_, option_.get());
 
-  std::shared_ptr<Piece> pieceGot = pss.getPiece(0);
+  auto pieceGot = pss.getPiece(0);
   CPPUNIT_ASSERT_EQUAL((size_t)0, pieceGot->getIndex());
   CPPUNIT_ASSERT_EQUAL(128, pieceGot->getLength());
   CPPUNIT_ASSERT_EQUAL(false, pieceGot->pieceComplete());
@@ -244,10 +244,10 @@ void DefaultPieceStorageTest::testGetPiece() {
 
 void DefaultPieceStorageTest::testGetPieceInUsedPieces() {
   DefaultPieceStorage pss(dctx_, option_.get());
-  std::shared_ptr<Piece> piece = std::shared_ptr<Piece>(new Piece(0, 128));
+  auto piece = std::make_shared<Piece>(0, 128);
   piece->completeBlock(0);
   pss.addUsedPiece(piece);
-  std::shared_ptr<Piece> pieceGot = pss.getPiece(0);
+  auto  pieceGot = pss.getPiece(0);
   CPPUNIT_ASSERT_EQUAL((size_t)0, pieceGot->getIndex());
   CPPUNIT_ASSERT_EQUAL(128, pieceGot->getLength());
   CPPUNIT_ASSERT_EQUAL((size_t)1, pieceGot->countCompleteBlock());
@@ -255,9 +255,9 @@ void DefaultPieceStorageTest::testGetPieceInUsedPieces() {
 
 void DefaultPieceStorageTest::testGetPieceCompletedPiece() {
   DefaultPieceStorage pss(dctx_, option_.get());
-  std::shared_ptr<Piece> piece = std::shared_ptr<Piece>(new Piece(0, 128));
+  auto piece = std::make_shared<Piece>(0, 128);
   pss.completePiece(piece);
-  std::shared_ptr<Piece> pieceGot = pss.getPiece(0);
+  auto pieceGot = pss.getPiece(0);
   CPPUNIT_ASSERT_EQUAL((size_t)0, pieceGot->getIndex());
   CPPUNIT_ASSERT_EQUAL(128, pieceGot->getLength());
   CPPUNIT_ASSERT_EQUAL(true, pieceGot->pieceComplete());
@@ -269,19 +269,20 @@ void DefaultPieceStorageTest::testCancelPiece()
   int64_t totalLength = 32*pieceLength; // <-- make the number of piece greater than END_GAME_PIECE_NUM
   std::deque<std::string> uris1;
   uris1.push_back("http://localhost/src/file1.txt");
-  std::shared_ptr<FileEntry> file1(new FileEntry("src/file1.txt", totalLength, 0 /*, uris1*/));
+  auto file1 = std::make_shared<FileEntry>("src/file1.txt", totalLength,
+                                           0 /*, uris1*/);
 
-  std::shared_ptr<DownloadContext> dctx
-    (new DownloadContext(pieceLength, totalLength, "src/file1.txt"));
+  auto dctx = std::make_shared<DownloadContext>(pieceLength, totalLength,
+                                                "src/file1.txt");
 
-  std::shared_ptr<DefaultPieceStorage> ps(new DefaultPieceStorage(dctx, option_.get()));
+  DefaultPieceStorage ps{dctx, option_.get()};
 
-  std::shared_ptr<Piece> p = ps->getMissingPiece(0, 1);
+  auto p = ps.getMissingPiece(0, 1);
   p->completeBlock(0);
 
-  ps->cancelPiece(p, 1);
+  ps.cancelPiece(p, 1);
 
-  std::shared_ptr<Piece> p2 = ps->getMissingPiece(0, 2);
+  auto p2 = ps.getMissingPiece(0, 2);
 
   CPPUNIT_ASSERT(p2->hasBlock(0));
   CPPUNIT_ASSERT(p2->usedBy(2));
@@ -292,8 +293,7 @@ void DefaultPieceStorageTest::testMarkPiecesDone()
 {
   size_t pieceLength = 256*1024;
   int64_t totalLength = 4*1024*1024;
-  std::shared_ptr<DownloadContext> dctx
-    (new DownloadContext(pieceLength, totalLength));
+  auto dctx =std::make_shared<DownloadContext>(pieceLength, totalLength);
 
   DefaultPieceStorage ps(dctx, option_.get());
 
@@ -305,7 +305,8 @@ void DefaultPieceStorageTest::testMarkPiecesDone()
   for(size_t i = 10; i < (totalLength+pieceLength-1)/pieceLength; ++i) {
     CPPUNIT_ASSERT(!ps.hasPiece(i));
   }
-  CPPUNIT_ASSERT_EQUAL((int64_t)pieceLength*10+16*1024*2, ps.getCompletedLength());
+  CPPUNIT_ASSERT_EQUAL((int64_t)pieceLength*10+16*1024*2,
+                       ps.getCompletedLength());
 
   ps.markPiecesDone(totalLength);
 
@@ -319,8 +320,7 @@ void DefaultPieceStorageTest::testMarkPiecesDone()
 
 void DefaultPieceStorageTest::testGetCompletedLength()
 {
-  std::shared_ptr<DownloadContext> dctx
-    (new DownloadContext(1024*1024, 256*1024*1024));
+  auto dctx = std::make_shared<DownloadContext>(1024*1024, 256*1024*1024);
 
   DefaultPieceStorage ps(dctx, option_.get());
 
@@ -329,9 +329,9 @@ void DefaultPieceStorageTest::testGetCompletedLength()
   ps.markPiecesDone(250*1024*1024);
   CPPUNIT_ASSERT_EQUAL((int64_t)250*1024*1024, ps.getCompletedLength());
 
-  std::vector<std::shared_ptr<Piece> > inFlightPieces;
+  std::vector<std::shared_ptr<Piece>> inFlightPieces;
   for(int i = 0; i < 2; ++i) {
-    std::shared_ptr<Piece> p(new Piece(250+i, 1024*1024));
+    auto p = std::make_shared<Piece>(250+i, 1024*1024);
     for(int j = 0; j < 32; ++j) {
       p->completeBlock(j);
     }
@@ -350,27 +350,27 @@ void DefaultPieceStorageTest::testGetCompletedLength()
 void DefaultPieceStorageTest::testGetFilteredCompletedLength()
 {
   const size_t pieceLength = 1024*1024;
-  std::shared_ptr<DownloadContext> dctx(new DownloadContext());
+  auto dctx = std::make_shared<DownloadContext>();
   dctx->setPieceLength(pieceLength);
-  std::shared_ptr<FileEntry> files[] = {
-    std::shared_ptr<FileEntry>(new FileEntry("foo", 2*pieceLength, 0)),
-    std::shared_ptr<FileEntry>(new FileEntry("bar", 4*pieceLength, 2*pieceLength))
+  auto files = std::vector<std::shared_ptr<FileEntry>>{
+    std::make_shared<FileEntry>("foo", 2*pieceLength, 0),
+    std::make_shared<FileEntry>("bar", 4*pieceLength, 2*pieceLength)
   };
   files[1]->setRequested(false);
-  dctx->setFileEntries(&files[0], &files[2]);
+  dctx->setFileEntries(std::begin(files), std::end(files));
 
   DefaultPieceStorage ps(dctx, option_.get());
-  std::vector<std::shared_ptr<Piece> > inflightPieces(2);
-  inflightPieces[0] = std::shared_ptr<Piece>(new Piece(1, pieceLength));
+  std::vector<std::shared_ptr<Piece>> inflightPieces(2);
+  inflightPieces[0] = std::make_shared<Piece>(1, pieceLength);
   inflightPieces[0]->completeBlock(0);
-  inflightPieces[1] = std::shared_ptr<Piece>(new Piece(2, pieceLength));
+  inflightPieces[1] = std::make_shared<Piece>(2, pieceLength);
   inflightPieces[1]->completeBlock(1);
   inflightPieces[1]->completeBlock(2);
 
   ps.addInFlightPiece(inflightPieces);
   ps.setupFileFilter();
 
-  std::shared_ptr<Piece> piece = ps.getMissingPiece(0, 1);
+  auto piece = ps.getMissingPiece(0, 1);
   ps.completePiece(piece);
 
   CPPUNIT_ASSERT_EQUAL((int64_t)pieceLength+16*1024,
@@ -381,7 +381,7 @@ void DefaultPieceStorageTest::testGetNextUsedIndex()
 {
   DefaultPieceStorage pss(dctx_, option_.get());
   CPPUNIT_ASSERT_EQUAL((size_t)3, pss.getNextUsedIndex(0));
-  std::shared_ptr<Piece> piece = pss.getMissingPiece(2, 1);
+  auto piece = pss.getMissingPiece(2, 1);
   CPPUNIT_ASSERT_EQUAL((size_t)2, pss.getNextUsedIndex(0));
   pss.completePiece(piece);
   CPPUNIT_ASSERT_EQUAL((size_t)2, pss.getNextUsedIndex(0));