All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.