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]
Message-ID: <93eaeb37-9913-49cb-be6d-367696a81baf@proton.me>
Date: Sat, 15 Feb 2025 01:39:17 +0000
From: Benno Lossin <benno.lossin@...ton.me>
To: Guilherme Giacomo Simoes <trintaeoitogc@...il.com>, a.hindborg@...nel.org, alex.gaynor@...il.com, aliceryhl@...gle.com, apw@...onical.com, arnd@...db.de, aswinunni01@...il.com, axboe@...nel.dk, bhelgaas@...gle.com, bjorn3_gh@...tonmail.com, boqun.feng@...il.com, dakr@...nel.org, dwaipayanray1@...il.com, ethan.twardy@...il.com, fujita.tomonori@...il.com, gary@...yguo.net, gregkh@...uxfoundation.org, joe@...ches.com, lukas.bulwahn@...il.com, ojeda@...nel.org, pbonzini@...hat.com, tmgross@...ch.edu, walmeida@...rosoft.com
Cc: rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org, Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
Subject: Re: [PATCH V2 1/2] rust: module: change author to be a array

On 14.02.25 19:45, Guilherme Giacomo Simoes wrote:
> In the module! macro, the author field has a string type. Once that the
> modules can has more than one author, this is impossible in the current
> scenary.
> Change the author field for accept a array string type and enable module
> creations with more than one author.
> In modules that use the author field, change its value to a string
> array.

This commit message looks a bit clunky to me, though I am not a native
speaker. My suggestion would be:

    Change the type of the `author` field in the `module!` macro from string
    to string array. This allows multiple authors for a single module as is
    already possible in C.
    Additionally, rename the field to `authors` to better reflect this
    change.

Feel free to change/adapt anything.

