From 05b4370c21b3abf7a1ff6aa83194cf95ab73579c Mon Sep 17 00:00:00 2001 From: Max Horn Date: Thu, 5 Mar 2009 20:37:53 +0000 Subject: 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 svn-id: r39135 --- backends/platform/ds/arm9/source/ramsave.cpp | 2 +- common/stream.cpp | 4 +- engines/gob/inter_v1.cpp | 2 +- engines/gob/inter_v2.cpp | 2 +- engines/parallaction/disk_ns.cpp | 2 +- engines/scumm/file.cpp | 2 +- test/common/bufferedseekablereadstream.h | 4 +- test/common/memoryreadstream.h | 61 ++++++++++++++++++++++++++++ test/common/seekablesubreadstream.h | 4 +- 9 files changed, 72 insertions(+), 11 deletions(-) create mode 100644 test/common/memoryreadstream.h diff --git a/backends/platform/ds/arm9/source/ramsave.cpp b/backends/platform/ds/arm9/source/ramsave.cpp index 113dd5df72..82830e9102 100644 --- a/backends/platform/ds/arm9/source/ramsave.cpp +++ b/backends/platform/ds/arm9/source/ramsave.cpp @@ -205,7 +205,7 @@ bool DSSaveFile::seek(int32 pos, int whence) { break; } case SEEK_END: { - ptr = save.size - pos; + ptr = save.size + pos; break; } } diff --git a/common/stream.cpp b/common/stream.cpp index 1cdcd50723..8614939e54 100644 --- a/common/stream.cpp +++ b/common/stream.cpp @@ -69,7 +69,7 @@ bool MemoryReadStream::seek(int32 offs, int whence) { 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 @@ bool SeekableSubReadStream::seek(int32 offset, int whence) { switch(whence) { case SEEK_END: - offset = size() - offset; + offset = size() + offset; // fallthrough case SEEK_SET: _pos = _begin + offset; diff --git a/engines/gob/inter_v1.cpp b/engines/gob/inter_v1.cpp index b6b82662bc..9c4679ca35 100644 --- a/engines/gob/inter_v1.cpp +++ b/engines/gob/inter_v1.cpp @@ -2222,7 +2222,7 @@ bool Inter_v1::o1_readData(OpFuncParams ¶ms) { _vm->_draw->animateCursor(4); if (offset < 0) - stream->seek(-offset - 1, SEEK_END); + stream->seek(offset + 1, SEEK_END); else stream->seek(offset); diff --git a/engines/gob/inter_v2.cpp b/engines/gob/inter_v2.cpp index 48ce40a335..8859f68ff6 100644 --- a/engines/gob/inter_v2.cpp +++ b/engines/gob/inter_v2.cpp @@ -1979,7 +1979,7 @@ bool Inter_v2::o2_readData(OpFuncParams ¶ms) { _vm->_draw->animateCursor(4); if (offset < 0) - stream->seek(-offset - 1, SEEK_END); + stream->seek(offset + 1, SEEK_END); else stream->seek(offset); diff --git a/engines/parallaction/disk_ns.cpp b/engines/parallaction/disk_ns.cpp index 94d9f8482b..6d78dc1a57 100644 --- a/engines/parallaction/disk_ns.cpp +++ b/engines/parallaction/disk_ns.cpp @@ -661,7 +661,7 @@ public: return; } - stream.seek(4, SEEK_END); + stream.seek(-4, SEEK_END); uint32 decrlen = stream.readUint32BE() >> 8; byte *dest = (byte*)malloc(decrlen); diff --git a/engines/scumm/file.cpp b/engines/scumm/file.cpp index 38c7a1a180..3db2801c01 100644 --- a/engines/scumm/file.cpp +++ b/engines/scumm/file.cpp @@ -142,7 +142,7 @@ bool ScummFile::seek(int32 offs, int whence) { // 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; diff --git a/test/common/bufferedseekablereadstream.h b/test/common/bufferedseekablereadstream.h index f039acd2a8..b91a27d221 100644 --- a/test/common/bufferedseekablereadstream.h +++ b/test/common/bufferedseekablereadstream.h @@ -56,13 +56,13 @@ class BufferedSeekableReadStreamTestSuite : public CxxTest::TestSuite { 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 ); diff --git a/test/common/memoryreadstream.h b/test/common/memoryreadstream.h new file mode 100644 index 0000000000..2cec073ad5 --- /dev/null +++ b/test/common/memoryreadstream.h @@ -0,0 +1,61 @@ +#include + +#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()); + } +}; diff --git a/test/common/seekablesubreadstream.h b/test/common/seekablesubreadstream.h index 68febc7fd6..354e2dd5c1 100644 --- a/test/common/seekablesubreadstream.h +++ b/test/common/seekablesubreadstream.h @@ -58,13 +58,13 @@ class SeekableSubReadStreamTestSuite : public CxxTest::TestSuite { 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 ); -- cgit v1.2.3