linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kent Gibson <warthog618@gmail.com>
To: Erik Schilling <erik.schilling@linaro.org>
Cc: linux-gpio@vger.kernel.org, brgl@bgdev.pl
Subject: Re: [libgpiod][PATCH] bindings: rust: fix clippy lint warnings
Date: Wed, 14 Jun 2023 17:06:29 +0800	[thread overview]
Message-ID: <ZImDFS2ATTxeFxDK@sol> (raw)
In-Reply-To: <CTC8M1AAQDLI.DNPMW5PQHFNK@fedora>

On Wed, Jun 14, 2023 at 10:40:56AM +0200, Erik Schilling wrote:
> On Wed Jun 14, 2023 at 10:29 AM CEST, Kent Gibson wrote:
> > On Wed, Jun 14, 2023 at 10:14:08AM +0200, Erik Schilling wrote:
> > > On Mon Jun 12, 2023 at 5:40 PM CEST, Kent Gibson wrote:
> > > > clippy from Rust 1.70 reports a host of warnings due to casting and type
> > > > conversions across the FFI interface to libgpiod.
> > > > These casts and conversions are required to support old versions of Rust
> > > > that do not support recent Rust FFI extensions.
> > > 
> > > Could you elaborate which extensions are relevant here? Would it be
> > > realistic to just update the minimum Rust version instead of needing
> > > to include these suppression directives?
> > > 
> >
> > Types were added in core::ffi[1] in 1.64 for just this purpose.
> > e.g. c_uint[2]
> > Though c_size_t[3] still remains in Experimental.
> >
> > And I guess the clippy lints followed soon after.
> >
> > Wrt setting the MSRV, but I assumed not, hence the allows.
> 
> For me bindgen seems to generate usize of size_t, thats why I asked.
> Does that depend on the Rust version somehow? Or more concretely:
> When will things like `gpiod_line_config_get_num_configured_offsets`
> not get translated to `usize` so that we need a cast?
> 

No idea - outside my area.

> On my end (with latest toolchain and nightly), I do not see any
> clippy warnings with `cargo clippy`. How exactly did you produce those
> warnings?
> 

Interesting.  With stable on libgpiod master in the rust/libgpiod
directory, and with these in my environment:

export SYSTEM_DEPS_LIBGPIOD_NO_PKG_CONFIG=1
export SYSTEM_DEPS_LIBGPIOD_SEARCH_NATIVE="${PWD}/../../../lib/.libs/"
export SYSTEM_DEPS_LIBGPIOD_LIB=gpiod
export SYSTEM_DEPS_LIBGPIOD_INCLUDE="${PWD}/../../../include/"

I get:

$ cargo clippy --tests
warning: casting to the same type is unnecessary (`u32` -> `u32`)
  --> libgpiod/src/info_event.rs:37:29
   |
37 |         InfoChangeKind::new(unsafe { gpiod::gpiod_info_event_get_event_type(self.event) } as u32)
   |                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unsafe { gpiod::gpiod_info_event_get_event_type(self.event) }`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast
   = note: `#[warn(clippy::unnecessary_cast)]` on by default

warning: casting to the same type is unnecessary (`usize` -> `usize`)
   --> libgpiod/src/chip.rs:282:18
    |
282 |         unsafe { gpiod::gpiod_chip_info_get_num_lines(self.info) as usize }
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `gpiod::gpiod_chip_info_get_num_lines(self.info)`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast

warning: casting to the same type is unnecessary (`u32` -> `u32`)
  --> libgpiod/src/edge_event.rs:44:23
   |
44 |         EdgeKind::new(unsafe { gpiod::gpiod_edge_event_get_event_type(self.0) } as u32)
   |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unsafe { gpiod::gpiod_edge_event_get_event_type(self.0) }`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast

warning: called `.nth(0)` on a `std::iter::Iterator`, when `.next()` is equivalent
  --> libgpiod/src/event_buffer.rs:57:9
   |
57 |         self.nth(0)
   |         ^^^^^^^^^^^ help: try calling `.next()` instead of `.nth(0)`: `self.next()`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#iter_nth_zero
   = note: `#[warn(clippy::iter_nth_zero)]` on by default

warning: casting to the same type is unnecessary (`usize` -> `usize`)
  --> libgpiod/src/event_buffer.rs:85:33
   |
85 |         let capacity = unsafe { gpiod::gpiod_edge_event_buffer_get_capacity(buffer) as usize };
   |                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `gpiod::gpiod_edge_event_buffer_get_capacity(buffer)`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast

warning: useless conversion to the same type: `usize`
   --> libgpiod/src/event_buffer.rs:111:17
    |
111 |                 self.events.len().try_into().unwrap(),
    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider removing `.try_into()`
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#useless_conversion
    = note: `#[warn(clippy::useless_conversion)]` on by default

warning: casting to the same type is unnecessary (`usize` -> `usize`)
  --> libgpiod/src/line_request.rs:31:18
   |
31 |         unsafe { gpiod::gpiod_line_request_get_num_requested_lines(self.request) as usize }
   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `gpiod::gpiod_line_request_get_num_requested_lines(self.request)`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast

warning: casting to the same type is unnecessary (`usize` -> `usize`)
  --> libgpiod/src/line_request.rs:36:35
   |
36 |         let mut offsets = vec![0; self.num_lines() as usize];
   |                                   ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `self.num_lines()`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast

warning: casting to the same type is unnecessary (`usize` -> `usize`)
  --> libgpiod/src/line_request.rs:46:27
   |
46 |         offsets.shrink_to(num_offsets as usize);
   |                           ^^^^^^^^^^^^^^^^^^^^ help: try: `num_offsets`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast

warning: casting to the same type is unnecessary (`usize` -> `usize`)
   --> libgpiod/src/line_request.rs:148:28
    |
