#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-- |
| Version: | 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)
Change History (6)
by , 16 years ago
| Attachment: | stream-offset.patch added |
|---|
comment:1 by , 16 years ago
comment:2 by , 16 years ago
| Owner: | set to |
|---|---|
| Priority: | normal → high |
| Summary: | Wrong offset calculation for memory-streams with SEEK_END → ALL: various Stream::seek(SEEK_END impls differs from docs |
comment:3 by , 16 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 , 16 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
comment:5 by , 6 years ago
| Component: | → --Other-- |
|---|
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.