netdev - Re: [PATCH net-next v6 2/5] rust: net::phy add module_phy

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]

Date: Wed, 25 Oct 2023 08:08:53 +0000
From: Benno Lossin <benno.lossin@...ton.me>
To: FUJITA Tomonori <fujita.tomonori@...il.com>
Cc: netdev@...r.kernel.org, rust-for-linux@...r.kernel.org, andrew@...n.ch, tmgross@...ch.edu, miguel.ojeda.sandonis@...il.com, wedsonaf@...il.com, greg@...ah.com
Subject: Re: [PATCH net-next v6 2/5] rust: net::phy add module_phy_driver macro

On 25.10.23 09:57, FUJITA Tomonori wrote:
> On Wed, 25 Oct 2023 07:29:32 +0000
> Benno Lossin <benno.lossin@...ton.me> wrote:
> 
>> On 25.10.23 02:02, FUJITA Tomonori wrote:
>>> On Tue, 24 Oct 2023 16:28:02 +0000
>>> Benno Lossin <benno.lossin@...ton.me> wrote:
>>>
>>>> On 24.10.23 02:58, FUJITA Tomonori wrote:
>>>>> This macro creates an array of kernel's `struct phy_driver` and
>>>>> registers it. This also corresponds to the kernel's
>>>>> `MODULE_DEVICE_TABLE` macro, which embeds the information for module
>>>>> loading into the module binary file.
>>>>>
>>>>> A PHY driver should use this macro.
>>>>>
>>>>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@...il.com>
>>>>> ---
>>>>>     rust/kernel/net/phy.rs | 129 +++++++++++++++++++++++++++++++++++++++++
>>>>>     1 file changed, 129 insertions(+)
>>>>>
>>>>> diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
>>>>> index 2d821c2475e1..f346b2b4d3cb 100644
>>>>> --- a/rust/kernel/net/phy.rs
>>>>> +++ b/rust/kernel/net/phy.rs
>>>>> @@ -706,3 +706,132 @@ const fn as_int(&self) -> u32 {
>>>>>             }
>>>>>         }
>>>>>     }
>>>>> +
>>>>> +/// Declares a kernel module for PHYs drivers.
>>>>> +///
>>>>> +/// This creates a static array of kernel's `struct phy_driver` and registers it.
>>>>> +/// This also corresponds to the kernel's `MODULE_DEVICE_TABLE` macro, which embeds the information
>>>>> +/// for module loading into the module binary file. Every driver needs an entry in `device_table`.
>>>>> +///
>>>>> +/// # Examples
>>>>> +///
>>>>> +/// ```ignore
>>>>
>>>> Is this example ignored, because it does not compile?
>>>
>>> The old version can't be compiled but the current version can so I'll
>>> drop ignore.
>>>
>>>
>>>> I think Wedson was wrapping his example with `module!` inside
>>>> of a module, so maybe try that?
>>>
>>> I'm not sure what you mean.
>>
>> Wedson did this [1], note the `# mod module_fs_sample`:
>>
>> /// # Examples
>> ///
>> /// ```
>> /// # mod module_fs_sample {
>> /// use kernel::prelude::*;
>> /// use kernel::{c_str, fs};
>> ///
>> /// kernel::module_fs! {
>> ///     type: MyFs,
>> ///     name: "myfs",
>> ///     author: "Rust for Linux Contributors",
>> ///     description: "My Rust fs",
>> ///     license: "GPL",
>> /// }
>> ///
>> /// struct MyFs;
>> /// impl fs::FileSystem for MyFs {
>> ///     const NAME: &'static CStr = c_str!("myfs");
>> /// }
>> /// # }
>> /// ```
>>
>> [1]: https://github.com/wedsonaf/linux/commit/e909f439481cf6a3df00c7064b0d64cee8630fe9#diff-9b893393ed2a537222d79f6e2fceffb7e9d8967791c2016962be3171c446210fR104-R124
> 
> You are suggesting like the following?
> 
> /// # Examples
> ///
> /// ```
> /// # mod module_phy_driver_sample {
> ...
> /// # }
> /// ```
> 
> What benefits?

IIRC Wedson mentioned that without that it did not compile. So
if it compiles for you without it, I would not add it.

-- 
Cheers,
Benno