linux-kernel - Re: [PATCH v9 2/3] rust: leds: split generic and normal led classdev abstractions up

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] [day] [month] [year] [list]

Message-ID: <501ad13182eb1c18f671343a34490deb043c2643.camel@posteo.de>
Date: Thu, 20 Nov 2025 13:21:48 +0000
From: Markus Probst <markus.probst@...teo.de>
To: Alice Ryhl <aliceryhl@...gle.com>
Cc: Lee Jones <lee@...nel.org>, Pavel Machek <pavel@...nel.org>, Greg
 Kroah-Hartman <gregkh@...uxfoundation.org>, Dave Ertman
 <david.m.ertman@...el.com>, Ira Weiny	 <ira.weiny@...el.com>, Leon
 Romanovsky <leon@...nel.org>, Miguel Ojeda	 <ojeda@...nel.org>, Alex Gaynor
 <alex.gaynor@...il.com>, Boqun Feng	 <boqun.feng@...il.com>, Gary Guo
 <gary@...yguo.net>,  Björn Roy Baron	
 <bjorn3_gh@...tonmail.com>, Benno Lossin <lossin@...nel.org>, Andreas
 Hindborg	 <a.hindborg@...nel.org>, Trevor Gross <tmgross@...ch.edu>, Danilo
 Krummrich	 <dakr@...nel.org>, "Rafael J. Wysocki" <rafael@...nel.org>,
 Bjorn Helgaas	 <bhelgaas@...gle.com>, Krzysztof Wilczyński
	 <kwilczynski@...nel.org>, rust-for-linux@...r.kernel.org, 
	linux-leds@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-pci@...r.kernel.org
Subject: Re: [PATCH v9 2/3] rust: leds: split generic and normal led
 classdev abstractions up

On Thu, 2025-11-20 at 10:54 +0000, Alice Ryhl wrote:
> On Wed, Nov 19, 2025 at 02:11:24PM +0000, Markus Probst wrote:
> > Move code specific to normal led class devices into a separate file and
> > introduce the `led::Mode` trait to allow for other types of led class
> > devices.
> > 
> > Signed-off-by: Markus Probst <markus.probst@...teo.de>
> 
> So it seems like the goal of this trait is to support both normal led
> and multicolor led under the same code. However, it seems the traits
> involved with this are pretty complex.
> 
> My primary feedback here is: please consider if we can avoid these
> complex traits. How much duplication would it really take to just have
> two Device structs and two LedOps traits? I think a few pieces of
> duplication would be far better than what this patch does.
I counted over 221 lines of code for each classdev type (not counting
LedOps). If we want to support `led_classdev_flash` later down the line
(if a Rust driver would need it), that would be over 663 lines of code.

It doesn't look too complex to me, but thats maybe because I wrote it?

> 
> In fact, I'm not sure you even need two LedOps traits if you did that;
> it seems like you could even get away with reusing the trait for both
> cases.
Currently it only uses one LedOps trait. The implementation in the
multicolor needs access to `Device::subleds()`. `Device` gets passed as
a reference to every LedOps function. If there would be two `Device`
structs, I am not sure how we can preserve one LedOps trait. Possibly a
type declaration on `Mode`? (assuming we still want the `Mode` trait)

Thanks
- Markus Probst

> 
> Alice