Re: [PATCH libgpiod 4/4] bindings: rust: clippy: silence false-positive on iterator

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



For future reference, the subject line should be "[libgpiod][PATCH...",
as per the README.
Makes it easier to filter visually, if nothing else.

On Thu, Jun 29, 2023 at 01:09:02PM +0200, Erik Schilling wrote:
> This was fixed, but it is not in stable yet.
> 

This is not a good checkin comment.
State what the problem is and how the patch addresses it.

Same applies to other patches in the series - but I have other comments
on this one, so raising it here.

> Tested build on x86_64, armv7hf, aarch64.
> 

When you say "Tested build", do you mean just compile/clippy, or have you
actually run tests?

Either way, not sure if this should go in the checkin comment - it is
generally implied by the Signed-off that you've tested it to your
satisfaction.
No problem with a more detailed description of how you've tested in
the cover letter.

> Reported-by: Kent Gibson <warthog618@xxxxxxxxx>
> Link: https://lore.kernel.org/r/20230612154055.56556-1-warthog618@xxxxxxxxx
> Signed-off-by: Erik Schilling <erik.schilling@xxxxxxxxxx>
> ---
>  bindings/rust/libgpiod/src/event_buffer.rs | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/bindings/rust/libgpiod/src/event_buffer.rs b/bindings/rust/libgpiod/src/event_buffer.rs
> index b79e9ea..2e4bfd3 100644
> --- a/bindings/rust/libgpiod/src/event_buffer.rs
> +++ b/bindings/rust/libgpiod/src/event_buffer.rs
> @@ -54,6 +54,9 @@ impl<'a> Iterator for Events<'a> {
>      }
>  
>      fn next(&mut self) -> Option<Self::Item> {
> +        // clippy false-positive, fixed in next clippy release:
> +        // https://github.com/rust-lang/rust-clippy/issues/9820
> +        #[allow(clippy::iter_nth_zero)]
>          self.nth(0)
>      }
>  }
> 

Specify the release in absolute terms, not "next clippy release".

Other than those nits, I'm good with the actual changes in the series
(checked with clippy and running tests on a variety of 32 and 64bit
platforms and compiler versions back to 1.60)

(I am seeing this one test failure on arm32, but that doesn't seem to be related
to this patch:
---- request_config::verify::default stdout ----
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `OperationFailed(RequestConfigGetConsumer, Errno { code: 2, description: Some("No such file or directory") })`,
 right: `OperationFailed(RequestConfigGetConsumer, Errno { code: 0, description: Some("Success") })`', libgpiod/tests/request_config.rs:18:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Not sure if that is a genuine bug or something funky in my build.)

Cheers,
Kent.



[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux