Skip to content

Warn about read into zero-length Vec #8886

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

Closed
joshtriplett opened this issue May 25, 2022 · 6 comments · Fixed by #8964
Closed

Warn about read into zero-length Vec #8886

joshtriplett opened this issue May 25, 2022 · 6 comments · Fixed by #8964
Assignees
Labels
A-lint Area: New lints E-help-wanted Call for participation: Help is requested to fix this issue.

Comments

@joshtriplett
Copy link
Member

joshtriplett commented May 25, 2022

What it does

This lint catches reads into a zero-length Vec. For instance, it should flag code like this:

let mut data = Vec::with_capacity(len);
r.read_exact(&mut data)?;

In general, it should flag any code that constructs a Vec (whether using new or vec![] or especially with_capacity) and then reads into it (using read or read_exact, whether sync or async versions) without changing the size of the Vec. The lint should warn that the number of bytes read depends on the size of the buffer read into, so this pattern will read 0 bytes. And in the specific case of a call to with_capacity, the lint should warn that read gets the number of bytes from the Vec's length, not its capacity.

In the with_capacity case, the lint should suggest adding something like data.resize(len, 0);; in other cases the lint can more generally suggest resizing before reading, or calling read_to_end.

Lint Name

read_zero_byte_vec

Category

correctness

Advantage

Correctness: reading zero bytes is almost certainly not the intended behavior of this code. This lint would catch a bug.

I've seen multiple instances of this bug, and I just wrote one myself.

Drawbacks

In theory, a very unusual read implementation could assign some semantic meaning to zero-byte reads. But it seems exceptionally unlikely that code intending to do a zero-byte read would allocate a Vec for it.

Example

let mut data = Vec::with_capacity(len);
r.read_exact(&mut data)?;

Could be written as:

let mut data = Vec::with_capacity(len);
data.resize(len, 0);
r.read_exact(&mut data)?;

let mut data = Vec::with_capacity(len);
r.read_exact(&mut data).await?;

Could be written as:

let mut data = Vec::with_capacity(len);
data.resize(len, 0);
r.read_exact(&mut data).await?;
@joshtriplett joshtriplett added the A-lint Area: New lints label May 25, 2022
@tamaroning
Copy link
Contributor

@rustbot claim

@tamaroning
Copy link
Contributor

tamaroning commented Jun 6, 2022

Hi, now I am trying to cover as many cases as possible.
Here are some of the cases:

// 1
let mut data = Vec::new();
f.read(&mut data)?;
// 2
let mut data = Vec::new();
let res = f.read(&mut data);
// 3
let mut data = Vec::new();
let usize = f.read(&mut data).unwrap();
// and many other cases...

I am trying to catch all uses of such reads by pattern matching but I think it is almost impossible.
Does clippy or rustc have any useful util for this?

@rustbot label +E-help-wanted

@rustbot rustbot added the E-help-wanted Call for participation: Help is requested to fix this issue. label Jun 6, 2022
@joshtriplett
Copy link
Member Author

From a dataflow perspective, all of those have the same structure: there's a call to read (or to read_exact) whose argument is a Vec known to be zero-size (because it hasn't been changed since a call to Vec::new() or Vec::with_capacity(...) or vec![]).

@xFrednet
Copy link
Member

xFrednet commented Jun 7, 2022

Hey @tamaroning, as mentioned by joshtriplett, these examples are almost the same from a dataflow perspective, the question is now how we can catch these. The simple approach would be to first find a vector initialization and then just check the next statement. Alternatively, you can take a look at a Visitor or expr_visitor. I suggest searching in Clippy to see example usages.

Possible interesting implementations:


Also, as a sidenote, I and most likely others rarely filter by the E-help-wanted label. If it seems like your help request was missed, you can also ping us or use another channel of communication. 🙃

@tamaroning
Copy link
Contributor

@joshtriplett @xFrednet
thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints E-help-wanted Call for participation: Help is requested to fix this issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants