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

Add a Reader::fill() method #13049

Merged
merged 2 commits into from
Mar 24, 2014
Merged

Add a Reader::fill() method #13049

merged 2 commits into from
Mar 24, 2014

Conversation

alexcrichton
Copy link
Member

This method can be used to fill a byte slice of data entirely, and it's considered an error if any error happens before its entirely filled.

/// bytes are read.
fn fill(&mut self, buf: &mut [u8]) -> IoResult<()> {
let mut read = 0;
while read < buf.len() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be an infinite loop if .read reads zero bytes repeatedly; is this a concern?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On unices, a 0-length read indicates EOF, and the I/O implementation layer is responsible for translating that to an error, and I imagine that windows is fairly similar.

I'm not 100% certain that a 0-length read returning infinitely is impossible, but I'm fairly certain it won't happen.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are readers that exist that don't end up calling POSIX read() (or the Windows equivalent).

@huonw
Copy link
Member

huonw commented Mar 21, 2014

r=me if possibility of an infinite loop isn't a problem.

I've found a common use case being to fill a slice (not an owned vector)
completely with bytes. It's posible for short reads to happen, and if you're
trying to get an exact number of bytes then this helper will be useful.
It's possible for a reader to have a short read, and there's no reason the task
should fail in this scenario. By using fill(), this has a stronger guarantee
that the buffer will get filled with data.
@huonw
Copy link
Member

huonw commented Mar 24, 2014

Should we land this with an #[experimental="may loop forever on some Readers"] on fill so that we can fix the RNG bug, without having to resolve the possible-infinite-loop issue right now? (r=me with an #[experimental], a FIXME and a bug, if you think this is appropriate.)

@alexcrichton
Copy link
Member Author

@sfackler points out that this is not a problem locally with this function, all of the default methods on the Reader trait will infinitely loop.

I do not think that this is a bug, you just shouldn't write a reader which continuously returns 0-length reads.

@lilyball
Copy link
Contributor

@alexcrichton Writing a non-blocking reader seems like a valid thing to do, as long as you're aware of the pitfalls.

My biggest worry is that all the methods should work if a single call to the Reader returns 0. I'm less concerned about infinitely looping 0's, although I think it would be nice to address that. Ideally any default Reader method that loops until it gets non-0 would also have a counter and abort if it loops too many times without any data (and of course resetting the counter if it gets any non-0 result back, even if it continues looping).

I feel like Go had an issue around this same problem last year, one that I filed, and that they eventually addressed it, but I forget the particulars.

@alexcrichton
Copy link
Member Author

@kballard, I would recommend opening up an issue about this. This plagues all methods on the Reader trait, not just this one.

@lilyball
Copy link
Contributor

@alexcrichton Will do. I found one checkin for Go making one method that uses a Reader bail out if it gets 100 0-length reads, but I suspect we can do better. I also feel like there was more to it, but searching Go's history isn't easy.

bors added a commit that referenced this pull request Mar 24, 2014
This method can be used to fill a byte slice of data entirely, and it's considered an error if any error happens before its entirely filled.
@bors bors closed this Mar 24, 2014
@bors bors merged commit 02dab5a into rust-lang:master Mar 24, 2014
@alexcrichton alexcrichton deleted the io-fill branch March 25, 2014 17:07
lnicola pushed a commit to lnicola/rust that referenced this pull request Aug 23, 2022
fix: resolve associated types of bare dyn types

Fixes rust-lang#13031

We've been dropping the associated type bindings of trait object types that were written without the `dyn` keyword. This patch reuses the lowering logic for `TypeRef::DynTrait` so the associated type bindings are properly lowered.
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jul 11, 2024
Remove `is_in_test_module_or_function`

Uses are replaced with `is_in_test` for consistency with other lints and to simplify the implementation of the lints. This means the module name is no longer checked, but that was a horrible hack from a time when late passes couldn't see `#[cfg(..)]` attributes.

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants