Opened 12 years ago

Closed 12 years ago

Last modified 2 years ago

#4209 closed defect (fixed)

ALL: various Stream::seek(SEEK_END impls differs from docs

Reported by: SF/tobigun Owned by: fingolfin
Priority: high Component: --Other--
Keywords: Cc:
Game:

Description

MemoryReadStream::seek() does not calculate the offset correctly if whence is set to SEEK_END. It calculates the offset as

offs = _size - offs;

what is wrong as the offs parameter is already negative (as it is relative to the end) and so the offset is added, resulting in an offset > eof.

The correct calculation should be

offs = _size + offs;

Same applies to SeekableSubReadStream::seek().

These bugs cause crashes if a GZipReadStream is wrapped around a memory-stream as the gzip-constructor seeks backwards from the end (seek(-4, SEEK_END)). StdioStream::seek() (normally used in conjunction with GZipReadStream) which relies on fseek() handles SEEK_END correctly.

Ticket imported from: #2664460. Ticket imported from: bugs/4209.

Attachments (1)

stream-offset.patch (652 bytes ) - added by SF/tobigun 12 years ago.

Download all attachments as: .zip

Change History (6)

by SF/tobigun, 12 years ago

Attachment: stream-offset.patch added

comment:1 by fingolfin, 12 years ago

Inded, the current code contradicts the documentation (and the behavior of fseek).

But this patch alone is dangerous. First off, the unit tests need to be adapted accordingly. Secondly, we need to check all code using SEEK_END (or implementing other streams) for compliance.

comment:2 by fingolfin, 12 years ago

Owner: set to fingolfin
Priority: normalhigh
Summary: Wrong offset calculation for memory-streams with SEEK_ENDALL: various Stream::seek(SEEK_END impls differs from docs

comment:3 by fingolfin, 12 years ago

OK, I think I found and fixed all wrong seek implentations, as well as all code using it incorrectly. Thank you very much for reporting this bug. It may well account for some weird problem occurring only on some ports.

comment:4 by fingolfin, 12 years ago

Resolution: fixed
Status: newclosed

comment:5 by digitall, 2 years ago

Component: --Other--
Note: See TracTickets for help on using tickets.