Re: [PATCH net] rust: net::phy scope ThisModule usage in the module_phy_driver macro

[Jakub Kicinski <kuba@kernel.org>]

On Wed, 13 Nov 2024 17:45:22 +0000 Rahul Rameshbabu wrote:
> Similar to the use of $crate::Module, ThisModule should be referred to as
> $crate::ThisModule in the macro evaluation. The reason the macro previously
> did not cause any errors is because all the users of the macro would use
> kernel::prelude::*, bringing ThisModule into scope.

You say "previously", does it mean there are no in-tree users where
this could cause bugs? If so no Fixes tag necessary..

> Fixes: 2fe11d5ab35d ("rust: net::phy add module_phy_driver macro")
> Signed-off-by: Rahul Rameshbabu <sergeantsagara@protonmail.com>
> ---
> 
> Notes:
>     How I came up with this change:
>     
>     I was working on my own rust bindings and rust driver when I compared my
>     macro_rule to the one used for module_phy_driver. I noticed, if I made a
>     driver that does not use kernel::prelude::*, that the ThisModule type
>     identifier used in the macro would cause an error without being scoped in
>     the macro_rule. I believe the correct implementation for the macro is one
>     where the types used are correctly expanded with needed scopes.

Rust experts, does the patch itself make sense?

> diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
> index 910ce867480a..80f9f571b88c 100644
> --- a/rust/kernel/net/phy.rs
> +++ b/rust/kernel/net/phy.rs
> @@ -837,7 +837,7 @@ const fn as_int(&self) -> u32 {
>  ///         [::kernel::net::phy::create_phy_driver::<PhySample>()];
>  ///
>  ///     impl ::kernel::Module for Module {
> -///         fn init(module: &'static ThisModule) -> Result<Self> {
> +///         fn init(module: &'static ::kernel::ThisModule) -> Result<Self> {
>  ///             let drivers = unsafe { &mut DRIVERS };
>  ///             let mut reg = ::kernel::net::phy::Registration::register(
>  ///                 module,
> @@ -899,7 +899,7 @@ struct Module {
>                  [$($crate::net::phy::create_phy_driver::<$driver>()),+];
>  
>              impl $crate::Module for Module {
> -                fn init(module: &'static ThisModule) -> Result<Self> {
> +                fn init(module: &'static $crate::ThisModule) -> Result<Self> {
>                      // SAFETY: The anonymous constant guarantees that nobody else can access
>                      // the `DRIVERS` static. The array is used only in the C side.
>                      let drivers = unsafe { &mut DRIVERS };
> 
> base-commit: 73af53d82076bbe184d9ece9e14b0dc8599e6055