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

Warn about read into zero-length Vec #8964

Merged
merged 1 commit into from
Jun 15, 2022

Conversation

tamaroning
Copy link
Contributor

Closes #8886

  • Followed [lint naming conventions][lint_naming]
  • Added passing UI tests (including committed .stderr file)
  • cargo test passes locally
  • Executed cargo dev update_lints
  • Added lint documentation
  • Run cargo dev fmt

changelog: none

@rust-highfive
Copy link

r? @dswij

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jun 7, 2022
@tamaroning tamaroning force-pushed the read_zero_byte_vec branch from 23e1ff5 to 34b42f4 Compare June 7, 2022 16:19
Copy link
Member

@dswij dswij left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Changes looks great with the tests

Can you help to squash some commits? After that, I think it's good to merge

@tamaroning tamaroning force-pushed the read_zero_byte_vec branch from 34b42f4 to 2b84657 Compare June 14, 2022 14:18
@tamaroning tamaroning force-pushed the read_zero_byte_vec branch from 2b84657 to 14478bb Compare June 14, 2022 14:31
@tamaroning
Copy link
Contributor Author

@dswij
Thank you for your review.
Squashed my commits :)

@dswij
Copy link
Member

dswij commented Jun 15, 2022

Thanks for this!

@bors r+

@bors
Copy link
Contributor

bors commented Jun 15, 2022

📌 Commit 14478bb has been approved by dswij

@bors
Copy link
Contributor

bors commented Jun 15, 2022

⌛ Testing commit 14478bb with merge 844c06a...

@bors
Copy link
Contributor

bors commented Jun 15, 2022

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: dswij
Pushing 844c06a to master...

@unrealhoang
Copy link

@tamaroning
I found a false positive case for this lint:

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

will cause this lint to go off.

@dswij
Copy link
Member

dswij commented Aug 1, 2022

@unrealhoang Seems like this is because the lint does not transverse down scopes. Can you help to create a new issue for this?

@unrealhoang
Copy link

@dswij I created a new issue: #9274

@dswij
Copy link
Member

dswij commented Aug 1, 2022

@dswij I created a new issue: #9274

@unrealhoang Thanks for this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warn about read into zero-length Vec
5 participants