[PATCH libgpiod 0/4] bindings: rust: clippy: fixed warnings

[PATCH libgpiod 0/4] bindings: rust: clippy: fixed warnings

[Erik Schilling]

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,
-- 
Erik Schilling <erik.schilling@linaro.org>

[PATCH libgpiod 1/4] bindings: rust: clippy: drop unnecessary casts

[Erik Schilling]

Ran cargo clippy --fix on clippy 0.1.70 (90c5418 2023-05-31).

Tested build on x86_64, armv7hf, aarch64.

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/gpiosim-sys/src/lib.rs         | 2 +-
 bindings/rust/gpiosim-sys/src/sim.rs         | 2 +-
 bindings/rust/libgpiod/src/chip.rs           | 2 +-
 bindings/rust/libgpiod/src/edge_event.rs     | 2 +-
 bindings/rust/libgpiod/src/event_buffer.rs   | 2 +-
 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_request.rs   | 8 ++++----
 bindings/rust/libgpiod/src/line_settings.rs  | 2 +-
 bindings/rust/libgpiod/src/request_config.rs | 2 +-
 bindings/rust/libgpiod/tests/chip.rs         | 2 +-
 12 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/bindings/rust/gpiosim-sys/src/lib.rs b/bindings/rust/gpiosim-sys/src/lib.rs
index eed2a42..bf9ae32 100644
--- a/bindings/rust/gpiosim-sys/src/lib.rs
+++ b/bindings/rust/gpiosim-sys/src/lib.rs
@@ -46,7 +46,7 @@ impl Value {
                     errno::errno(),
                 ))
             }
-            _ => return Err(Error::InvalidEnumValue("Value", val as i32)),
+            _ => return Err(Error::InvalidEnumValue("Value", val)),
         })
     }
 }
diff --git a/bindings/rust/gpiosim-sys/src/sim.rs b/bindings/rust/gpiosim-sys/src/sim.rs
index 896596f..16c2b3e 100644
--- a/bindings/rust/gpiosim-sys/src/sim.rs
+++ b/bindings/rust/gpiosim-sys/src/sim.rs
@@ -164,7 +164,7 @@ impl SimBank {
                 errno::errno(),
             ))
         } else {
-            Value::new(ret as i32)
+            Value::new(ret)
         }
     }
 
diff --git a/bindings/rust/libgpiod/src/chip.rs b/bindings/rust/libgpiod/src/chip.rs
index f4de008..81e1be6 100644
--- a/bindings/rust/libgpiod/src/chip.rs
+++ b/bindings/rust/libgpiod/src/chip.rs
@@ -279,7 +279,7 @@ impl Info {
     /// Get the number of GPIO lines exposed by the chip.
     pub fn num_lines(&self) -> usize {
         // SAFETY: `gpiod_chip` is guaranteed to be valid here.
-        unsafe { gpiod::gpiod_chip_info_get_num_lines(self.info) as usize }
+        unsafe { gpiod::gpiod_chip_info_get_num_lines(self.info) }
     }
 }
 
diff --git a/bindings/rust/libgpiod/src/edge_event.rs b/bindings/rust/libgpiod/src/edge_event.rs
index d324ce6..0c0cfbc 100644
--- a/bindings/rust/libgpiod/src/edge_event.rs
+++ b/bindings/rust/libgpiod/src/edge_event.rs
@@ -41,7 +41,7 @@ impl Event {
     /// Get the event type.
     pub fn event_type(&self) -> Result<EdgeKind> {
         // SAFETY: `gpiod_edge_event` is guaranteed to be valid here.
-        EdgeKind::new(unsafe { gpiod::gpiod_edge_event_get_event_type(self.0) } as u32)
+        EdgeKind::new(unsafe { gpiod::gpiod_edge_event_get_event_type(self.0) })
     }
 
     /// Get the timestamp of the event.
diff --git a/bindings/rust/libgpiod/src/event_buffer.rs b/bindings/rust/libgpiod/src/event_buffer.rs
index 1deaf2b..520eb2a 100644
--- a/bindings/rust/libgpiod/src/event_buffer.rs
+++ b/bindings/rust/libgpiod/src/event_buffer.rs
@@ -82,7 +82,7 @@ impl Buffer {
         }
 
         // SAFETY: `gpiod_edge_event_buffer` is guaranteed to be valid here.
-        let capacity = unsafe { gpiod::gpiod_edge_event_buffer_get_capacity(buffer) as usize };
+        let capacity = unsafe { gpiod::gpiod_edge_event_buffer_get_capacity(buffer) };
 
         Ok(Self {
             buffer,
diff --git a/bindings/rust/libgpiod/src/info_event.rs b/bindings/rust/libgpiod/src/info_event.rs
index b0ceb3b..db60600 100644
--- a/bindings/rust/libgpiod/src/info_event.rs
+++ b/bindings/rust/libgpiod/src/info_event.rs
@@ -34,7 +34,7 @@ impl Event {
     /// Get the event type of the status change event.
     pub fn event_type(&self) -> Result<InfoChangeKind> {
         // SAFETY: `gpiod_info_event` is guaranteed to be valid here.
-        InfoChangeKind::new(unsafe { gpiod::gpiod_info_event_get_event_type(self.event) } as u32)
+        InfoChangeKind::new(unsafe { gpiod::gpiod_info_event_get_event_type(self.event) })
     }
 
     /// Get the timestamp of the event, read from the monotonic clock.
diff --git a/bindings/rust/libgpiod/src/lib.rs b/bindings/rust/libgpiod/src/lib.rs
index 26354e5..3acc98c 100644
--- a/bindings/rust/libgpiod/src/lib.rs
+++ b/bindings/rust/libgpiod/src/lib.rs
@@ -193,7 +193,7 @@ pub mod line {
                         errno::errno(),
                     ))
                 }
-                _ => return Err(Error::InvalidEnumValue("Value", val as i32)),
+                _ => return Err(Error::InvalidEnumValue("Value", val)),
             })
         }
 
diff --git a/bindings/rust/libgpiod/src/line_config.rs b/bindings/rust/libgpiod/src/line_config.rs
index e973cde..f4f03f1 100644
--- a/bindings/rust/libgpiod/src/line_config.rs
+++ b/bindings/rust/libgpiod/src/line_config.rs
@@ -108,7 +108,7 @@ impl Config {
         let mut map = SettingsMap::new();
         // SAFETY: gpiod_line_config is guaranteed to be valid here
         let num_lines = unsafe { gpiod::gpiod_line_config_get_num_configured_offsets(self.config) };
-        let mut offsets = vec![0; num_lines as usize];
+        let mut offsets = vec![0; num_lines];
 
         // SAFETY: gpiod_line_config is guaranteed to be valid here.
         let num_stored = unsafe {
@@ -119,7 +119,7 @@ impl Config {
             )
         };
 
-        for offset in &offsets[0..num_stored as usize] {
+        for offset in &offsets[0..num_stored] {
             // SAFETY: `gpiod_line_config` is guaranteed to be valid here.
             let settings =
                 unsafe { gpiod::gpiod_line_config_get_line_settings(self.config, *offset) };
diff --git a/bindings/rust/libgpiod/src/line_request.rs b/bindings/rust/libgpiod/src/line_request.rs
index b175eea..1140aa9 100644
--- a/bindings/rust/libgpiod/src/line_request.rs
+++ b/bindings/rust/libgpiod/src/line_request.rs
@@ -28,12 +28,12 @@ impl Request {
     /// Get the number of lines in the request.
     pub fn num_lines(&self) -> usize {
         // SAFETY: `gpiod_line_request` is guaranteed to be valid here.
-        unsafe { gpiod::gpiod_line_request_get_num_requested_lines(self.request) as usize }
+        unsafe { gpiod::gpiod_line_request_get_num_requested_lines(self.request) }
     }
 
     /// Get the offsets of lines in the request.
     pub fn offsets(&self) -> Vec<Offset> {
-        let mut offsets = vec![0; self.num_lines() as usize];
+        let mut offsets = vec![0; self.num_lines()];
 
         // SAFETY: `gpiod_line_request` is guaranteed to be valid here.
         let num_offsets = unsafe {
@@ -43,7 +43,7 @@ impl Request {
                 self.num_lines(),
             )
         };
-        offsets.shrink_to(num_offsets as usize);
+        offsets.shrink_to(num_offsets);
         offsets
     }
 
@@ -145,7 +145,7 @@ impl Request {
 
     /// Set values of all lines associated with the request.
     pub fn set_values(&mut self, values: &[Value]) -> Result<&mut Self> {
-        if values.len() != self.num_lines() as usize {
+        if values.len() != self.num_lines() {
             return Err(Error::InvalidArguments);
         }
 
diff --git a/bindings/rust/libgpiod/src/line_settings.rs b/bindings/rust/libgpiod/src/line_settings.rs
index 918d6c2..79ee2f5 100644
--- a/bindings/rust/libgpiod/src/line_settings.rs
+++ b/bindings/rust/libgpiod/src/line_settings.rs
@@ -244,7 +244,7 @@ impl Settings {
     /// Get the event clock setting.
     pub fn event_clock(&self) -> Result<EventClock> {
         // SAFETY: `gpiod_line_settings` is guaranteed to be valid here.
-        EventClock::new(unsafe { gpiod::gpiod_line_settings_get_event_clock(self.settings) } as u32)
+        EventClock::new(unsafe { gpiod::gpiod_line_settings_get_event_clock(self.settings) })
     }
 
     /// Set the output value setting.
diff --git a/bindings/rust/libgpiod/src/request_config.rs b/bindings/rust/libgpiod/src/request_config.rs
index 0c6c5c1..5bde7c6 100644
--- a/bindings/rust/libgpiod/src/request_config.rs
+++ b/bindings/rust/libgpiod/src/request_config.rs
@@ -83,7 +83,7 @@ impl Config {
     /// Get the edge event buffer size setting for the request config.
     pub fn event_buffer_size(&self) -> usize {
         // SAFETY: `gpiod_request_config` is guaranteed to be valid here.
-        unsafe { gpiod::gpiod_request_config_get_event_buffer_size(self.config) as usize }
+        unsafe { gpiod::gpiod_request_config_get_event_buffer_size(self.config) }
     }
 }
 
diff --git a/bindings/rust/libgpiod/tests/chip.rs b/bindings/rust/libgpiod/tests/chip.rs
index f264708..60b4ecc 100644
--- a/bindings/rust/libgpiod/tests/chip.rs
+++ b/bindings/rust/libgpiod/tests/chip.rs
@@ -59,7 +59,7 @@ mod chip {
             assert_eq!(info.label().unwrap(), LABEL);
             assert_eq!(info.name().unwrap(), sim.chip_name());
             assert_eq!(chip.path().unwrap(), sim.dev_path().to_str().unwrap());
-            assert_eq!(info.num_lines(), NGPIO as usize);
+            assert_eq!(info.num_lines(), NGPIO);
         }
 
         #[test]

-- 
2.40.1

[PATCH libgpiod 2/4] bindings: rust: clippy: silence false-positives on casts

[Erik Schilling]

Tested build on x86_64, armv7hf, aarch64.

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/line_info.rs     | 3 +++
 bindings/rust/libgpiod/src/line_settings.rs | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/bindings/rust/libgpiod/src/line_info.rs b/bindings/rust/libgpiod/src/line_info.rs
index 702f1b4..c4f488c 100644
--- a/bindings/rust/libgpiod/src/line_info.rs
+++ b/bindings/rust/libgpiod/src/line_info.rs
@@ -142,6 +142,9 @@ impl Info {
 
     /// Get the debounce period of the line.
     pub fn debounce_period(&self) -> Duration {
+        // c_ulong may be 32bit OR 64bit, clippy reports a false-positive here:
+        // https://github.com/rust-lang/rust-clippy/issues/10555
+        #[allow(clippy::unnecessary_cast)]
         // SAFETY: `gpiod_line_info` is guaranteed to be valid here.
         Duration::from_micros(unsafe {
             gpiod::gpiod_line_info_get_debounce_period_us(self.info) as u64
diff --git a/bindings/rust/libgpiod/src/line_settings.rs b/bindings/rust/libgpiod/src/line_settings.rs
index 79ee2f5..f0b3e9c 100644
--- a/bindings/rust/libgpiod/src/line_settings.rs
+++ b/bindings/rust/libgpiod/src/line_settings.rs
@@ -218,6 +218,9 @@ impl Settings {
 
     /// Get the debounce period.
     pub fn debounce_period(&self) -> Result<Duration> {
+        // c_ulong may be 32bit OR 64bit, clippy reports a false-positive here:
+        // https://github.com/rust-lang/rust-clippy/issues/10555
+        #[allow(clippy::unnecessary_cast)]
         // SAFETY: `gpiod_line_settings` is guaranteed to be valid here.
         Ok(Duration::from_micros(unsafe {
             gpiod::gpiod_line_settings_get_debounce_period_us(self.settings) as u64

-- 
2.40.1

[PATCH libgpiod 3/4] bindings: rust: clippy: drop unneeded conversions

[Erik Schilling]

Ran cargo clippy --fix on clippy 0.1.70 (90c5418 2023-05-31).

Tested build on x86_64, armv7hf, aarch64.

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/gpiosim-sys/src/sim.rs       | 2 +-
 bindings/rust/libgpiod/src/event_buffer.rs | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/bindings/rust/gpiosim-sys/src/sim.rs b/bindings/rust/gpiosim-sys/src/sim.rs
index 16c2b3e..71b9453 100644
--- a/bindings/rust/gpiosim-sys/src/sim.rs
+++ b/bindings/rust/gpiosim-sys/src/sim.rs
@@ -186,7 +186,7 @@ impl SimBank {
 
     fn set_num_lines(&self, num: usize) -> Result<()> {
         // SAFETY: `gpiosim_bank` is guaranteed to be valid here.
-        let ret = unsafe { gpiosim_bank_set_num_lines(self.bank, num.try_into().unwrap()) };
+        let ret = unsafe { gpiosim_bank_set_num_lines(self.bank, num) };
         if ret == -1 {
             Err(Error::OperationFailed(
                 OperationType::SimBankSetNumLines,
diff --git a/bindings/rust/libgpiod/src/event_buffer.rs b/bindings/rust/libgpiod/src/event_buffer.rs
index 520eb2a..b79e9ea 100644
--- a/bindings/rust/libgpiod/src/event_buffer.rs
+++ b/bindings/rust/libgpiod/src/event_buffer.rs
@@ -108,7 +108,7 @@ impl Buffer {
             gpiod::gpiod_line_request_read_edge_events(
                 request.request,
                 self.buffer,
-                self.events.len().try_into().unwrap(),
+                self.events.len(),
             )
         };
 

-- 
2.40.1

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

[Erik Schilling]

This was fixed, but it is not in stable yet.

Tested build on x86_64, armv7hf, aarch64.

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)
     }
 }

-- 
2.40.1

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

[Kent Gibson]

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.

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

[Erik Schilling]

On Fri Jun 30, 2023 at 11:08 AM CEST, Kent Gibson wrote:
>
> For future reference, the subject line should be "[libgpiod][PATCH...",
> as per the README.
> Makes it easier to filter visually, if nothing else.

I am using b4 to send the patches (otherwise I would screw up this stuff
even more) which always merges the prefixes together. Raised the point
there:

https://lore.kernel.org/r/CTPW75Q2ISUN.251ELTNP6RB22@fedora

> 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.

Will rephrase it in next version.

> 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?

This is a bit complex... Originally I intended to send this along some
shunit2 script that checks build tests and clippy lints. But that became
a bit more involving than I hoped... By defauly we link gpiosim into the
tests statically and the static lib is not built with PIC by default.

Rust, however, by default wants to build the tests with PIE. For some
reason that works on Debian, but not on Fedora. It also broke down on
Debian on armv7hf. So I was not able to come up with a configuration
set that worked perfectly in all situations (apart from building the
C lib with PIC) and decided not to send scripts to automate the check
(for now).

After wasting way too much time on this, I then realized that I was
missing the GPIO_SIM module and skipped the tests for armv7hf since I
dropped the check script from this series anyway.

TLDR: I tested build AND cargo test on x86_64 and aarch64, but only the
build with armv7hf (I built test and examples though).

> 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.

Right... Since I dropped my script idea how now I can move this to the
cover, will expand what I tested in v2.

> > 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".

I cannot tell the version of the next release that will include it since
it is not released yet, but I can state the clippy version that caused
the error.

> 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)

Thanks for testing!

> (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?

- Erik

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

[Kent Gibson]

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

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

[Erik Schilling]

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

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

[Kent Gibson]

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

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

[Erik Schilling]

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.

- Erik

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

[Kent Gibson]

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.

Re: [PATCH libgpiod 0/4] bindings: rust: clippy: fixed warnings

[Viresh Kumar]

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>

-- 
viresh