[Scummvm-cvs-logs] SF.net SVN: scummvm:[39135] scummvm/trunk

[fingolfin at users.sourceforge.net]

Revision: 39135
          http://scummvm.svn.sourceforge.net/scummvm/?rev=39135&view=rev
Author:   fingolfin
Date:     2009-03-05 20:37:53 +0000 (Thu, 05 Mar 2009)

Log Message:
-----------
Fix for bug #2664460: Various SeekableReadStream::seek() implementations (as well as our unit tests, ouch) handled SEEK_END incorrectly (using -offset instead of offset), contrary to what the docs said and what fseek does. Hopefully I found and fixed all affected parts, but still watch out for regressions

Modified Paths:
--------------
    scummvm/trunk/backends/platform/ds/arm9/source/ramsave.cpp
    scummvm/trunk/common/stream.cpp
    scummvm/trunk/engines/gob/inter_v1.cpp
    scummvm/trunk/engines/gob/inter_v2.cpp
    scummvm/trunk/engines/parallaction/disk_ns.cpp
    scummvm/trunk/engines/scumm/file.cpp
    scummvm/trunk/test/common/bufferedseekablereadstream.h
    scummvm/trunk/test/common/seekablesubreadstream.h

Added Paths:
-----------
    scummvm/trunk/test/common/memoryreadstream.h

Modified: scummvm/trunk/backends/platform/ds/arm9/source/ramsave.cpp
===================================================================
--- scummvm/trunk/backends/platform/ds/arm9/source/ramsave.cpp	2009-03-05 19:43:12 UTC (rev 39134)
+++ scummvm/trunk/backends/platform/ds/arm9/source/ramsave.cpp	2009-03-05 20:37:53 UTC (rev 39135)
@@ -205,7 +205,7 @@
 			break;
 		}
 		case SEEK_END: {
-			ptr = save.size - pos;
+			ptr = save.size + pos;
 			break;
 		}
 	}

Modified: scummvm/trunk/common/stream.cpp
===================================================================
--- scummvm/trunk/common/stream.cpp	2009-03-05 19:43:12 UTC (rev 39134)
+++ scummvm/trunk/common/stream.cpp	2009-03-05 20:37:53 UTC (rev 39135)
@@ -69,7 +69,7 @@
 	case SEEK_END:
 		// SEEK_END works just like SEEK_SET, only 'reversed',
 		// i.e. from the end.
-		offs = _size - offs;
+		offs = _size + offs;
 		// Fall through
 	case SEEK_SET:
 		_ptr = _ptrOrig + offs;
@@ -204,7 +204,7 @@
 
 	switch(whence) {
 	case SEEK_END:
-		offset = size() - offset;
+		offset = size() + offset;
 		// fallthrough
 	case SEEK_SET:
 		_pos = _begin + offset;

Modified: scummvm/trunk/engines/gob/inter_v1.cpp
===================================================================
--- scummvm/trunk/engines/gob/inter_v1.cpp	2009-03-05 19:43:12 UTC (rev 39134)
+++ scummvm/trunk/engines/gob/inter_v1.cpp	2009-03-05 20:37:53 UTC (rev 39135)
@@ -2222,7 +2222,7 @@
 
 		_vm->_draw->animateCursor(4);
 		if (offset < 0)
-			stream->seek(-offset - 1, SEEK_END);
+			stream->seek(offset + 1, SEEK_END);
 		else
 			stream->seek(offset);
 

Modified: scummvm/trunk/engines/gob/inter_v2.cpp
===================================================================
--- scummvm/trunk/engines/gob/inter_v2.cpp	2009-03-05 19:43:12 UTC (rev 39134)
+++ scummvm/trunk/engines/gob/inter_v2.cpp	2009-03-05 20:37:53 UTC (rev 39135)
@@ -1979,7 +1979,7 @@
 
 	_vm->_draw->animateCursor(4);
 	if (offset < 0)
-		stream->seek(-offset - 1, SEEK_END);
+		stream->seek(offset + 1, SEEK_END);
 	else
 		stream->seek(offset);
 

Modified: scummvm/trunk/engines/parallaction/disk_ns.cpp
===================================================================
--- scummvm/trunk/engines/parallaction/disk_ns.cpp	2009-03-05 19:43:12 UTC (rev 39134)
+++ scummvm/trunk/engines/parallaction/disk_ns.cpp	2009-03-05 20:37:53 UTC (rev 39135)
@@ -661,7 +661,7 @@
 			return;
 		}
 
-		stream.seek(4, SEEK_END);
+		stream.seek(-4, SEEK_END);
 		uint32 decrlen = stream.readUint32BE() >> 8;
 		byte *dest = (byte*)malloc(decrlen);
 

