linux-kernel - Re: [PATCH v5 4/9] rust: device: Enable printing fwnode name and path

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 for Android: free password hash cracker in your pocket

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

Message-ID: <2025052116-gem-blend-2585@gregkh>
Date: Wed, 21 May 2025 18:58:45 +0200
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Remo Senekowitsch <remo@...nzli.dev>
Cc: Rob Herring <robh@...nel.org>, Saravana Kannan <saravanak@...gle.com>,
	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 <benno.lossin@...ton.me>,
	Andreas Hindborg <a.hindborg@...nel.org>,
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
	Danilo Krummrich <dakr@...nel.org>,
	"Rafael J. Wysocki" <rafael@...nel.org>,
	Dirk Behme <dirk.behme@...bosch.com>, linux-kernel@...r.kernel.org,
	devicetree@...r.kernel.org, rust-for-linux@...r.kernel.org
Subject: Re: [PATCH v5 4/9] rust: device: Enable printing fwnode name and path

On Wed, May 21, 2025 at 03:03:07PM +0200, Remo Senekowitsch wrote:
> On Wed May 21, 2025 at 2:02 PM CEST, Greg Kroah-Hartman wrote:
> > On Tue, May 20, 2025 at 10:00:19PM +0200, Remo Senekowitsch wrote:
> >> Add two new public methods `display_name` and `display_path` to
> >> `FwNode`. They can be used by driver authors for logging purposes. In
> >> addition, they will be used by core property abstractions for automatic
> >> logging, for example when a driver attempts to read a required but
> >> missing property.
> >> 
> >> Signed-off-by: Remo Senekowitsch <remo@...nzli.dev>
> >> ---
> >>  rust/kernel/device/property.rs | 72 ++++++++++++++++++++++++++++++++++
> >>  1 file changed, 72 insertions(+)
> >> 
> >> diff --git a/rust/kernel/device/property.rs b/rust/kernel/device/property.rs
> >> index 70593343bd811..6ccc7947f9c31 100644
> >> --- a/rust/kernel/device/property.rs
> >> +++ b/rust/kernel/device/property.rs
> >> @@ -32,6 +32,78 @@ pub(crate) fn as_raw(&self) -> *mut bindings::fwnode_handle {
> >>          self.0.get()
> >>      }
> >>  
> >> +    /// Returns an object that implements [`Display`](core::fmt::Display) for
> >> +    /// printing the name of a node.
> >> +    pub fn display_name(&self) -> impl core::fmt::Display + '_ {
> >> +        struct FwNodeDisplayName<'a>(&'a FwNode);
> >> +
> >> +        impl core::fmt::Display for FwNodeDisplayName<'_> {
> >> +            fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
> >> +                // SAFETY: self is valid by its type invariant
> >> +                let name = unsafe { bindings::fwnode_get_name(self.0.as_raw()) };
> >> +                if name.is_null() {
> >> +                    return Ok(());
> >
> > So if there is no name, you are returning Ok()?  Are you sure that's ok
> > to do?  What will the result of the string look like then?
> 
> In that case we're not writing anything to the formatter, which is
> equivalent to an empty string. `Ok(())` means that writing succeeded.
> 
> I assumed that a valid node would always have a name. And we're
> guaranteed to have a valid node. So I assumed this case would never
> happen and didn't think too hard about it. But even if a valid node has
> not name, empty string is probably the correct thing, right?

I don't know what this "name" is used for.  An empty string might not be
what you want to use here, given that you could be naming something
based on it, right?  fwnode_get_name() is used for many things,
including the detection if a name is not present at all, and if not,
then the code needs to clean up and abort.

So what exactly are you going to be using this for?

thanks,

greg k-h