read_zero_byte_vec emitted when read is in the next block

[unrealhoang]

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 with resize before read, the lint is still fired.

Lint Name

read_zero_byte_vec

Reproducer

I tried this code:

    let mut v = Vec::new();
    {
        v.resize(10, 0);
        r.read(&mut v).unwrap();
    }

I saw this happen:

error: reading zero byte data to `Vec`
   --> src/main.rs:10:5
    |
130 | /     {
131 | |         v.resize(10, 0);
132 | |         r.read(&mut v).unwrap();
133 | |     }
    | |_____^
    |
    = note: `#[deny(clippy::read_zero_byte_vec)]` on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#read_zero_byte_vec

I expected to not seeing this lint fired off.

Version

rustc 1.64.0-nightly (0f4bcadb4 2022-07-30)
binary: rustc
commit-hash: 0f4bcadb46006bc484dad85616b484f93879ca4e
commit-date: 2022-07-30
host: x86_64-unknown-linux-gnu
release: 1.64.0-nightly
LLVM version: 14.0.6

Additional Labels

No response

[tamaroning]

I added this lint. (#8964)
For now, this lint checks reads immidiately after creating Vecs and dosen't catch other many cases.
For example, the code below doesn't trigger the lint.

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.

[crepererum]

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;
    }
}

[wildbook]

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.

[joshtriplett]

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.

[joshtriplett]

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.

[joshtriplett]

I'm still encountering this. I'd love to see this lint improved to have better dataflow handling.

[Stargateur]

a lint that bug shouldn't be in deny by default, warn at best.

[joshtriplett]

Yeah, if this can't be easily fixed (and I'm hoping it can be), it should be disabled due to false positives.

[nyurik]

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)?;
  }
}

[iczero]

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);

[Ralith]

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?

[dswij]

@rustbot claim

[pac-work]

We have encountered this bug yesterday with clippy 0.1.72 (d5c2e9c 2023-09-13). The interesting thing is that it does not fire when initializing using construct similar to this: let mut buffer = vec![0; BUFFER_SIZE];. See full attached example:

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;
        }
    }
}

[dswij]

@pac-work the lint only fires for Vec::with_capacity, Vec::default, or Vec::new. It won't fire with vec![] inits.

[bonsairobo]

I just hit this. I echo the sentiment of other comments: don't lint this by default until false positives are fixed.