read_zero_byte_vec emitted when read is in the next block
#9274
Comments
unrealhoang
added
C-bug
|
I added this lint. (#8964) let mut data = Vec::with_capacity(100);
();
f.read(&mut data).unwrap();Linting all cases by just using AST is tough so I made a compromise. |
This lint is triggering incorrectly, maybe due to: rust-lang/rust-clippy#9274
This lint is triggering incorrectly, maybe due to: rust-lang/rust-clippy#9274
|
I've also encountered this and found it quite confusing under which conditions the lint is triggered: // triggers read_zero_byte_vec
pub fn f1<T: std::io::Read>(x: &mut T) {
let mut to_read = 100;
let mut buf = vec![];
while to_read > 0 {
let step = to_read.min(10);
buf.resize(step, 0);
x.read_exact(&mut buf).unwrap();
to_read -= step;
}
}
// does NOT trigger read_zero_byte_vec
pub fn f2<T: std::io::Read>(x: &mut T) {
let mut buf = vec![];
let mut to_read = 100;
while to_read > 0 {
let step = to_read.min(10);
buf.resize(step, 0);
x.read_exact(&mut buf).unwrap();
to_read -= step;
}
}
// does NOT trigger read_zero_byte_vec
pub fn f3<T: std::io::Read>(x: &mut T) {
let mut to_read = 100;
while to_read > 0 {
let mut buf = vec![];
let step = to_read.min(10);
buf.resize(step, 0);
x.read_exact(&mut buf).unwrap();
to_read -= step;
}
} |
|
I just discovered this lint through false positives when running clippy against older code. All of my so far hits can be simplified down to variants of this: let mut data = vec![];
match () {
() => {
data.resize(10, 0);
input.read_exact(&mut data)?;
},
}It'd be nice if there was a mention of the false positives or restrictions in the lint index, similar to most other lints with known failure cases. I'm also not sure that a lint with so easy-to-trigger false positives should be deny by default, but perhaps I'm just unlucky with it. |
|
It's fine if this has false negatives, but it needs to not have false positives. This shouldn't be checking "the next statement" in a way that includes the whole next block. |
|
I ran into this with code that reduces to the following: pub fn func(mut stream: impl std::io::Read) {
let mut size_buf = [0u8; 4];
let mut buf = Vec::new();
loop {
stream.read_exact(&mut size_buf).unwrap();
let size = u32::from_le_bytes(size_buf) as usize;
buf.resize(size, 0);
stream.read_exact(&mut buf).unwrap();
}
}This is literally reading the size and resizing the buffer to the appropriate size before reading. |
This avoids reporting false-positives; see rust-lang/rust-clippy#9274 for details.
[stable] Lower lint level for READ_ZERO_BYTE_VEC This avoids reporting false-positives; see rust-lang/rust-clippy#9274 for details. cc `@rust-lang/clippy` -- do we want a direct change landed on stable here? If so, please r this PR, otherwise we can just close it. Would appreciate confirmation this is the right change to make as well. cc `@joshtriplett` -- filing due to https://rust-lang.zulipchat.com/#narrow/stream/301329-t-devtools/topic/clippy.20false.20positive
|
I'm still encountering this. I'd love to see this lint improved to have better dataflow handling. |
|
a lint that bug shouldn't be in deny by default, warn at best. |
|
Yeah, if this can't be easily fixed (and I'm hoping it can be), it should be disabled due to false positives. |
|
This example also generated this false-positive, and allowing it for any specific line did not work - I had to wrap it in a block: #[allow(clippy::read_zero_byte_vec)]
{
let mut buf = Vec::with_capacity(2048);
for i in 0..10 {
buf.resize(100, 0);
reader.read_exact(&mut buf)?;
}
} |
|
Hello, I've encountered a false positive with the following code: let file = File::open(args.input_file)?;
let mut reader = BufReader::new(file);
let mut pkt_buf = Vec::new();
loop {
let mut len_buf = [0u8; 4];
reader.read_exact(&mut len_buf)?;
let len = parse_pkt_line_length(&len_buf);
println!("{}", len);
if len <= 4 {
continue;
}
pkt_buf.resize((len - 4) as usize, 0);
reader.read_exact(&mut pkt_buf)?;
dump_as_readable_ascii(&pkt_buf, true);
}Swapping these two lines fixes it: let mut pkt_buf = Vec::new();
let mut reader = BufReader::new(file); |
|
This is still happening: loop {
let mut len = [0; 2];
stream.read_exact(&mut len).await?;
let len = u16::from_le_bytes(len);
buf.resize(len.into(), 0);
stream.read_exact(&mut buf).await?;
// ...
}Can we just disable this broken lint until it's fixed? |
|
@rustbot claim |
move `read_zero_byte_vec` to nursery I think the concerns in #9274 are valid, and we should move this to nursery while we're reworking this. changelog: [`read_zero_byte_vec`] moved to nursery
|
We have encountered this bug yesterday with use std::fs::File;
use std::io::Read;
/// This is a very simplified example of strange Clippy behavior we see. The code logic may not make
/// sense to you that much, real world code is much more complex, involves some serial lines,
/// waiting for data to be available there, reading different amounts of data in each loop, etc.
fn main() {
const BUFFER_SIZE: usize = 100;
let mut file = File::open("/tmp/test.txt").unwrap();
// Depending on the vector initialization below, one does or does not get the error of "reading
// zero byte data to `Vec`". One is more performant then the other, true, but neither of them
// ever reads 0 bytes.
let mut buffer = Vec::new();
//let mut buffer = vec![0; BUFFER_SIZE];
loop {
buffer.resize(BUFFER_SIZE, 0);
let bytes_read = file.read(&mut buffer).unwrap();
buffer.truncate(bytes_read);
println!("{:?}", buffer);
if bytes_read == 0 || buffer[0] == b'0' {
break;
}
}
} |
|
@pac-work the lint only fires for |
|
I just hit this. I echo the sentiment of other comments: don't lint this by default until false positives are fixed. |
`read_zero_byte_vec` refactor for better heuristics Fixes #9274 Previously, the implementation of `read_zero_byte_vec` only checks for the next statement after the vec init. This fails when there is a block with statements that are expanded and walked by the old visitor. This PR refactors so that: 1. It checks if there is a `resize` on the vec 2. It works on blocks properly e.g. This should properly lint now: ``` let mut v = Vec::new(); { f.read(&mut v)?; //~^ ERROR: reading zero byte data to `Vec` } ``` changelog: [`read_zero_byte_vec`] Refactored for better heuristics
`read_zero_byte_vec` refactor for better heuristics Fixes #9274 Previously, the implementation of `read_zero_byte_vec` only checks for the next statement after the vec init. This fails when there is a block with statements that are expanded and walked by the old visitor. This PR refactors so that: 1. It checks if there is a `resize` on the vec 2. It works on blocks properly e.g. This should properly lint now: ``` let mut v = Vec::new(); { f.read(&mut v)?; //~^ ERROR: reading zero byte data to `Vec` } ``` changelog: [`read_zero_byte_vec`] Refactored for better heuristics
Summary
As the heuristic is checking if the next statement of an empty vec includes
read(&mut v), if the next statement/expression is a block withresizebeforeread, the lint is still fired.Lint Name
read_zero_byte_vec
Reproducer
I tried this code:
I saw this happen:
I expected to not seeing this lint fired off.
Version
Additional Labels
No response
The text was updated successfully, but these errors were encountered: