From 5c4910f71e7002c4f5dc5977be27b84d65609c27 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 8 Nov 2008 08:36:40 +0000 Subject: [PATCH] 2008-11-08 Tatsuhiro Tsujikawa Fixed the bug that the DiskWriter of the first FileEntry whose `needsFileAllocation' property is false is not created even if it shares a piece with next FileEntry which `requested' property is true. Fixed the bug that zero-length file is not created if pre file allocation is not done. * src/MultiDiskAdaptor.cc * src/MultiDiskAdaptor.h * test/MultiDiskAdaptorTest.cc --- ChangeLog | 12 +++ src/MultiDiskAdaptor.cc | 27 +++++- src/MultiDiskAdaptor.h | 3 + test/MultiDiskAdaptorTest.cc | 178 +++++++++++++++++++++++++++++++---- 4 files changed, 195 insertions(+), 25 deletions(-) diff --git a/ChangeLog b/ChangeLog index bebd216c..3625cc3a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,15 @@ +2008-11-08 Tatsuhiro Tsujikawa + + Fixed the bug that the DiskWriter of the first FileEntry whose + `needsFileAllocation' property is false is not created + even if it shares a piece with next FileEntry which `requested' property + is true. + Fixed the bug that zero-length file is not created if pre file + allocation is not done. + * src/MultiDiskAdaptor.cc + * src/MultiDiskAdaptor.h + * test/MultiDiskAdaptorTest.cc + 2008-11-05 Tatsuhiro Tsujikawa Added the ability to pool proxy connection. diff --git a/src/MultiDiskAdaptor.cc b/src/MultiDiskAdaptor.cc index ed1055de..5cd8774e 100644 --- a/src/MultiDiskAdaptor.cc +++ b/src/MultiDiskAdaptor.cc @@ -33,6 +33,10 @@ */ /* copyright --> */ #include "MultiDiskAdaptor.h" + +#include +#include + #include "DefaultDiskWriter.h" #include "message.h" #include "Util.h" @@ -44,8 +48,6 @@ #include "StringFormat.h" #include "Logger.h" #include "SimpleRandomizer.h" -#include -#include namespace aria2 { @@ -208,14 +210,18 @@ void MultiDiskAdaptor::resetDiskWriterEntries() ((*itr)->getFileEntry()->getOffset()/pieceLength)*pieceLength; if(itr != diskWriterEntries.begin()) { for(std::deque >::iterator i = - itr-1; i != done; --i) { + itr-1; true; --i) { const SharedHandle& fileEntry = (*i)->getFileEntry(); - if((uint64_t)pieceStartOffset < + if(pieceStartOffset <= fileEntry->getOffset() || + (uint64_t)pieceStartOffset < fileEntry->getOffset()+fileEntry->getLength()) { (*i)->needsFileAllocation(true); } else { break; } + if(i == done) { + break; + } } } @@ -292,7 +298,12 @@ void MultiDiskAdaptor::openFile() _cachedTopDirPath = getTopDirPath(); resetDiskWriterEntries(); mkdir(_cachedTopDirPath); - // TODO we should call openIfNot here? + // Call DiskWriterEntry::openFile to make sure that zero-length files are + // created. + for(DiskWriterEntries::iterator itr = diskWriterEntries.begin(); + itr != diskWriterEntries.end(); ++itr) { + openIfNot(*itr, &DiskWriterEntry::openFile, _cachedTopDirPath); + } } void MultiDiskAdaptor::initAndOpenFile() @@ -514,4 +525,10 @@ size_t MultiDiskAdaptor::utime(const Time& actime, const Time& modtime) return numOK; } +const std::deque >& +MultiDiskAdaptor::getDiskWriterEntries() const +{ + return diskWriterEntries; +} + } // namespace aria2 diff --git a/src/MultiDiskAdaptor.h b/src/MultiDiskAdaptor.h index 6a759bea..51ea1094 100644 --- a/src/MultiDiskAdaptor.h +++ b/src/MultiDiskAdaptor.h @@ -191,6 +191,9 @@ public: void setMaxOpenFiles(size_t maxOpenFiles); virtual size_t utime(const Time& actime, const Time& modtime); + + const std::deque >& + getDiskWriterEntries() const; }; typedef SharedHandle MultiDiskAdaptorHandle; diff --git a/test/MultiDiskAdaptorTest.cc b/test/MultiDiskAdaptorTest.cc index 0b520e62..9c5b4cc8 100644 --- a/test/MultiDiskAdaptorTest.cc +++ b/test/MultiDiskAdaptorTest.cc @@ -1,13 +1,17 @@ #include "MultiDiskAdaptor.h" + +#include +#include +#include + +#include + #include "FileEntry.h" #include "Exception.h" #include "a2io.h" #include "array_fun.h" #include "TestUtil.h" -#include -#include -#include -#include +#include "DiskWriter.h" namespace aria2 { @@ -19,6 +23,7 @@ class MultiDiskAdaptorTest:public CppUnit::TestFixture { CPPUNIT_TEST(testCutTrailingGarbage); CPPUNIT_TEST(testSize); CPPUNIT_TEST(testUtime); + CPPUNIT_TEST(testResetDiskWriterEntries); CPPUNIT_TEST_SUITE_END(); private: SharedHandle adaptor; @@ -35,25 +40,151 @@ public: void testCutTrailingGarbage(); void testSize(); void testUtime(); + void testResetDiskWriterEntries(); }; CPPUNIT_TEST_SUITE_REGISTRATION( MultiDiskAdaptorTest ); std::deque > createEntries() { - SharedHandle entry1(new FileEntry("file1.txt", 15, 0)); - SharedHandle entry2(new FileEntry("file2.txt", 7, 15)); - SharedHandle entry3(new FileEntry("file3.txt", 3, 22)); - unlink("file1.txt"); - unlink("file2.txt"); - unlink("file3.txt"); - std::deque > entries; - entries.push_back(entry1); - entries.push_back(entry2); - entries.push_back(entry3); + SharedHandle array[] = { + SharedHandle(new FileEntry("file1.txt", 0, 0)), + SharedHandle(new FileEntry("file2.txt", 15, 0)), + SharedHandle(new FileEntry("file3.txt", 7, 15)), + SharedHandle(new FileEntry("file4.txt", 0, 22)), + SharedHandle(new FileEntry("file5.txt", 2, 22)), + SharedHandle(new FileEntry("file6.txt", 0, 24)), + }; + std::deque > entries(&array[0], + &array[arrayLength(array)]); + for(std::deque >::const_iterator i = entries.begin(); + i != entries.end(); ++i) { + File((*i)->getPath()).remove(); + } return entries; } +void MultiDiskAdaptorTest::testResetDiskWriterEntries() +{ + { + std::deque > fileEntries = createEntries(); + adaptor->setFileEntries(fileEntries); + // In openFile(), resetDiskWriterEntries() are called. + adaptor->openFile(); + + std::deque > entries = + adaptor->getDiskWriterEntries(); + CPPUNIT_ASSERT(!entries[0]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(!entries[1]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(!entries[2]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(!entries[3]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(!entries[4]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(!entries[5]->getDiskWriter().isNull()); + } + { + std::deque > fileEntries = createEntries(); + fileEntries[0]->setRequested(false); + adaptor->setFileEntries(fileEntries); + // In openFile(), resetDiskWriterEntries() are called. + adaptor->openFile(); + + std::deque > entries = + adaptor->getDiskWriterEntries(); + // Because entries[1] spans entries[0] + CPPUNIT_ASSERT(!entries[0]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(!entries[1]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(!entries[2]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(!entries[3]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(!entries[4]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(!entries[5]->getDiskWriter().isNull()); + } + { + std::deque > fileEntries = createEntries(); + fileEntries[0]->setRequested(false); + fileEntries[1]->setRequested(false); + adaptor->setFileEntries(fileEntries); + // In openFile(), resetDiskWriterEntries() are called. + adaptor->openFile(); + + std::deque > entries = + adaptor->getDiskWriterEntries(); + CPPUNIT_ASSERT(entries[0]->getDiskWriter().isNull()); + // Because entries[2] spans entries[1] + CPPUNIT_ASSERT(!entries[1]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(!entries[2]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(!entries[3]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(!entries[4]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(!entries[5]->getDiskWriter().isNull()); + } + { + std::deque > fileEntries = createEntries(); + fileEntries[3]->setRequested(false); + adaptor->setFileEntries(fileEntries); + // In openFile(), resetDiskWriterEntries() are called. + adaptor->openFile(); + + std::deque > entries = + adaptor->getDiskWriterEntries(); + CPPUNIT_ASSERT(!entries[0]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(!entries[1]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(!entries[2]->getDiskWriter().isNull()); + // Because entries[4] spans entries[3] + CPPUNIT_ASSERT(!entries[3]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(!entries[4]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(!entries[5]->getDiskWriter().isNull()); + } + { + std::deque > fileEntries = createEntries(); + fileEntries[4]->setRequested(false); + adaptor->setFileEntries(fileEntries); + // In openFile(), resetDiskWriterEntries() are called. + adaptor->openFile(); + + std::deque > entries = + adaptor->getDiskWriterEntries(); + CPPUNIT_ASSERT(!entries[0]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(!entries[1]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(!entries[2]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(!entries[3]->getDiskWriter().isNull()); + // Because entries[3] spans entries[4] + CPPUNIT_ASSERT(!entries[4]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(!entries[5]->getDiskWriter().isNull()); + } + { + std::deque > fileEntries = createEntries(); + fileEntries[3]->setRequested(false); + fileEntries[4]->setRequested(false); + adaptor->setFileEntries(fileEntries); + // In openFile(), resetDiskWriterEntries() are called. + adaptor->openFile(); + + std::deque > entries = + adaptor->getDiskWriterEntries(); + CPPUNIT_ASSERT(!entries[0]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(!entries[1]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(!entries[2]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(entries[3]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(entries[4]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(!entries[5]->getDiskWriter().isNull()); + } + { + std::deque > fileEntries = createEntries(); + fileEntries[5]->setRequested(false); + adaptor->setFileEntries(fileEntries); + // In openFile(), resetDiskWriterEntries() are called. + adaptor->openFile(); + + std::deque > entries = + adaptor->getDiskWriterEntries(); + CPPUNIT_ASSERT(!entries[0]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(!entries[1]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(!entries[2]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(!entries[3]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(!entries[4]->getDiskWriter().isNull()); + CPPUNIT_ASSERT(entries[5]->getDiskWriter().isNull()); + } +} + void readFile(const std::string& filename, char* buf, int bufLength) { FILE* f = fopen(filename.c_str(), "r"); if(f == NULL) { @@ -75,8 +206,9 @@ void MultiDiskAdaptorTest::testWriteData() { adaptor->writeData((const unsigned char*)msg.c_str(), msg.size(), 0); adaptor->closeFile(); + CPPUNIT_ASSERT(File("file1.txt").isFile()); char buf[128]; - readFile("file1.txt", buf, 5); + readFile("file2.txt", buf, 5); buf[5] = '\0'; CPPUNIT_ASSERT_EQUAL(msg, std::string(buf)); @@ -85,10 +217,10 @@ void MultiDiskAdaptorTest::testWriteData() { adaptor->writeData((const unsigned char*)msg2.c_str(), msg2.size(), 5); adaptor->closeFile(); - readFile("file1.txt", buf, 15); + readFile("file2.txt", buf, 15); buf[15] = '\0'; CPPUNIT_ASSERT_EQUAL(std::string("1234567890ABCDE"), std::string(buf)); - readFile("file2.txt", buf, 1); + readFile("file3.txt", buf, 1); buf[1] = '\0'; CPPUNIT_ASSERT_EQUAL(std::string("F"), std::string(buf)); @@ -97,15 +229,21 @@ void MultiDiskAdaptorTest::testWriteData() { adaptor->writeData((const unsigned char*)msg3.c_str(), msg3.size(), 10); adaptor->closeFile(); - readFile("file1.txt", buf, 15); + readFile("file2.txt", buf, 15); buf[15] = '\0'; CPPUNIT_ASSERT_EQUAL(std::string("123456789012345"), std::string(buf)); - readFile("file2.txt", buf, 7); + readFile("file3.txt", buf, 7); buf[7] = '\0'; CPPUNIT_ASSERT_EQUAL(std::string("1234567"), std::string(buf)); - readFile("file3.txt", buf, 2); + + CPPUNIT_ASSERT(File("file4.txt").isFile()); + + readFile("file5.txt", buf, 2); buf[2] = '\0'; CPPUNIT_ASSERT_EQUAL(std::string("12"), std::string(buf)); + + CPPUNIT_ASSERT(File("file6.txt").isFile()); + } catch(Exception& e) { CPPUNIT_FAIL(e.stackTrace()); }