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: <20260117155631.5a65b3c6@nimda>
Date: Sat, 17 Jan 2026 15:56:31 +0300
From: Onur Özkan <work@...rozkan.dev>
To: Greg KH <gregkh@...uxfoundation.org>
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, 17 Jan 2026 13:07:54 +0100
Greg KH <gregkh@...uxfoundation.org> wrote:

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

I didn't know this term before - good to know :)

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

No, it doesn't.

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

Fair point, you are right.

Thanks,
Onur Özkan

> thanks,
> 
> greg k-h