All of lore.kernel.org
 help / color / mirror / Atom feed
From: Boqun Feng <boqun.feng@gmail.com>
To: FUJITA Tomonori <fujita.tomonori@gmail.com>
Cc: alice@ryhl.io, netdev@vger.kernel.org,
	rust-for-linux@vger.kernel.org, andrew@lunn.ch,
	tmgross@umich.edu, miguel.ojeda.sandonis@gmail.com,
	benno.lossin@proton.me, wedsonaf@gmail.com, aliceryhl@google.com
Subject: Re: [PATCH net-next v10 1/4] rust: core abstractions for network PHY drivers
Date: Mon, 11 Dec 2023 15:40:33 -0800	[thread overview]
Message-ID: <ZXed8cQLJhDSTuXG@boqun-archlinux> (raw)
In-Reply-To: <20231212.081505.1423250811446494582.fujita.tomonori@gmail.com>

On Tue, Dec 12, 2023 at 08:15:05AM +0900, FUJITA Tomonori wrote:
[...]
> >> +    /// 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..

Regards,
Boqun

  reply	other threads:[~2023-12-11 23:40 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-10 23:49 [PATCH net-next v10 0/4] Rust abstractions for network PHY drivers FUJITA Tomonori
2023-12-10 23:49 ` [PATCH net-next v10 1/4] rust: core " FUJITA Tomonori
2023-12-11 14:01   ` Andrew Lunn
2023-12-11 19:49   ` [net-next PATCH] rust: net: phy: Correct the safety comment for impl Sync Boqun Feng
2023-12-11 20:23     ` Boqun Feng
2023-12-11 21:50     ` Alice Ryhl
2023-12-11 23:22       ` Boqun Feng
2023-12-11 23:55         ` FUJITA Tomonori
2023-12-12  9:17           ` Alice Ryhl
2023-12-12 10:36             ` FUJITA Tomonori
2023-12-11 21:46   ` [PATCH net-next v10 1/4] rust: core abstractions for network PHY drivers Alice Ryhl
2023-12-11 23:15     ` FUJITA Tomonori
2023-12-11 23:40       ` Boqun Feng [this message]
2023-12-11 23:47         ` FUJITA Tomonori
2023-12-12  0:49           ` Boqun Feng
2023-12-12  1:46             ` FUJITA Tomonori
2023-12-12  2:30               ` Boqun Feng
2023-12-12  4:04                 ` FUJITA Tomonori
2023-12-12  6:11                   ` Boqun Feng
2023-12-12 13:02                     ` FUJITA Tomonori
2023-12-12 17:35                       ` Benno Lossin
2023-12-12 20:23                         ` Boqun Feng
2023-12-12 22:40                           ` Benno Lossin
2023-12-12 23:27                             ` FUJITA Tomonori
2023-12-13  0:02                               ` Benno Lossin
2023-12-12 23:31                           ` FUJITA Tomonori
2023-12-13  0:01                             ` Benno Lossin
2023-12-12 23:01                         ` FUJITA Tomonori
2023-12-12 23:15                           ` Benno Lossin
2023-12-13 10:28                         ` Andrew Lunn
2023-12-13 12:14                           ` Benno Lossin
2023-12-13 10:24                     ` Andrew Lunn
2023-12-13 16:43                       ` Boqun Feng
2023-12-13 17:12                         ` Boqun Feng
2023-12-13 21:48                         ` Andrew Lunn
2023-12-13 23:40                           ` Benno Lossin
2023-12-13 23:51                             ` Benno Lossin
2023-12-14  9:26                             ` Andrew Lunn
2023-12-13 23:59                           ` Boqun Feng
2023-12-12 12:55                   ` Miguel Ojeda
2023-12-12  9:23       ` Alice Ryhl
2023-12-12 10:56         ` FUJITA Tomonori
2023-12-10 23:49 ` [PATCH net-next v10 2/4] rust: net::phy add module_phy_driver macro FUJITA Tomonori
2023-12-11 14:01   ` Andrew Lunn
2023-12-12 22:52   ` Trevor Gross
2023-12-10 23:49 ` [PATCH net-next v10 3/4] MAINTAINERS: add Rust PHY abstractions for ETHERNET PHY LIBRARY FUJITA Tomonori
2023-12-10 23:49 ` [PATCH net-next v10 4/4] net: phy: add Rust Asix PHY driver FUJITA Tomonori
2023-12-11 14:01   ` Andrew Lunn
2023-12-11 21:52   ` Alice Ryhl

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZXed8cQLJhDSTuXG@boqun-archlinux \
    --to=boqun.feng@gmail.com \
    --cc=alice@ryhl.io \
    --cc=aliceryhl@google.com \
    --cc=andrew@lunn.ch \
    --cc=benno.lossin@proton.me \
    --cc=fujita.tomonori@gmail.com \
    --cc=miguel.ojeda.sandonis@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    --cc=wedsonaf@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.