148 |         if values.len() != self.num_lines() as usize {
    |                            ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `self.num_lines()`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast

warning: casting to the same type is unnecessary (`usize` -> `usize`)
  --> libgpiod/src/request_config.rs:86:18
   |
86 |         unsafe { gpiod::gpiod_request_config_get_event_buffer_size(self.config) as usize }
   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `gpiod::gpiod_request_config_get_event_buffer_size(self.config)`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast

warning: casting to the same type is unnecessary (`usize` -> `usize`)
   --> libgpiod/src/line_config.rs:111:35
    |
111 |         let mut offsets = vec![0; num_lines as usize];
    |                                   ^^^^^^^^^^^^^^^^^^ help: try: `num_lines`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast

warning: casting to the same type is unnecessary (`usize` -> `usize`)
   --> libgpiod/src/line_config.rs:122:35
    |
122 |         for offset in &offsets[0..num_stored as usize] {
    |                                   ^^^^^^^^^^^^^^^^^^^ help: try: `num_stored`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast

warning: casting to the same type is unnecessary (`u64` -> `u64`)
   --> libgpiod/src/line_info.rs:147:13
    |
147 |             gpiod::gpiod_line_info_get_debounce_period_us(self.info) as u64
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `gpiod::gpiod_line_info_get_debounce_period_us(self.info)`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast

warning: casting to the same type is unnecessary (`u64` -> `u64`)
   --> libgpiod/src/line_settings.rs:223:13
    |
223 |             gpiod::gpiod_line_settings_get_debounce_period_us(self.settings) as u64
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `gpiod::gpiod_line_settings_get_debounce_period_us(self.settings)`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast

warning: casting to the same type is unnecessary (`u32` -> `u32`)
   --> libgpiod/src/line_settings.rs:247:25
    |
247 |         EventClock::new(unsafe { gpiod::gpiod_line_settings_get_event_clock(self.settings) } as u32)
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unsafe { gpiod::gpiod_line_settings_get_event_clock(self.settings) }`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast

warning: casting to the same type is unnecessary (`i32` -> `i32`)
   --> libgpiod/src/lib.rs:196:66
    |
196 |                 _ => return Err(Error::InvalidEnumValue("Value", val as i32)),
    |                                                                  ^^^^^^^^^^ help: try: `val`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast

warning: `libgpiod` (lib) generated 17 warnings (run `cargo clippy --fix --lib -p libgpiod` to apply 16 suggestions)
warning: casting to the same type is unnecessary (`i32` -> `i32`)
   --> gpiosim-sys/src/sim.rs:167:24
    |
167 |             Value::new(ret as i32)
    |                        ^^^^^^^^^^ help: try: `ret`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast
    = note: `#[warn(clippy::unnecessary_cast)]` on by default

warning: useless conversion to the same type: `usize`
   --> gpiosim-sys/src/sim.rs:189:66
    |
189 |         let ret = unsafe { gpiosim_bank_set_num_lines(self.bank, num.try_into().unwrap()) };
    |                                                                  ^^^^^^^^^^^^^^
    |
    = help: consider removing `.try_into()`
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#useless_conversion
    = note: `#[warn(clippy::useless_conversion)]` on by default

warning: casting to the same type is unnecessary (`i32` -> `i32`)
  --> gpiosim-sys/src/lib.rs:49:62
   |
49 |             _ => return Err(Error::InvalidEnumValue("Value", val as i32)),
   |                                                              ^^^^^^^^^^ help: try: `val`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast

warning: `gpiosim-sys` (lib) generated 3 warnings (run `cargo clippy --fix --lib -p gpiosim-sys` to apply 2 suggestions)
warning: casting to the same type is unnecessary (`usize` -> `usize`)
  --> libgpiod/tests/chip.rs:62:42
   |
62 |             assert_eq!(info.num_lines(), NGPIO as usize);
   |                                          ^^^^^^^^^^^^^^ help: try: `{ NGPIO }`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast
   = note: `#[warn(clippy::unnecessary_cast)]` on by default

warning: `libgpiod` (lib test) generated 17 warnings (17 duplicates)
warning: `libgpiod` (test "chip") generated 1 warning (run `cargo clippy --fix --test "chip"` to apply 1 suggestion)
    Finished dev [unoptimized + debuginfo] target(s) in 0.03s


where stable is:
stable-x86_64-unknown-linux-gnu unchanged - rustc 1.70.0 (90c541806 2023-05-31)

I get the same from nightly.
nightly-x86_64-unknown-linux-gnu unchanged - rustc 1.72.0-nightly (dd5d7c729 2023-06-02)

The --tests is to extend the check to the tests - you get the vast
majority of those without it.

In both cases the patched version is clean.

Cheers,
Kent.

  reply	other threads:[~2023-06-14  9:06 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-12 15:40 [libgpiod][PATCH] bindings: rust: fix clippy lint warnings Kent Gibson
2023-06-14  8:14 ` Erik Schilling
2023-06-14  8:29   ` Kent Gibson
2023-06-14  8:40     ` Erik Schilling
2023-06-14  9:06       ` Kent Gibson [this message]
2023-06-14  9:16         ` Erik Schilling
2023-06-19  7:36     ` Erik Schilling
2023-06-19  7:49       ` Erik Schilling
2023-06-19  7:57       ` Kent Gibson
2023-06-19  8:13         ` Erik Schilling
2023-06-19  8:33           ` Kent Gibson
2023-06-19  8:50           ` Viresh Kumar
2023-06-19  8:59             ` Erik Schilling

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZImDFS2ATTxeFxDK@sol \
    --to=warthog618@gmail.com \
    --cc=brgl@bgdev.pl \
    --cc=erik.schilling@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).