Modified: scummvm/trunk/engines/scumm/file.cpp
===================================================================
--- scummvm/trunk/engines/scumm/file.cpp	2009-03-05 19:43:12 UTC (rev 39134)
+++ scummvm/trunk/engines/scumm/file.cpp	2009-03-05 20:37:53 UTC (rev 39135)
@@ -142,7 +142,7 @@
 		// Constrain the seek to the subfile
 		switch (whence) {
 		case SEEK_END:
-			offs = _subFileStart + _subFileLen - offs;
+			offs = _subFileStart + _subFileLen + offs;
 			break;
 		case SEEK_SET:
 			offs += _subFileStart;

Modified: scummvm/trunk/test/common/bufferedseekablereadstream.h
===================================================================
--- scummvm/trunk/test/common/bufferedseekablereadstream.h	2009-03-05 19:43:12 UTC (rev 39134)
+++ scummvm/trunk/test/common/bufferedseekablereadstream.h	2009-03-05 20:37:53 UTC (rev 39135)
@@ -56,13 +56,13 @@
 		b = ssrs.readByte();
 		TS_ASSERT( ssrs.eos() );
 
-		ssrs.seek(3, SEEK_END);
+		ssrs.seek(-3, SEEK_END);
 		TS_ASSERT( !ssrs.eos() );
 		TS_ASSERT_EQUALS( ssrs.pos(), 7 );
 		b = ssrs.readByte();
 		TS_ASSERT_EQUALS( b, 7 );
 
-		ssrs.seek(8, SEEK_END);
+		ssrs.seek(-8, SEEK_END);
 		TS_ASSERT_EQUALS( ssrs.pos(), 2 );
 		b = ssrs.readByte();
 		TS_ASSERT_EQUALS( b, 2 );

Added: scummvm/trunk/test/common/memoryreadstream.h
===================================================================
--- scummvm/trunk/test/common/memoryreadstream.h	                        (rev 0)
+++ scummvm/trunk/test/common/memoryreadstream.h	2009-03-05 20:37:53 UTC (rev 39135)
@@ -0,0 +1,61 @@
+#include <cxxtest/TestSuite.h>
+
+#include "common/stream.h"
+
+class MemoryReadStreamTestSuite : public CxxTest::TestSuite {
+	public:
+	void test_seek_set(void) {
+		byte contents[] = { 'a', 'b', '\n', '\n', 'c', '\n' };
+		Common::MemoryReadStream ms(contents, sizeof(contents));
+
+		ms.seek(0, SEEK_SET);
+		TS_ASSERT_EQUALS(ms.pos(), 0);
+		TS_ASSERT(!ms.eos());
+
+		ms.seek(1, SEEK_SET);
+		TS_ASSERT_EQUALS(ms.pos(), 1);
+		TS_ASSERT(!ms.eos());
+
+		ms.seek(5, SEEK_SET);
+		TS_ASSERT_EQUALS(ms.pos(), 5);
+		TS_ASSERT(!ms.eos());
+	}
+
+	void test_seek_cur(void) {
+		byte contents[] = { 'a', 'b', '\n', '\n', 'c' };
+		Common::MemoryReadStream ms(contents, sizeof(contents));
+
+		ms.seek(3, SEEK_CUR);
+		TS_ASSERT_EQUALS(ms.pos(), 3);
+		TS_ASSERT(!ms.eos());
+
+		ms.seek(-1, SEEK_CUR);
+		TS_ASSERT_EQUALS(ms.pos(), 2);
+		TS_ASSERT(!ms.eos());
+
+		ms.seek(3, SEEK_CUR);
+		TS_ASSERT_EQUALS(ms.pos(), 5);
+		TS_ASSERT(!ms.eos());
+
+		ms.seek(-1, SEEK_CUR);
+		TS_ASSERT_EQUALS(ms.pos(), 4);
+		TS_ASSERT(!ms.eos());
+	}
+
+	void test_seek_end(void) {
+		byte contents[] = { 'a', 'b', '\n', '\n', 'c' };
+		Common::MemoryReadStream ms(contents, sizeof(contents));
+
+		ms.seek(0, SEEK_END);
+		TS_ASSERT_EQUALS(ms.pos(), 5);
+		TS_ASSERT(!ms.eos());
+
+		ms.seek(-1, SEEK_END);
+		TS_ASSERT_EQUALS(ms.pos(), 4);
+		TS_ASSERT(!ms.eos());
+
+		ms.seek(-5, SEEK_END);
+		TS_ASSERT_EQUALS(ms.pos(), 0);
+		TS_ASSERT(!ms.eos());
+	}
+};


Property changes on: scummvm/trunk/test/common/memoryreadstream.h
___________________________________________________________________
Added: svn:mime-type
   + text/plain
Added: svn:keywords
   + Date Rev Author URL Id
Added: svn:eol-style
   + native

Modified: scummvm/trunk/test/common/seekablesubreadstream.h
===================================================================
--- scummvm/trunk/test/common/seekablesubreadstream.h	2009-03-05 19:43:12 UTC (rev 39134)
+++ scummvm/trunk/test/common/seekablesubreadstream.h	2009-03-05 20:37:53 UTC (rev 39135)
@@ -58,13 +58,13 @@
 		b = ssrs.readByte();
 		TS_ASSERT( ssrs.eos() );
 
-		ssrs.seek(3, SEEK_END);
+		ssrs.seek(-3, SEEK_END);
 		TS_ASSERT( !ssrs.eos() );
 		TS_ASSERT_EQUALS( ssrs.pos(), 5 );
 		b = ssrs.readByte();
 		TS_ASSERT_EQUALS( b, 6 );
 
-		ssrs.seek(8, SEEK_END);
+		ssrs.seek(-8, SEEK_END);
 		TS_ASSERT_EQUALS( ssrs.pos(), 0 );
 		b = ssrs.readByte();
 		TS_ASSERT_EQUALS( b, 1 );


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.