Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

io::reader::read_byte for *FILE behaves surprisingly on EOF #2960

Closed
catamorphism opened this issue Jul 19, 2012 · 7 comments
Closed

io::reader::read_byte for *FILE behaves surprisingly on EOF #2960

catamorphism opened this issue Jul 19, 2012 · 7 comments
Labels
E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added.

Comments

@catamorphism
Copy link
Contributor

In the io::reader impl for *libc::FILE, read_byte returns -1 on EOF. This is not surprising since it just calls libc::fgetc, but it's arguably wrong since callers are presumably going to use eof() to check for EOF and thus shouldn't have to worry about dealing with a bogus trailing -1.

Instead, perhaps it could fail? I'm not really sure. Or maybe this is supposed to be a low-level interface, but it still seems weird for Rust code to have to deal with sentinel values like this.

@catamorphism
Copy link
Contributor Author

This came up with https://gist.github.com/8f81d3e2b39f31cca1cc -- maybe the real issue is that libc::eof is returning false one byte past when it should? I'm not really sure.

@Havvy
Copy link
Contributor

Havvy commented Jul 19, 2012

java.lang.Scanner uses hasNext() to say whether or not the file has anymore contents. Why not keep io::reader.eof for its low-level impl and add io::reader.has_next for that use case?

@Havvy
Copy link
Contributor

Havvy commented Jul 19, 2012

Since that gist is mine, I'd like to say that I find it odd I had to do a cast to get it in a format that is usable to convert to a str. I'd like read_str to return a u8 just to avoid the cast operation which feels inefficient to me.

@graydon
Copy link
Contributor

graydon commented Jun 6, 2013

I believe this is going to be completely gutted soon, and in the process I expect reading past EOF to become a condition. Consider this as a sub-bug of #6169 and/or #6168

@graydon
Copy link
Contributor

graydon commented Aug 1, 2013

still legit but still, I suspect, likely to be absorbed in the IO rewrite. Any day now.

@catamorphism
Copy link
Contributor Author

New IO should solve this problem, but if you add a test, you can close this :-)

@alexcrichton
Copy link
Member

This code has long since been purged, closing.

RalfJung pushed a commit to RalfJung/rust that referenced this issue Jul 8, 2023
Restore test filtering by substring.

Previously it was only looking for tests whose path was a prefix of the given filter

fixes rust-lang#2958
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added.
Projects
None yet
Development

No branches or pull requests

4 participants