Opened 11 years ago

Closed 11 years ago

Last modified 12 months ago

#3944 closed defect (fixed)

COMMON: readLine_NEW fails to read last line in file

Reported by: salty-horse Owned by: wjp
Priority: normal Component: --Other--
Keywords: Cc:
Game:

Description

Using latest svn r34343.

If the last "line" in a file doesn't end with a newline, readLine_NEW will return 0 instead of the real data.

In my case, I am opening a file with Common::File::open() and calling readLine_NEW on it.

Suppose the last characters of the file are 'a', 'b' followed by EOF.

readLine_NEW will execute "c = readByte();" in common/stream.cpp:166 twice.
After the third time, c will be '\0'.
It then gets to the check:

if (ioFailed() || (len == 0 && eos()))
return 0;

ioFailed() returns True and then 0 is returned, even if len > 0.

Maybe the real problem is ioFailed returning 'true' when nothing is actually failing.
It's set when readByte() calls File::read(&b, 1).

AFAIK, this doesn't actually cause any errors in existing engines. I've encountered it in my KoM engine.

Ticket imported from: #2095303. Ticket imported from: bugs/3944.

Attachments (1)

readline_fix.patch (825 bytes ) - added by salty-horse 11 years ago.

Download all attachments as: .zip

Change History (7)

by salty-horse, 11 years ago

Attachment: readline_fix.patch added

comment:1 by salty-horse, 11 years ago

Attaching a proposed fix.

It changes File::read() to *not* set ioFailed if it's the end of the file, and adds a check for eos() in readLine_NEW()
File Added: readline_fix.patch

comment:2 by fingolfin, 11 years ago

Owner: set to fingolfin

comment:3 by fingolfin, 11 years ago

Fixed thanks to the recent Stream::eos() work by wjp. Confirmed by a unit test, at least for MemoryReadStream.

File streams *should* also work, if not, please reopen this item. I hope that we can add a unit test for those eventually, too, but this will require improving the "make test" linking system, as we'd have to compile in backends/ into our unit test runner.

comment:4 by fingolfin, 11 years ago

Owner: changed from fingolfin to wjp
Resolution: fixed
Status: newclosed

comment:5 by salty-horse, 11 years ago

Confirming file streams work fine now. Thanks for fixing!

comment:6 by digitall, 12 months ago

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