On Mon, 11 Dec 2023 15:40:33 -0800
Boqun Feng [off-list ref] wrote:

On Tue, Dec 12, 2023 at 08:15:05AM +0900, FUJITA Tomonori wrote:
[...]

quoted

quoted

quoted

+    /// Reads a given C22 PHY register.
+ // This function reads a hardware register and updates the stats so
takes `&mut self`.
+    pub fn read(&mut self, regnum: u16) -> Result<u16> {
+        let phydev = self.0.get();
+ // SAFETY: `phydev` is pointing to a valid object by the type
invariant of `Self`.
+        // So an FFI call with a valid pointer.

This sentence also doesn't parse in my brain. Perhaps "So it's just an
FFI call" or similar?

"So it's just an FFI call" looks good. I'll fix all the places that
use the same comment.

I would also mention that `(*phydev).mdio.addr` is smaller than
PHY_MAX_ADDR (per C side invariants in mdio maybe), since otherwise
mdiobus_read() would cause out-of-bound accesses at ->stats. The safety
comments are supposed to describe why calling the C function won't cause
memory safety issues..

(*phydev).mdio.addr is managed in the C side and Rust code doesn't
touch it (doesn't need to know anything about it). What safety comment
should be written here?