linux-kernel - Re: [PATCH v2 1/1] rust: simplify `Adapter::id

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2026011703-bony-unloader-9099@gregkh>
Date: Sat, 17 Jan 2026 13:07:54 +0100
From: Greg KH <gregkh@...uxfoundation.org>
To: Onur Özkan <work@...rozkan.dev>
Cc: rust-for-linux@...r.kernel.org, rafael@...nel.org, dakr@...nel.org,
	ojeda@...nel.org, boqun.feng@...il.com, gary@...yguo.net,
	bjorn3_gh@...tonmail.com, lossin@...nel.org, a.hindborg@...nel.org,
	aliceryhl@...gle.com, tmgross@...ch.edu,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/1] rust: simplify `Adapter::id_info`

On Sat, Jan 17, 2026 at 02:02:40PM +0300, Onur Özkan wrote:
> On Sat, 17 Jan 2026 11:00:11 +0100
> Greg KH <gregkh@...uxfoundation.org> wrote:
> 
> > On Sat, Jan 17, 2026 at 12:47:10PM +0300, Onur Özkan wrote:
> > > id_info() checks ACPI first and falls back to OF.
> > > 
> > > This replaces the unnecessarily verbose approach with a
> > > simple or_else() chain and drops temporary variables.
> > > 
> > > No functional change intended.
> > > 
> > > Signed-off-by: Onur Özkan <work@...rozkan.dev>
> > > ---
> > >  rust/kernel/driver.rs | 12 +-----------
> > >  1 file changed, 1 insertion(+), 11 deletions(-)
> > > 
> > > diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs
> > > index 649d06468f41..6cef792d54e4 100644
> > > --- a/rust/kernel/driver.rs
> > > +++ b/rust/kernel/driver.rs
> > > @@ -307,16 +307,6 @@ fn of_id_info(dev: &device::Device) ->
> > > Option<&'static Self::IdInfo> { /// If this returns `None`, it
> > > means that there is no match in any of the ID tables directly ///
> > > associated with a [`device::Device`]. fn id_info(dev:
> > > &device::Device) -> Option<&'static Self::IdInfo> {
> > > -        let id = Self::acpi_id_info(dev);
> > > -        if id.is_some() {
> > > -            return id;
> > > -        }
> > > -
> > > -        let id = Self::of_id_info(dev);
> > > -        if id.is_some() {
> > > -            return id;
> > > -        }
> > > -
> > > -        None
> > > +        Self::acpi_id_info(dev).or_else(|| Self::of_id_info(dev))
> > 
> > Have we already started the game of "rust golf" on the kernel?  The
> > original code here is much easier to read, and the compiler should
> > produce the same thing for both, right?
> > 
> > thanks,
> > 
> > greg k-h
> 
> Hi Greg,
> 
> I don't know what "rust golf" means, the main motivation here was to use
> a more idiomatic Rust pattern.

I was referring to this:
	https://en.wikipedia.org/wiki/Code_golf
sorry for not being explicit :)

> Functions called in Adapter::id_info already return the same Result
> type and it's quite annoying(IMO) to do "if x.is_some() { return x }"
> in cases like this. Clippy would argue on many cases similar to this.
> Readability is subjective, but to me the new code is slightly simpler
> to follow (as it does the "try X, then Y" thing in a shorter way).

Does clippy complain about this one?

> That said, I don't have strong feelings on the new code. The main
> motivation was to drop the "annoying" code with more idiomatic one,
> but seems like not everyone agrees with it. Feel free to ignore the
> patch if you prefer the original code.

I don't have strong feelings either, but the original is "easier" for
those of us used to C code.

thanks,

greg k-h