Re: [libgpiod][PATCH] bindings: rust: fix clippy lint warnings

["Erik Schilling" <erik.schilling@xxxxxxxxxx>]

On Mon Jun 19, 2023 at 9:57 AM CEST, Kent Gibson wrote:
> On Mon, Jun 19, 2023 at 09:36:48AM +0200, Erik Schilling wrote:
> > Sorry, got distracted while sorting out the MSRV mess that I sent a
> > separate patch for [4].
> > 
> > I do not think that this is the reason why we need the casts...
> > bindgen generates bindings using std::os::raw::c_uint [5] which is
> > stable since 1.1.0 (and was previously defined as u32 [6]). I think we
> > can just drop the casts entirely? I can run cargo clippy --fix on latest
> > stable (1.70.0), then go back to 1.60 and everything is still building.
> > I am having trouble to execute the tests in that version due to some
> > linkage errors, but that should not be the fault of the casts.
> > 
> > Did I got this correct or am I misunderstanding your reasoning?
> > 
>
> My reasoning was simply that building the bindings as you suggested
> resulted in lint warnings, which is noisy and iritating when trying to
> lint my own code.

I fully agree that we should fix them! I was just confused about the
explanation.

> But I'm just the messenger.  Your question would be better directed at
> Viresh - it is his code so he should be able to tell you why the casts
> are there.
> IIRC we needed the casts historically, though I don't recall the rust
> version we were using at the time.
> If we've moved beyond that then I have no problem with the casts being
> removedi, in fact in my initial comment I lamented the fact they were
> necessary.

ACK. Not sure how the situation was when the patches came together, but
today I would suggest to just drop the casts. Do you want to respin your
series or shall I send the fixes?

> > Note: One needs to fix a bug that cargo clippy --fix introduces since
> > it replaces nth(0) with next() in event_buffers.rs and introduces a
> > unconditional recursion.
> > 
>
> Who is using --fix??

I do all the time (and spend the time I saved typing on reviewing)!
(Thought mostly I use the suggested actions suggested by the LSP in
the editor instead of the CLI tools)

> I did put an allow in there for that one, with a comment about the
> recursion, though I'm not sure the comment is sufficiently clear without
> the warning in front of you - and you no longer get that with the allow
> in place.

Ah. Did not notice that... My brain was only thinking about the casts.

- Erik