> Suggested-by: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
> Link: https://github.com/Rust-for-Linux/linux/issues/244
> Signed-off-by: Guilherme Giacomo Simoes <trintaeoitogc@...il.com>
> ---
>  drivers/block/rnull.rs           | 2 +-
>  rust/kernel/net/phy.rs           | 4 ++--
>  rust/kernel/pci.rs               | 2 +-
>  rust/macros/lib.rs               | 4 ++--
>  rust/macros/module.rs            | 8 +++++---
>  samples/rust/rust_driver_pci.rs  | 2 +-
>  samples/rust/rust_minimal.rs     | 2 +-
>  samples/rust/rust_misc_device.rs | 2 +-
>  samples/rust/rust_print_main.rs  | 2 +-
>  9 files changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/block/rnull.rs b/drivers/block/rnull.rs
> index ddf3629d8894..cb133993f27f 100644
> --- a/drivers/block/rnull.rs
> +++ b/drivers/block/rnull.rs
> @@ -27,7 +27,7 @@
>  module! {
>      type: NullBlkModule,
>      name: "rnull_mod",
> -    author: "Andreas Hindborg",
> +    author: ["Andreas Hindborg"],

As already mentioned above, I think it makes sense to also rename the
field to `authors`.

>      description: "Rust implementation of the C null block driver",
>      license: "GPL v2",
>  }
> diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
> index bb654a28dab3..b179ac3a8d00 100644
> --- a/rust/kernel/net/phy.rs
> +++ b/rust/kernel/net/phy.rs
> @@ -790,7 +790,7 @@ const fn as_int(&self) -> u32 {
>  ///         DeviceId::new_with_driver::<PhySample>()
>  ///     ],
>  ///     name: "rust_sample_phy",
> -///     author: "Rust for Linux Contributors",
> +///     author: ["Rust for Linux Contributors"],
>  ///     description: "Rust sample PHYs driver",
>  ///     license: "GPL",
>  /// }
> @@ -819,7 +819,7 @@ const fn as_int(&self) -> u32 {
>  /// module! {
>  ///     type: Module,
>  ///     name: "rust_sample_phy",
> -///     author: "Rust for Linux Contributors",
> +///     author: ["Rust for Linux Contributors"],
>  ///     description: "Rust sample PHYs driver",
>  ///     license: "GPL",
>  /// }
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 4c98b5b9aa1e..1218eaa7be02 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -103,7 +103,7 @@ extern "C" fn remove_callback(pdev: *mut bindings::pci_dev) {
>  /// kernel::module_pci_driver! {
>  ///     type: MyDriver,
>  ///     name: "Module name",
> -///     author: "Author name",
> +///     author: ["Author name"],
>  ///     description: "Description",
>  ///     license: "GPL v2",
>  /// }
> diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
> index d61bc6a56425..8d74e18caf96 100644
> --- a/rust/macros/lib.rs
> +++ b/rust/macros/lib.rs

You should also change the documentation in this file, there the field
`author` is listed as "string literal of the author of the kernel
module.", so it also needs to be updated.

---
Cheers,
Benno

> @@ -36,7 +36,7 @@
>  /// module!{
>  ///     type: MyModule,
>  ///     name: "my_kernel_module",
> -///     author: "Rust for Linux Contributors",
> +///     author: ["Rust for Linux Contributors"],
>  ///     description: "My very own kernel module!",
>  ///     license: "GPL",
>  ///     alias: ["alternate_module_name"],
> @@ -69,7 +69,7 @@
>  /// module!{
>  ///     type: MyDeviceDriverModule,
>  ///     name: "my_device_driver_module",
> -///     author: "Rust for Linux Contributors",
> +///     author: ["Rust for Linux Contributors"],
>  ///     description: "My device driver requires firmware",
>  ///     license: "GPL",
>  ///     firmware: ["my_device_firmware1.bin", "my_device_firmware2.bin"],
> diff --git a/rust/macros/module.rs b/rust/macros/module.rs
> index cdf94f4982df..09265d18b44d 100644
> --- a/rust/macros/module.rs
> +++ b/rust/macros/module.rs
> @@ -94,7 +94,7 @@ struct ModuleInfo {
>      type_: String,
>      license: String,
>      name: String,
> -    author: Option<String>,
> +    author: Option<Vec<String>>,
>      description: Option<String>,
>      alias: Option<Vec<String>>,
>      firmware: Option<Vec<String>>,
> @@ -135,7 +135,7 @@ fn parse(it: &mut token_stream::IntoIter) -> Self {
>              match key.as_str() {
>                  "type" => info.type_ = expect_ident(it),
>                  "name" => info.name = expect_string_ascii(it),
> -                "author" => info.author = Some(expect_string(it)),
> +                "author" => info.author = Some(expect_string_array(it)),
>                  "description" => info.description = Some(expect_string(it)),
>                  "license" => info.license = expect_string_ascii(it),
>                  "alias" => info.alias = Some(expect_string_array(it)),
> @@ -184,7 +184,9 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
> 
>      let mut modinfo = ModInfoBuilder::new(info.name.as_ref());
>      if let Some(author) = info.author {
> -        modinfo.emit("author", &author);
> +        for author in author {
> +            modinfo.emit("author", &author);
> +        }
>      }
>      if let Some(description) = info.description {
>          modinfo.emit("description", &description);
> diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs
> index 1fb6e44f3395..5784677c797b 100644
> --- a/samples/rust/rust_driver_pci.rs
> +++ b/samples/rust/rust_driver_pci.rs
> @@ -104,7 +104,7 @@ fn drop(&mut self) {
>  kernel::module_pci_driver! {
>      type: SampleDriver,
>      name: "rust_driver_pci",
> -    author: "Danilo Krummrich",
> +    author: ["Danilo Krummrich"],
>      description: "Rust PCI driver",
>      license: "GPL v2",
>  }
> diff --git a/samples/rust/rust_minimal.rs b/samples/rust/rust_minimal.rs
> index 4aaf117bf8e3..74279c3bd039 100644
> --- a/samples/rust/rust_minimal.rs
> +++ b/samples/rust/rust_minimal.rs
> @@ -7,7 +7,7 @@
>  module! {
>      type: RustMinimal,
>      name: "rust_minimal",
> -    author: "Rust for Linux Contributors",
> +    author: ["Rust for Linux Contributors"],
>      description: "Rust minimal sample",
>      license: "GPL",
>  }
> diff --git a/samples/rust/rust_misc_device.rs b/samples/rust/rust_misc_device.rs
> index 40ad7266c225..e840c12005cc 100644
> --- a/samples/rust/rust_misc_device.rs
> +++ b/samples/rust/rust_misc_device.rs
> @@ -116,7 +116,7 @@
>  module! {
>      type: RustMiscDeviceModule,
>      name: "rust_misc_device",
> -    author: "Lee Jones",
> +    author: ["Lee Jones"],
>      description: "Rust misc device sample",
>      license: "GPL",
>  }
> diff --git a/samples/rust/rust_print_main.rs b/samples/rust/rust_print_main.rs
> index 7e8af5f176a3..f6d51b0884fb 100644
> --- a/samples/rust/rust_print_main.rs
> +++ b/samples/rust/rust_print_main.rs
> @@ -8,7 +8,7 @@
>  module! {
>      type: RustPrint,
>      name: "rust_print",
> -    author: "Rust for Linux Contributors",
> +    author: ["Rust for Linux Contributors"],
>      description: "Rust printing macros sample",
>      license: "GPL",
>  }
> --
> 2.34.1
> 


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