Skip to content

Warn about read into zero-length Vec#8964

Merged
bors merged 1 commit into
rust-lang:masterfrom
tamaroning:read_zero_byte_vec
Jun 15, 2022
Merged

Warn about read into zero-length Vec#8964
bors merged 1 commit into
rust-lang:masterfrom
tamaroning:read_zero_byte_vec

Conversation

@tamaroning

Copy link
Copy Markdown
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
Copy Markdown

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

@dswij dswij left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown
Contributor Author

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

@dswij

dswij commented Jun 15, 2022

Copy link
Copy Markdown
Member

Thanks for this!

@bors r+

@bors

bors commented Jun 15, 2022

Copy link
Copy Markdown
Contributor

📌 Commit 14478bb has been approved by dswij

@bors

bors commented Jun 15, 2022

Copy link
Copy Markdown
Contributor

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

@bors

bors commented Jun 15, 2022

Copy link
Copy Markdown
Contributor

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

@unrealhoang

Copy link
Copy Markdown

@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

dswij commented Aug 1, 2022

Copy link
Copy Markdown
Member

@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
Copy Markdown

@dswij I created a new issue: #9274

@dswij

dswij commented Aug 1, 2022

Copy link
Copy Markdown
Member

@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