netdev - Re: [PATCH net-next v5 1/5] rust: core abstractions for network PHY drivers

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: <20231021.134406.872906134955921319.fujita.tomonori@gmail.com>
Date: Sat, 21 Oct 2023 13:44:06 +0900 (JST)
From: FUJITA Tomonori <fujita.tomonori@...il.com>
To: andrew@...n.ch
Cc: benno.lossin@...ton.me, fujita.tomonori@...il.com,
 netdev@...r.kernel.org, rust-for-linux@...r.kernel.org,
 miguel.ojeda.sandonis@...il.com, tmgross@...ch.edu, boqun.feng@...il.com,
 wedsonaf@...il.com, greg@...ah.com
Subject: Re: [PATCH net-next v5 1/5] rust: core abstractions for network
 PHY drivers

On Fri, 20 Oct 2023 20:42:10 +0200
Andrew Lunn <andrew@...n.ch> wrote:

>> > +//! All the PHYLIB helper functions for `phy_device` modify some members in `phy_device`. Except for
>> > +//! getter functions, [`Device`] methods take `&mut self`. This also applied to `read()`, which reads
>> > +//! a hardware register and updates the stats.
>> 
>> I would use [`Device`] instead of `phy_device`, since the Rust reader
>> might not be aware what wraps `phy_device`.
> 
> We don't want to hide phy_device too much, since at the moment, the
> abstraction is very minimal. Anybody writing a driver is going to need
> a good understanding of the C code in order to find the helpers they
> need, and then add them to the abstraction. So i would say we need to
> explain the relationship between the C structure and the Rust
> structure, to aid developers.

I'm sure that Rust readers would notice Device wraps `phy_device`
because the comment on Device clearly says so.

/// An instance of a PHY device.
///
/// Wraps the kernel's `struct phy_device`.
///
/// # Invariants
///
/// `self.0` is always in a valid state.
#[repr(transparent)]
pub struct Device(Opaque<bindings::phy_device>);


I think that the relationships between the C and Rust structures are
documented in phy.rs.