Opened 11 years ago

Closed 9 years ago

#3145 closed bug (fixed)

fseek does not discard ungetc buffer

Reported by: bhaible Owned by: zooey
Priority: normal Milestone: R1
Component: System/libroot.so Version: R1/pre-alpha1
Keywords: Cc: haiku@…
Blocked By: Blocking:
Has a Patch: yes Platform: All

Description

ISO C 99 section 7.19.9.2 specifies that "a successful call to the fseek function undoes any effects of the ungetc function on the stream". Same in POSIX, see

http://www.opengroup.org/susv3/functions/fseek.html

Attached is a test case showing that this is not fulfilled in Haiku.

Attachments (5)

fseek-bug.c (900 bytes ) - added by bhaible 11 years ago.
test case
haiku-fseek-bug.png (2.9 KB ) - added by bhaible 11 years ago.
screenshot of execution of the test case
fseek-ftell-bug.c (1.2 KB ) - added by bhaible 11 years ago.
a second test case
haiku-fseek-ftell-bug.png (3.1 KB ) - added by bhaible 11 years ago.
screenshot of execution of the second test case
fseek-fix.diff (1.7 KB ) - added by hamish 9 years ago.
Fixes ungetc/fseek bug as described in #3145

Download all attachments as: .zip

Change History (15)

by bhaible, 11 years ago

Attachment: fseek-bug.c added

test case

by bhaible, 11 years ago

Attachment: haiku-fseek-bug.png added

screenshot of execution of the test case

by bhaible, 11 years ago

Attachment: fseek-ftell-bug.c added

a second test case

by bhaible, 11 years ago

Attachment: haiku-fseek-ftell-bug.png added

screenshot of execution of the second test case

comment:2 by scottmc, 10 years ago

Cc: haiku@… added

by hamish, 9 years ago

Attachment: fseek-fix.diff added

Fixes ungetc/fseek bug as described in #3145

comment:3 by hamish, 9 years ago

Has a Patch: set

With these changes, both tests work fine. I observed no repercussions elsewhere, but more testing is recommended.

Last edited 9 years ago by hamish (previous) (diff)

comment:4 by zooey, 9 years ago

Owner: changed from axeld to zooey
Status: newin-progress

in reply to:  3 comment:5 by zooey, 9 years ago

Replying to hamish:

With these changes, both tests work fine. I observed no repercussions elsewhere, but more testing is recommended.

Hm, is there some reasoning behind this patch, or did you get there by trial and error?

Please elaborate, as I don't quite understand the patch.

comment:6 by hamish, 9 years ago

_IO_buf_base points to the start of the entire stream buffer, while _IO_read_base points to the start of the read part of the stream buffer. ungetc() and friends store characters in the bytes between _IO_buf_base and _IO_read_base, so the seek function must take these into account and skip them.

in reply to:  6 comment:7 by zooey, 9 years ago

Replying to hamish:

_IO_buf_base points to the start of the entire stream buffer, while _IO_read_base points to the start of the read part of the stream buffer. ungetc() and friends store characters in the bytes between _IO_buf_base and _IO_read_base, so the seek function must take these into account and skip them.

Not quite, no: ungetc() invokes _IO_sputbackc(), which puts back characters between _IO_read_ptr and _IO_read_base - _IO_buf_base doesn't play any role there.

That's why I still fail to understand what exactly your patch does that causes the good effect. More importantly, I'm not at all sure it doesn't actually break other use cases. Have you tried running all gnulib tests to see if applying your patch doesn't introduce new problems?

comment:8 by hamish, 9 years ago

I got tst-fseek.c from the latest glibc. After disabling all the stats-related tests (stat64 stuff missing), one fread test fails. Without my patch, four fseek tests also fail.

Yes, I was wrong about why it worked. I think I've figured it out now.

The main read area starts at _IO_buf_base (see _IO_switch_to_get_mode). The put-back read area actually exists elsewhere in memory and isn't contiguous with the main read area. When the two areas are switched between, _IO_read_base is moved. After an ungetc(), _IO_read_base points to the put-back area, so any subsequent call to fseek() seeks in there, which it shouldn't.

Given that fseek() should ignore put-back characters, it makes sense to seek relative to the position of the normal read buffer, which is _IO_buf_base.

Last edited 9 years ago by hamish (previous) (diff)

comment:9 by zooey, 9 years ago

The proposed patch indeed fixes the problem and I have been unable to find any negative side effects.

Applied in hrev40055 - thanks a lot!

comment:10 by zooey, 9 years ago

Resolution: fixed
Status: in-progressclosed
Note: See TracTickets for help on using tickets.