| Message ID | 20230629-clippy-v1-0-9ff088713c54@linaro.org |
|---|---|
| Headers | show |
| Series | bindings: rust: clippy: fixed warnings | expand |
On 29-06-23, 13:08, Erik Schilling wrote: > This follows up on my promise on Kent's series [1] to look into whether > these casts are needed or not. Most are not, a few are false-positives. > > I also explored some shunit2 based test-script to automate the testing, > but that became ugly with linking issue and needs me to revisit it > another time. So this only sends the fixes for now. > > [1] https://lore.kernel.org/r/20230612154055.56556-1-warthog618@gmail.com > > To: Linux-GPIO <linux-gpio@vger.kernel.org> > Cc: Kent Gibson <warthog618@gmail.com> > Cc: Viresh Kumar <viresh.kumar@linaro.org> > > Signed-off-by: Erik Schilling <erik.schilling@linaro.org> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
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@gmail.com> > Link: https://lore.kernel.org/r/20230612154055.56556-1-warthog618@gmail.com > Signed-off-by: Erik Schilling <erik.schilling@linaro.org> > --- > 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.
On Fri, Jun 30, 2023 at 12:05:28PM +0200, Erik Schilling wrote: > On Fri Jun 30, 2023 at 11:08 AM CEST, Kent Gibson wrote: > > > > > (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.) > > Is the GPIO_SIM module loaded? Did you test with a custom kernel or > some distro that ships with it? > That is the only test failing out of the whole suite, so gpiosim is not the problem. That is with the latest from the Raspberry Pi rpi-6.4.y branch[1] (a867309b7a55 so a few days old now), with CONFIG_GPIO_SIM so that I can run the tests. Happens to be running on a Pi ZeroW, but I don't think that test is speed sensitive. I have done a complete rebuild - same result. Are there any distos enabling GPIO_SIM yet? Cheers, Kent. [1] https://github.com/raspberrypi/linux
On Fri Jun 30, 2023 at 12:19 PM CEST, Kent Gibson wrote: > On Fri, Jun 30, 2023 at 12:05:28PM +0200, Erik Schilling wrote: > > On Fri Jun 30, 2023 at 11:08 AM CEST, Kent Gibson wrote: > > > (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.) > > > > Is the GPIO_SIM module loaded? Did you test with a custom kernel or > > some distro that ships with it? > > > > That is the only test failing out of the whole suite, so gpiosim is not > the problem. > > That is with the latest from the Raspberry Pi rpi-6.4.y branch[1] > (a867309b7a55 so a few days old now), with CONFIG_GPIO_SIM so that I can > run the tests. > Happens to be running on a Pi ZeroW, but I don't think that test is speed > sensitive. I have done a complete rebuild - same result. > > Are there any distos enabling GPIO_SIM yet? Fedora does now (after I asked for it [2]). But it does not support any 32-bit ARM targets anymore :/. Can you try reproducing it without the patches? I would be surprised if this was related to the patches. I will rebuild my armv7hf VM soon and retry with a self-built kernel. but not sure when I will get around that. [2] https://bugzilla.redhat.com/show_bug.cgi?id=2215980 - Erik
On Fri, Jun 30, 2023 at 12:46:11PM +0200, Erik Schilling wrote: > On Fri Jun 30, 2023 at 12:19 PM CEST, Kent Gibson wrote: > > > > > > > That is the only test failing out of the whole suite, so gpiosim is not > > the problem. > > > > That is with the latest from the Raspberry Pi rpi-6.4.y branch[1] > > (a867309b7a55 so a few days old now), with CONFIG_GPIO_SIM so that I can > > run the tests. > > Happens to be running on a Pi ZeroW, but I don't think that test is speed > > sensitive. I have done a complete rebuild - same result. > > > > Are there any distos enabling GPIO_SIM yet? > > Fedora does now (after I asked for it [2]). But it does not support any > 32-bit ARM targets anymore :/. Can you try reproducing it without the > patches? I would be surprised if this was related to the patches. > Tried that - same result with libgpiod master. So it is not from your patches. Cheers. Kent. > I will rebuild my armv7hf VM soon and retry with a self-built kernel. > but not sure when I will get around that. > > [2] https://bugzilla.redhat.com/show_bug.cgi?id=2215980 > > - Erik
On Fri, Jun 30, 2023 at 12:53:36PM +0200, Erik Schilling wrote: > On Fri Jun 30, 2023 at 12:50 PM CEST, Kent Gibson wrote: > > On Fri, Jun 30, 2023 at 12:46:11PM +0200, Erik Schilling wrote: > > > On Fri Jun 30, 2023 at 12:19 PM CEST, Kent Gibson wrote: > > > > > > > > > > > > > That is the only test failing out of the whole suite, so gpiosim is not > > > > the problem. > > > > > > > > That is with the latest from the Raspberry Pi rpi-6.4.y branch[1] > > > > (a867309b7a55 so a few days old now), with CONFIG_GPIO_SIM so that I can > > > > run the tests. > > > > Happens to be running on a Pi ZeroW, but I don't think that test is speed > > > > sensitive. I have done a complete rebuild - same result. > > > > > > > > Are there any distos enabling GPIO_SIM yet? > > > > > > Fedora does now (after I asked for it [2]). But it does not support any > > > 32-bit ARM targets anymore :/. Can you try reproducing it without the > > > patches? I would be surprised if this was related to the patches. > > > > > > > Tried that - same result with libgpiod master. > > So it is not from your patches. > > Thanks for checking! Will try to reproduce it in the next weeks. > Seems there was something broken in my build. I did a clean slate rebuild and now all tests are passing. Cheers, Kent.
This follows up on my promise on Kent's series [1] to look into whether these casts are needed or not. Most are not, a few are false-positives. I also explored some shunit2 based test-script to automate the testing, but that became ugly with linking issue and needs me to revisit it another time. So this only sends the fixes for now. [1] https://lore.kernel.org/r/20230612154055.56556-1-warthog618@gmail.com To: Linux-GPIO <linux-gpio@vger.kernel.org> Cc: Kent Gibson <warthog618@gmail.com> Cc: Viresh Kumar <viresh.kumar@linaro.org> Signed-off-by: Erik Schilling <erik.schilling@linaro.org> --- Erik Schilling (4): bindings: rust: clippy: drop unnecessary casts bindings: rust: clippy: silence false-positives on casts bindings: rust: clippy: drop unneeded conversions bindings: rust: clippy: silence false-positive on iterator bindings/rust/gpiosim-sys/src/lib.rs | 2 +- bindings/rust/gpiosim-sys/src/sim.rs | 4 ++-- bindings/rust/libgpiod/src/chip.rs | 2 +- bindings/rust/libgpiod/src/edge_event.rs | 2 +- bindings/rust/libgpiod/src/event_buffer.rs | 7 +++++-- bindings/rust/libgpiod/src/info_event.rs | 2 +- bindings/rust/libgpiod/src/lib.rs | 2 +- bindings/rust/libgpiod/src/line_config.rs | 4 ++-- bindings/rust/libgpiod/src/line_info.rs | 3 +++ bindings/rust/libgpiod/src/line_request.rs | 8 ++++---- bindings/rust/libgpiod/src/line_settings.rs | 5 ++++- bindings/rust/libgpiod/src/request_config.rs | 2 +- bindings/rust/libgpiod/tests/chip.rs | 2 +- 13 files changed, 27 insertions(+), 18 deletions(-) --- base-commit: 4510231c95a087f58a155cf74164e403e1e0584f change-id: 20230629-clippy-890c541c6d09 Best regards,