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
This commit is contained in:
parent
2017d1c9ea
commit
05b4370c21
9 changed files with 72 additions and 11 deletions
|
@ -205,7 +205,7 @@ bool DSSaveFile::seek(int32 pos, int whence) {
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
case SEEK_END: {
|
case SEEK_END: {
|
||||||
ptr = save.size - pos;
|
ptr = save.size + pos;
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -69,7 +69,7 @@ bool MemoryReadStream::seek(int32 offs, int whence) {
|
||||||
case SEEK_END:
|
case SEEK_END:
|
||||||
// SEEK_END works just like SEEK_SET, only 'reversed',
|
// SEEK_END works just like SEEK_SET, only 'reversed',
|
||||||
// i.e. from the end.
|
// i.e. from the end.
|
||||||
offs = _size - offs;
|
offs = _size + offs;
|
||||||
// Fall through
|
// Fall through
|
||||||
case SEEK_SET:
|
case SEEK_SET:
|
||||||
_ptr = _ptrOrig + offs;
|
_ptr = _ptrOrig + offs;
|
||||||
|
@ -204,7 +204,7 @@ bool SeekableSubReadStream::seek(int32 offset, int whence) {
|
||||||
|
|
||||||
switch(whence) {
|
switch(whence) {
|
||||||
case SEEK_END:
|
case SEEK_END:
|
||||||
offset = size() - offset;
|
offset = size() + offset;
|
||||||
// fallthrough
|
// fallthrough
|
||||||
case SEEK_SET:
|
case SEEK_SET:
|
||||||
_pos = _begin + offset;
|
_pos = _begin + offset;
|
||||||
|
|
|
@ -2222,7 +2222,7 @@ bool Inter_v1::o1_readData(OpFuncParams ¶ms) {
|
||||||
|
|
||||||
_vm->_draw->animateCursor(4);
|
_vm->_draw->animateCursor(4);
|
||||||
if (offset < 0)
|
if (offset < 0)
|
||||||
stream->seek(-offset - 1, SEEK_END);
|
stream->seek(offset + 1, SEEK_END);
|
||||||
else
|
else
|
||||||
stream->seek(offset);
|
stream->seek(offset);
|
||||||
|
|
||||||
|
|
|
@ -1979,7 +1979,7 @@ bool Inter_v2::o2_readData(OpFuncParams ¶ms) {
|
||||||
|
|
||||||
_vm->_draw->animateCursor(4);
|
_vm->_draw->animateCursor(4);
|
||||||
if (offset < 0)
|
if (offset < 0)
|
||||||
stream->seek(-offset - 1, SEEK_END);
|
stream->seek(offset + 1, SEEK_END);
|
||||||
else
|
else
|
||||||
stream->seek(offset);
|
stream->seek(offset);
|
||||||
|
|
||||||
|
|
|
@ -661,7 +661,7 @@ public:
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
stream.seek(4, SEEK_END);
|
stream.seek(-4, SEEK_END);
|
||||||
uint32 decrlen = stream.readUint32BE() >> 8;
|
uint32 decrlen = stream.readUint32BE() >> 8;
|
||||||
byte *dest = (byte*)malloc(decrlen);
|
byte *dest = (byte*)malloc(decrlen);
|
||||||
|
|
||||||
|
|
|
@ -142,7 +142,7 @@ bool ScummFile::seek(int32 offs, int whence) {
|
||||||
// Constrain the seek to the subfile
|
// Constrain the seek to the subfile
|
||||||
switch (whence) {
|
switch (whence) {
|
||||||
case SEEK_END:
|
case SEEK_END:
|
||||||
offs = _subFileStart + _subFileLen - offs;
|
offs = _subFileStart + _subFileLen + offs;
|
||||||
break;
|
break;
|
||||||
case SEEK_SET:
|
case SEEK_SET:
|
||||||
offs += _subFileStart;
|
offs += _subFileStart;
|
||||||
|
|
|
@ -56,13 +56,13 @@ class BufferedSeekableReadStreamTestSuite : public CxxTest::TestSuite {
|
||||||
b = ssrs.readByte();
|
b = ssrs.readByte();
|
||||||
TS_ASSERT( ssrs.eos() );
|
TS_ASSERT( ssrs.eos() );
|
||||||
|
|
||||||
ssrs.seek(3, SEEK_END);
|
ssrs.seek(-3, SEEK_END);
|
||||||
TS_ASSERT( !ssrs.eos() );
|
TS_ASSERT( !ssrs.eos() );
|
||||||
TS_ASSERT_EQUALS( ssrs.pos(), 7 );
|
TS_ASSERT_EQUALS( ssrs.pos(), 7 );
|
||||||
b = ssrs.readByte();
|
b = ssrs.readByte();
|
||||||
TS_ASSERT_EQUALS( b, 7 );
|
TS_ASSERT_EQUALS( b, 7 );
|
||||||
|
|
||||||
ssrs.seek(8, SEEK_END);
|
ssrs.seek(-8, SEEK_END);
|
||||||
TS_ASSERT_EQUALS( ssrs.pos(), 2 );
|
TS_ASSERT_EQUALS( ssrs.pos(), 2 );
|
||||||
b = ssrs.readByte();
|
b = ssrs.readByte();
|
||||||
TS_ASSERT_EQUALS( b, 2 );
|
TS_ASSERT_EQUALS( b, 2 );
|
||||||
|
|
61
test/common/memoryreadstream.h
Normal file
61
test/common/memoryreadstream.h
Normal file
|
@ -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());
|
||||||
|
}
|
||||||
|
};
|
|
@ -58,13 +58,13 @@ class SeekableSubReadStreamTestSuite : public CxxTest::TestSuite {
|
||||||
b = ssrs.readByte();
|
b = ssrs.readByte();
|
||||||
TS_ASSERT( ssrs.eos() );
|
TS_ASSERT( ssrs.eos() );
|
||||||
|
|
||||||
ssrs.seek(3, SEEK_END);
|
ssrs.seek(-3, SEEK_END);
|
||||||
TS_ASSERT( !ssrs.eos() );
|
TS_ASSERT( !ssrs.eos() );
|
||||||
TS_ASSERT_EQUALS( ssrs.pos(), 5 );
|
TS_ASSERT_EQUALS( ssrs.pos(), 5 );
|
||||||
b = ssrs.readByte();
|
b = ssrs.readByte();
|
||||||
TS_ASSERT_EQUALS( b, 6 );
|
TS_ASSERT_EQUALS( b, 6 );
|
||||||
|
|
||||||
ssrs.seek(8, SEEK_END);
|
ssrs.seek(-8, SEEK_END);
|
||||||
TS_ASSERT_EQUALS( ssrs.pos(), 0 );
|
TS_ASSERT_EQUALS( ssrs.pos(), 0 );
|
||||||
b = ssrs.readByte();
|
b = ssrs.readByte();
|
||||||
TS_ASSERT_EQUALS( b, 1 );
|
TS_ASSERT_EQUALS( b, 1 );
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue