linux-kernel - Re: [PATCH v4 2/3] rust: pci: impl TryFrom<&Device> for &pci::Device

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: <D8OOFRRSLHP4.1B2FHQRGH3LKW@proton.me>
Date: Mon, 24 Mar 2025 17:36:45 +0000
From: Benno Lossin <benno.lossin@...ton.me>
To: Danilo Krummrich <dakr@...nel.org>
Cc: Greg KH <gregkh@...uxfoundation.org>, bhelgaas@...gle.com, rafael@...nel.org, ojeda@...nel.org, alex.gaynor@...il.com, boqun.feng@...il.com, gary@...yguo.net, bjorn3_gh@...tonmail.com, a.hindborg@...nel.org, aliceryhl@...gle.com, tmgross@...ch.edu, linux-pci@...r.kernel.org, rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 2/3] rust: pci: impl TryFrom<&Device> for &pci::Device

On Mon Mar 24, 2025 at 5:49 PM CET, Danilo Krummrich wrote:
> On Mon, Mar 24, 2025 at 04:39:25PM +0000, Benno Lossin wrote:
>> On Sun Mar 23, 2025 at 11:10 PM CET, Danilo Krummrich wrote:
>> > On Sat, Mar 22, 2025 at 11:10:57AM +0100, Danilo Krummrich wrote:
>> >> On Fri, Mar 21, 2025 at 08:25:07PM -0700, Greg KH wrote:
>> >> > Along these lines, if you can convince me that this is something that we
>> >> > really should be doing, in that we should always be checking every time
>> >> > someone would want to call to_pci_dev(), that the return value is
>> >> > checked, then why don't we also do this in C if it's going to be
>> >> > something to assure people it is going to be correct?  I don't want to
>> >> > see the rust and C sides get "out of sync" here for things that can be
>> >> > kept in sync, as that reduces the mental load of all of us as we travers
>> >> > across the boundry for the next 20+ years.
>> >> 
>> >> I think in this case it is good when the C and Rust side get a bit
>> >> "out of sync":
>> >
>> > A bit more clarification on this:
>> >
>> > What I want to say with this is, since we can cover a lot of the common cases
>> > through abstractions and the type system, we're left with the not so common
>> > ones, where the "upcasts" are not made in the context of common and well
>> > established patterns, but, for instance, depend on the semantics of the driver;
>> > those should not be unsafe IMHO.
>> 
>> I don't think that we should use `TryFrom` for stuff that should only be
>> used seldomly. A function that we can document properly is a much better
>> fit, since we can point users to the "correct" API.
>
> Most of the cases where drivers would do this conversion should be covered by
> the abstraction to already provide that actual bus specific device, rather than
> a generic one or some priv pointer, etc.
>
> So, the point is that the APIs we design won't leave drivers with a reason to
> make this conversion in the first place. For the cases where they have to
> (which should be rare), it's the right thing to do. There is not an alternative
> API to point to.

Yes, but for such a case, I wouldn't want to use `TryFrom`, since that
trait to me is a sign of a canonical way to convert a value. So if it
shouldn't be used lightly, then I would prefer a normal method. Even if
there is no alternative API, we could say that it is unusual to use it
and the correct type should normally be available. This kind of
documentation is not possible with `TryFrom`.

For a user it won't make a big difference, they'll just call a method
not named `try_from`.

---
Cheers,
Benno