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

Reading more bytes than a file contains yields EOF instead of data #12892

Closed
jdm opened this issue Mar 14, 2014 · 7 comments · Fixed by #12907
Closed

Reading more bytes than a file contains yields EOF instead of data #12892

jdm opened this issue Mar 14, 2014 · 7 comments · Fixed by #12907

Comments

@jdm
Copy link
Contributor

jdm commented Mar 14, 2014

The most recent Rust upgrade for Servo has uncovered a problem with the code at larsbergstrom/servo@3445fc9#diff-b8f19d42333587901f805bdc9be6df88R16 . A call to reader.read_bytes(1024) on a file that is <1024 bytes long does not yield any data, and instead returns an immediate EOF error result.

@alexcrichton
Copy link
Member

Hm, it may be something else going on, this appears to work for me:

$ cat foo.rs
use std::io::File;

fn main() {
    let mut buf = [0, .. 2048];
    let mut f = File::open(&Path::new("foo.rs"));
    println!("{}", f.read(buf));
}
$ rustc foo.rs
$ ./foo
Ok(149)

@zkamsler
Copy link
Contributor

This appears to the documented behavior of read_bytes (not be be confused with read). It basically reads until it has read the specified number of bytes or there is an error.

@jdm
Copy link
Contributor Author

jdm commented Mar 14, 2014

Wow. That seems like a really niche use case in which it's acceptable to potentially lose bytes.

@alexcrichton
Copy link
Member

Ah sorry, yes, I see what's going on now. read_bytes is for reading an exact number of bytes, not for generally "read some bytes".

Perhaps is should be renamed to read_exact, it may have made more sense when conditions existed.

@jdm
Copy link
Contributor Author

jdm commented Mar 14, 2014

I'll note that using push_bytes doesn't seem to be much of an improvement if you're trying to read a file in chunks; I'm seeing similar behaviour with it.

@zkamsler
Copy link
Contributor

push_bytes will also bubble up an EOF if there are less than the amount requested, but the bytes that were read will still have been pushed to the buffer. All read_bytes does is call push_bytes on a new ~[u8].

See source:
https://github.com/mozilla/rust/blob/b4d324334cb48198c27d782002d75eba14a6abde/src/libstd/io/mod.rs#L380

But it definitely is not that great if you don't care about reading the exact number of bytes.

@sfackler
Copy link
Member

👍 to read_exact.

bors added a commit that referenced this issue Mar 22, 2014
These methods can be mistaken for general "read some bytes" utilities when
they're actually only meant for reading an exact number of bytes. By renaming
them it's much clearer about what they're doing without having to read the
documentation.

Closes #12892
dingxiangfei2009 pushed a commit to dingxiangfei2009/rust that referenced this issue Jul 28, 2024
… r=xFrednet

needless_borrows_for_generic_args: Fix for &mut

This commit fixes a bug introduced in rust-lang#12706, where the behavior of the lint has been changed, to avoid suggestions that introduce a move. The motivation in the commit message is quite poor (if the detection for significant drops is not sufficient because it's not transitive, the proper fix would be to make it transitive). However, rust-lang#12454, the linked issue, provides a good reason for the change — if the value being borrowed is bound to a variable, then moving it will only introduce friction into future refactorings.

Thus rust-lang#12706 changes the logic so that the lint triggers if the value being borrowed is Copy, or is the result of a function call, simplifying the logic to the point where analysing "is this the only use of this value" isn't necessary.

However, said PR also introduces an undocumented carveout, where referents that themselves are mutable references are treated as Copy, to catch some cases that we do want to lint against. However, that is not sound — it's possible to consume a mutable reference by moving it.

To avoid emitting false suggestions, this PR reintroduces the referent_used_exactly_once logic and runs that check for referents that are themselves mutable references.

Thinking about the code shape of &mut x, where x: &mut T, raises the point that while removing the &mut outright won't work, the extra indirection is still undesirable, and perhaps instead we should suggest reborrowing: &mut *x. That, however, is left as possible future work.

Fixes rust-lang#12856

changelog: none
Jarcho pushed a commit to Jarcho/rust that referenced this issue Aug 26, 2024
… r=xFrednet

