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

Clippy errors and says to use Read::read_exact inside implementations of Read::read_exact that don't directly use the output of Read::read #4836

Closed
slightlyoutofphase opened this issue Nov 22, 2019 · 3 comments · Fixed by #13605
Labels
C-bug Category: Clippy is not doing the correct thing L-suggestion Lint: Improving, adding or fixing lint suggestions

Comments

@slightlyoutofphase
Copy link

Given the following read_exact implementation:

#[inline(always)]
fn read_exact(&mut self, buf: &mut [u8]) -> io::Result<()> {
  //Our implementation of `read` always returns `Ok(read_length)`, so we can unwrap safely here.
  self.read(buf).unwrap();
  Ok(())
}

Clippy gives the following error:

error: handle read amount returned or use `Read::read_exact` instead
    --> src/lib.rs:1331:7
     |
1331 |       self.read(buf).unwrap();
     |       ^^^^^^^^^^^^^^^^^^^^^^^
     |
     = note: `#[deny(clippy::unused_io_amount)]` on by default
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unused_io_amount
@flip1995 flip1995 added the C-bug Category: Clippy is not doing the correct thing label Nov 22, 2019
@flip1995
Copy link
Member

flip1995 commented Nov 22, 2019

Hmm. Suggesting to use read_exact inside read_exact is definitely wrong. But I would rewrite this function as:

#[inline(always)]
fn read_exact(&mut self, buf: &mut [u8]) -> io::Result<()> {
  //Our implementation of `read` always returns `Ok(read_length)`, so we can unwrap safely here.
  let buf_len = buf.len();
  let bytes_read = self.read(buf).unwrap();
  assert_eq!(bytes_read, buf_len); // Or `debug_assert_eq!` or return an `Err`
  Ok(())
}

So I think the lint triggerring here is correct, it shouldn't suggest read_exact though.

@flip1995 flip1995 added the L-suggestion Lint: Improving, adding or fixing lint suggestions label Nov 22, 2019
@slightlyoutofphase
Copy link
Author

Good point! I adjusted it on my end.

On a general note I don't really get why read_exact doesn't just return io::Result<usize> like read does instead of io::Result<()> anyways, but obviously that's kind of a moot point.

@flip1995
Copy link
Member

I guess this is because by definition it would always return Ok(buf.len()), panic (not recommended) or return an Err.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing L-suggestion Lint: Improving, adding or fixing lint suggestions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants