2010-06-21 Tatsuhiro Tsujikawa <t-tujikawa@users.sourceforge.net>

	Fixed the bug that corrups file if segment returned from
	SegmetnMan::getCleanSegmentIfOwnerIsIdle() has writtenLength > 0.
	* src/DownloadCommand.cc
	* src/SegmentMan.cc
	* src/SegmentMan.h
	* test/SegmentManTest.cc

ChangeLog

@@ -1,3 +1,12 @@
+2010-06-21  Tatsuhiro Tsujikawa  <t-tujikawa@users.sourceforge.net>
+
+	Fixed the bug that corrups file if segment returned from
+	SegmetnMan::getCleanSegmentIfOwnerIsIdle() has writtenLength > 0.
+	* src/DownloadCommand.cc
+	* src/SegmentMan.cc
+	* src/SegmentMan.h
+	* test/SegmentManTest.cc
+
 2010-06-21  Tatsuhiro Tsujikawa  <t-tujikawa@users.sourceforge.net>
 
 	Changed naming standards for class member variable: now it looks

src/DownloadCommand.cc

@@ -315,10 +315,16 @@ bool DownloadCommand::prepareForNextSegment() {
       if(!tempSegment->complete()) {
         return prepareForRetry(0);
       }
-      SharedHandle<Segment> nextSegment =
-        getSegmentMan()->getCleanSegmentIfOwnerIsIdle
+      SharedHandle<Segment> nextSegment = getSegmentMan()->getSegment
         (getCuid(), tempSegment->getIndex()+1);
       if(nextSegment.isNull()) {
+        nextSegment = getSegmentMan()->getCleanSegmentIfOwnerIsIdle
+        (getCuid(), tempSegment->getIndex()+1);
+      }
+      if(nextSegment.isNull() || nextSegment->getWrittenLength() > 0) {
+        // If nextSegment->getWrittenLength() > 0, current socket must
+        // be closed because writing incoming data at
+        // nextSegment->getWrittenLength() corrupts file.
         return prepareForRetry(0);
       } else {
         getDownloadEngine()->addCommand(this);

src/SegmentMan.cc

@@ -236,7 +236,7 @@ SharedHandle<Segment> SegmentMan::getCleanSegmentIfOwnerIsIdle
       }
     }
   }
-  return checkoutSegment(cuid, pieceStorage_->getMissingPiece(index));
+  return SharedHandle<Segment>();
 }
 
 void SegmentMan::cancelSegment(const SharedHandle<Segment>& segment)

src/SegmentMan.h

@@ -155,12 +155,10 @@ public:
    */
   SharedHandle<Segment> getSegment(cuid_t cuid, size_t index);
 
-  // Returns a segment whose index is index.  If the segment whose
-  // index is index is free, it is assigned to cuid and it is
-  // returned.  If it has already be assigned to another cuid, and if
-  // it is idle state and segment's written length is 0, then cancels
-  // the assignment and re-attach the segment to given cuid and the
-  // segment is returned. Otherwise returns null.
+  // Returns a currently used segment whose index is index and written
+  // length is 0.  The current owner(in idle state) of segment cancels
+  // the segment and cuid command acquires the ownership of the
+  // segment.  If no such segment exists, returns null.
   SharedHandle<Segment> getCleanSegmentIfOwnerIsIdle(cuid_t cuid, size_t index);
 
   /**

test/SegmentManTest.cc

@@ -194,15 +194,6 @@ void SegmentManTest::testGetCleanSegmentIfOwnerIsIdle()
   CPPUNIT_ASSERT(segmentMan_->getCleanSegmentIfOwnerIsIdle(5, 0).isNull());
   // Segment::updateWrittenLength != 0
   CPPUNIT_ASSERT(segmentMan_->getCleanSegmentIfOwnerIsIdle(5, 1).isNull());
-
-  // Test with UnknownLengthPieceStorage
-  SharedHandle<DownloadContext> dctx(new DownloadContext(1024, 0, "aria2"));
-  SharedHandle<UnknownLengthPieceStorage> ps
-    (new UnknownLengthPieceStorage(dctx, option_.get()));
-  segmentMan_.reset(new SegmentMan(option_.get(), dctx, ps));
-  
-  CPPUNIT_ASSERT(!segmentMan_->getCleanSegmentIfOwnerIsIdle(1, 0).isNull());
-
 }
 
 } // namespace aria2