Re: [PATCH net-next v5 1/5] rust: core abstractions for network PHY drivers
From: FUJITA Tomonori <fujita.tomonori@gmail.com>
Date: 2023-10-21 21:45:05
Also in:
rust-for-linux
On Sat, 21 Oct 2023 13:35:57 +0000 Benno Lossin [off-list ref] wrote:
quoted
Currently, it needs &'static DriverVTable array so it works.That is actually also incorrect. As the C side is going to modify the `DriverVTable`, you should actually use `&'static mut DriverVTable`. But since it is not allowed to be moved you have to use `Pin<&'static mut DriverVTable>`.
I updated Registration::register(). Needs to add comments on requirement?
impl Registration {
/// Registers a PHY driver.
pub fn register(
module: &'static crate::ThisModule,
drivers: Pin<&'static mut [DriverVTable]>,
) -> Result<Self> {
// SAFETY: The type invariants of [`DriverVTable`] ensure that all elements of the `drivers` slice
// are initialized properly. So an FFI call with a valid pointer.
to_result(unsafe {
bindings::phy_drivers_register(drivers[0].0.get(), drivers.len().try_into()?, module.0)
})?;
// INVARIANT: The `drivers` slice is successfully registered to the kernel via `phy_drivers_register`.
Ok(Registration { drivers })
}
}
quoted
The C side uses static allocation too. If someone asks for, we could loosen the restriction with a complicated implentation. But I doubt that someone would ask for such.With Wedson's patch you also would be using the static allocation from `module!`. What my problem is, is that you are using a `static mut` which is `unsafe` and you do not actually have to use it (with Wedson's patch of course).
Like your vtable patch, I improve the code when something useful is available.