needless_borrows_for_generic_args: Fix for &mut

This commit fixes a bug introduced in rust-lang#12706, where the behavior of the lint has been changed, to avoid suggestions that introduce a move. The motivation in the commit message is quite poor (if the detection for significant drops is not sufficient because it's not transitive, the proper fix would be to make it transitive). However, rust-lang#12454, the linked issue, provides a good reason for the change — if the value being borrowed is bound to a variable, then moving it will only introduce friction into future refactorings.

Thus rust-lang#12706 changes the logic so that the lint triggers if the value being borrowed is Copy, or is the result of a function call, simplifying the logic to the point where analysing "is this the only use of this value" isn't necessary.

However, said PR also introduces an undocumented carveout, where referents that themselves are mutable references are treated as Copy, to catch some cases that we do want to lint against. However, that is not sound — it's possible to consume a mutable reference by moving it.

To avoid emitting false suggestions, this PR reintroduces the referent_used_exactly_once logic and runs that check for referents that are themselves mutable references.

Thinking about the code shape of &mut x, where x: &mut T, raises the point that while removing the &mut outright won't work, the extra indirection is still undesirable, and perhaps instead we should suggest reborrowing: &mut *x. That, however, is left as possible future work.

Fixes rust-lang#12856

changelog: none
Jarcho pushed a commit to Jarcho/rust that referenced this issue Aug 26, 2024
… r=xFrednet

needless_borrows_for_generic_args: Fix for &mut

This commit fixes a bug introduced in rust-lang#12706, where the behavior of the lint has been changed, to avoid suggestions that introduce a move. The motivation in the commit message is quite poor (if the detection for significant drops is not sufficient because it's not transitive, the proper fix would be to make it transitive). However, rust-lang#12454, the linked issue, provides a good reason for the change — if the value being borrowed is bound to a variable, then moving it will only introduce friction into future refactorings.

Thus rust-lang#12706 changes the logic so that the lint triggers if the value being borrowed is Copy, or is the result of a function call, simplifying the logic to the point where analysing "is this the only use of this value" isn't necessary.

However, said PR also introduces an undocumented carveout, where referents that themselves are mutable references are treated as Copy, to catch some cases that we do want to lint against. However, that is not sound — it's possible to consume a mutable reference by moving it.

To avoid emitting false suggestions, this PR reintroduces the referent_used_exactly_once logic and runs that check for referents that are themselves mutable references.

Thinking about the code shape of &mut x, where x: &mut T, raises the point that while removing the &mut outright won't work, the extra indirection is still undesirable, and perhaps instead we should suggest reborrowing: &mut *x. That, however, is left as possible future work.

Fixes rust-lang#12856

changelog: none
MabezDev pushed a commit to esp-rs/rust that referenced this issue Sep 3, 2024
… r=xFrednet

needless_borrows_for_generic_args: Fix for &mut

This commit fixes a bug introduced in rust-lang#12706, where the behavior of the lint has been changed, to avoid suggestions that introduce a move. The motivation in the commit message is quite poor (if the detection for significant drops is not sufficient because it's not transitive, the proper fix would be to make it transitive). However, rust-lang#12454, the linked issue, provides a good reason for the change — if the value being borrowed is bound to a variable, then moving it will only introduce friction into future refactorings.

Thus rust-lang#12706 changes the logic so that the lint triggers if the value being borrowed is Copy, or is the result of a function call, simplifying the logic to the point where analysing "is this the only use of this value" isn't necessary.

However, said PR also introduces an undocumented carveout, where referents that themselves are mutable references are treated as Copy, to catch some cases that we do want to lint against. However, that is not sound — it's possible to consume a mutable reference by moving it.

To avoid emitting false suggestions, this PR reintroduces the referent_used_exactly_once logic and runs that check for referents that are themselves mutable references.

Thinking about the code shape of &mut x, where x: &mut T, raises the point that while removing the &mut outright won't work, the extra indirection is still undesirable, and perhaps instead we should suggest reborrowing: &mut *x. That, however, is left as possible future work.

Fixes rust-lang#12856

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 a pull request may close this issue.

4 participants