rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2 0/2] author field in module! macro should be a array
@ 2025-02-14 18:45 Guilherme Giacomo Simoes
  2025-02-14 18:45 ` [PATCH V2 1/2] rust: module: change author to " Guilherme Giacomo Simoes
  2025-02-14 18:45 ` [PATCH V2 2/2] checkpatch: throw error in malformed arrays Guilherme Giacomo Simoes
  0 siblings, 2 replies; 10+ messages in thread
From: Guilherme Giacomo Simoes @ 2025-02-14 18:45 UTC (permalink / raw)
  To: a.hindborg, alex.gaynor, aliceryhl, apw, arnd, aswinunni01, axboe,
	benno.lossin, bhelgaas, bjorn3_gh, boqun.feng, dakr,
	dwaipayanray1, ethan.twardy, fujita.tomonori, gary, gregkh, joe,
	lukas.bulwahn, ojeda, pbonzini, tmgross, walmeida
  Cc: trintaeoitogc, rust-for-linux, linux-kernel

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.

- Change the check patch to find poorly formatted arrays in the macro
  module!

--- 
V2 changes
- Merge the changes in module.rs and in modules that contains a author of
  module in a only one commit.

- Remove uneccessary "FOR EXAMPLE" in checkpatch

- Change from ERROR to WARN in message that is throw in checkpatch

- Remove the hungarian style namings in checkptach

- Improve code formatting in checkpatch

Guilherme Giacomo Simoes (2):
  rust: module: change author to be a array
  checkpatch: throw error in malformed arrays

 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 +-
 scripts/checkpatch.pl            | 43 ++++++++++++++++++++++++++++++++
 10 files changed, 58 insertions(+), 13 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH V2 1/2] rust: module: change author to be a array
  2025-02-14 18:45 [PATCH V2 0/2] author field in module! macro should be a array Guilherme Giacomo Simoes
@ 2025-02-14 18:45 ` Guilherme Giacomo Simoes
  2025-02-14 20:19   ` Charalampos Mitrodimas
                     ` (2 more replies)
  2025-02-14 18:45 ` [PATCH V2 2/2] checkpatch: throw error in malformed arrays Guilherme Giacomo Simoes
  1 sibling, 3 replies; 10+ messages in thread
From: Guilherme Giacomo Simoes @ 2025-02-14 18:45 UTC (permalink / raw)
  To: a.hindborg, alex.gaynor, aliceryhl, apw, arnd, aswinunni01, axboe,
	benno.lossin, bhelgaas, bjorn3_gh, boqun.feng, dakr,
	dwaipayanray1, ethan.twardy, fujita.tomonori, gary, gregkh, joe,
	lukas.bulwahn, ojeda, pbonzini, tmgross, walmeida
  Cc: trintaeoitogc, rust-for-linux, linux-kernel, Miguel Ojeda

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.

Suggested-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Link: https://github.com/Rust-for-Linux/linux/issues/244
Signed-off-by: Guilherme Giacomo Simoes <trintaeoitogc@gmail.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"],
     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
@@ -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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH V2 2/2] checkpatch: throw error in malformed arrays
  2025-02-14 18:45 [PATCH V2 0/2] author field in module! macro should be a array Guilherme Giacomo Simoes
  2025-02-14 18:45 ` [PATCH V2 1/2] rust: module: change author to " Guilherme Giacomo Simoes
@ 2025-02-14 18:45 ` Guilherme Giacomo Simoes
  2025-02-15  5:24   ` Joe Perches
  1 sibling, 1 reply; 10+ messages in thread
From: Guilherme Giacomo Simoes @ 2025-02-14 18:45 UTC (permalink / raw)
  To: a.hindborg, alex.gaynor, aliceryhl, apw, arnd, aswinunni01, axboe,
	benno.lossin, bhelgaas, bjorn3_gh, boqun.feng, dakr,
	dwaipayanray1, ethan.twardy, fujita.tomonori, gary, gregkh, joe,
	lukas.bulwahn, ojeda, pbonzini, tmgross, walmeida
  Cc: trintaeoitogc, rust-for-linux, linux-kernel

In module! macro, some fields have a string array type, and we need a
check for guarantee a good formatation.
Check if the current line contains one of these keys that must be a
string array and if so, make a regex for check if contains more than one
value in array, if yes, the array should be in vertical. If array have
only one value, so the array can be in the same line.

Signed-off-by: Guilherme Giacomo Simoes <trintaeoitogc@gmail.com>
---
 scripts/checkpatch.pl | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7b28ad331742..89fbec713bf3 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2775,6 +2775,9 @@ sub process {
 	$realcnt = 0;
 	$linenr = 0;
 	$fixlinenr = -1;
+
+	my %array_parse_module;
+
 	foreach my $line (@lines) {
 		$linenr++;
 		$fixlinenr++;
@@ -3567,6 +3570,46 @@ sub process {
 # ignore non-hunk lines and lines being removed
 		next if (!$hunk_line || $line =~ /^-/);
 
+# check if arrays has more than one value in the same line
+		my $inline = 0;
+		my $key = "";
+		my $add_line = $line =~ /^\+/;
+
+		if ($line =~ /\s*.*\b(author|alias|firmware)\s*:\s*\[/) {
+			$inline = 1;
+			$array_parse_module{$1} = 1;
+		}
+
+		my @keys = keys %array_parse_module;
+		if (@keys) {
+			$key = $keys[0];
+		}
+
+		if ($add_line && $key) {
+			my $herevet = "\n$here\n" . cat_vet($rawline) . "\n";
+
+			my $counter = () = $line =~ /"/g;
+			my $more_than_one = $counter > 2;
+			if ($more_than_one) {
+				WARN("ARRAY_MODULE_MACRO",
+						"Prefere one value per line$herevet");
+			} elsif ($inline && $line !~ /\]/ && $line !~ /,/ && $line =~ /"/) {
+				WARN("ARRAY_MODULE_MACRO",
+						"Prefere declare ] in the same line$herevet");
+			} elsif (!$inline && $line =~ /\]/ && $line =~ /\"/) {
+				WARN("ARRAY_MODULE_MACRO",
+						"Prefere a new line after last value and before ]$herevet");
+			} elsif ($inline && $line =~ /,/ && $line !~ /\]/) {
+				WARN("ARRAY_MODULE_MACRO",
+						"Prefere a new line after [$herevet");
+			}
+		}
+
+		#END OF ANALYZE FIELD
+		if ($line =~ /\]/) {
+			delete $array_parse_module{$key};
+		}
+
 #trailing whitespace
 		if ($line =~ /^\+.*\015/) {
 			my $herevet = "$here\n" . cat_vet($rawline) . "\n";
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH V2 1/2] rust: module: change author to be a array
  2025-02-14 18:45 ` [PATCH V2 1/2] rust: module: change author to " Guilherme Giacomo Simoes
@ 2025-02-14 20:19   ` Charalampos Mitrodimas
  2025-02-15  1:11     ` Guilherme Giacomo Simoes
  2025-02-15  1:39   ` Benno Lossin
  2025-02-16 12:35   ` kernel test robot
  2 siblings, 1 reply; 10+ messages in thread
From: Charalampos Mitrodimas @ 2025-02-14 20:19 UTC (permalink / raw)
  To: Guilherme Giacomo Simoes
  Cc: a.hindborg, alex.gaynor, aliceryhl, apw, arnd, aswinunni01, axboe,
	benno.lossin, bhelgaas, bjorn3_gh, boqun.feng, dakr,
	dwaipayanray1, ethan.twardy, fujita.tomonori, gary, gregkh, joe,
	lukas.bulwahn, ojeda, pbonzini, tmgross, walmeida, rust-for-linux,
	linux-kernel, Miguel Ojeda

Guilherme Giacomo Simoes <trintaeoitogc@gmail.com> writes:

> 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.
>
> Suggested-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
> Link: https://github.com/Rust-for-Linux/linux/issues/244
> Signed-off-by: Guilherme Giacomo Simoes <trintaeoitogc@gmail.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"],
>      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
> @@ -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);
> +        }

I wonder if we should make this "for author in authors", for code
clarity concerns.

>      }
>      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",
>  }

It could be that you developed this with an old tree history. You missed
changing the author for the samples/rust/rust_driver_platform.rs sample.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V2 1/2] rust: module: change author to be a array
  2025-02-14 20:19   ` Charalampos Mitrodimas
@ 2025-02-15  1:11     ` Guilherme Giacomo Simoes
  0 siblings, 0 replies; 10+ messages in thread
From: Guilherme Giacomo Simoes @ 2025-02-15  1:11 UTC (permalink / raw)
  To: charmitro
  Cc: a.hindborg, alex.gaynor, aliceryhl, apw, arnd, aswinunni01, axboe,
	benno.lossin, bhelgaas, bjorn3_gh, boqun.feng, dakr,
	dwaipayanray1, ethan.twardy, fujita.tomonori, gary, gregkh, joe,
	linux-kernel, lukas.bulwahn, miguel.ojeda.sandonis, ojeda,
	pbonzini, rust-for-linux, tmgross, trintaeoitogc, walmeida

Charalampos Mitrodimas <charmitro@posteo.net> wrotes:
> I wonder if we should make this "for author in authors", for code
> clarity concerns.
You is write, I will make this change.

> It could be that you developed this with an old tree history. You missed
> changing the author for the samples/rust/rust_driver_platform.rs sample.
Yes, my bad. 
I will include this driver

Thanks,
Guilherme

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V2 1/2] rust: module: change author to be a array
  2025-02-14 18:45 ` [PATCH V2 1/2] rust: module: change author to " Guilherme Giacomo Simoes
  2025-02-14 20:19   ` Charalampos Mitrodimas
@ 2025-02-15  1:39   ` Benno Lossin
  2025-02-15 13:52     ` Guilherme Giacomo Simoes
  2025-02-16 12:35   ` kernel test robot
  2 siblings, 1 reply; 10+ messages in thread
From: Benno Lossin @ 2025-02-15  1:39 UTC (permalink / raw)
  To: Guilherme Giacomo Simoes, a.hindborg, alex.gaynor, aliceryhl, apw,
	arnd, aswinunni01, axboe, bhelgaas, bjorn3_gh, boqun.feng, dakr,
	dwaipayanray1, ethan.twardy, fujita.tomonori, gary, gregkh, joe,
	lukas.bulwahn, ojeda, pbonzini, tmgross, walmeida
  Cc: rust-for-linux, linux-kernel, Miguel Ojeda

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@gmail.com>
> Link: https://github.com/Rust-for-Linux/linux/issues/244
> Signed-off-by: Guilherme Giacomo Simoes <trintaeoitogc@gmail.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
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V2 2/2] checkpatch: throw error in malformed arrays
  2025-02-14 18:45 ` [PATCH V2 2/2] checkpatch: throw error in malformed arrays Guilherme Giacomo Simoes
@ 2025-02-15  5:24   ` Joe Perches
  0 siblings, 0 replies; 10+ messages in thread
From: Joe Perches @ 2025-02-15  5:24 UTC (permalink / raw)
  To: Guilherme Giacomo Simoes, a.hindborg, alex.gaynor, aliceryhl, apw,
	arnd, aswinunni01, axboe, benno.lossin, bhelgaas, bjorn3_gh,
	boqun.feng, dakr, dwaipayanray1, ethan.twardy, fujita.tomonori,
	gary, gregkh, lukas.bulwahn, ojeda, pbonzini, tmgross, walmeida
  Cc: rust-for-linux, linux-kernel

On Fri, 2025-02-14 at 15:45 -0300, Guilherme Giacomo Simoes wrote:
> In module! macro, some fields have a string array type, and we need a
> check for guarantee a good formatation.
> Check if the current line contains one of these keys that must be a
> string array and if so, make a regex for check if contains more than one
> value in array, if yes, the array should be in vertical. If array have
> only one value, so the array can be in the same line.
> 
> Signed-off-by: Guilherme Giacomo Simoes <trintaeoitogc@gmail.com>
[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -3567,6 +3570,46 @@ sub process {
[]
> +
> +		if ($add_line && $key) {
> +			my $herevet = "\n$here\n" . cat_vet($rawline) . "\n";

Please look at the other checkpatch uses of $herevet 
$herevet does not begin with '\n'

> +
> +			my $counter = () = $line =~ /"/g;
> +			my $more_than_one = $counter > 2;
> +			if ($more_than_one) {
> +				WARN("ARRAY_MODULE_MACRO",
> +						"Prefere one value per line$herevet");

Prefer not Prefere
Align to open parenthesis
So this should be
				WARN("ARRAY_MODULE_MACRO"
				     "Prefer one value per line\n" . $herevet);

etc...


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V2 1/2] rust: module: change author to be a array
  2025-02-15  1:39   ` Benno Lossin
@ 2025-02-15 13:52     ` Guilherme Giacomo Simoes
  2025-02-15 14:34       ` Benno Lossin
  0 siblings, 1 reply; 10+ messages in thread
From: Guilherme Giacomo Simoes @ 2025-02-15 13:52 UTC (permalink / raw)
  To: benno.lossin
  Cc: a.hindborg, alex.gaynor, aliceryhl, apw, arnd, aswinunni01, axboe,
	bhelgaas, bjorn3_gh, boqun.feng, dakr, dwaipayanray1,
	ethan.twardy, fujita.tomonori, gary, gregkh, joe, linux-kernel,
	lukas.bulwahn, miguel.ojeda.sandonis, ojeda, pbonzini,
	rust-for-linux, tmgross, trintaeoitogc, walmeida

Benno Lossin <benno.lossin@proton.me> wrote:
> 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.
Thanks for your suggestions, in the truth my english is not so good.

> As already mentioned above, I think it makes sense to also rename the
> field to `authors`.
When I maked this change, I thinked in change the field name from author to
authors, but I don't doing this because anothers fields that is a array in
module! macro (firmware and alias) are in singular too.

> 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.
Yes, you is right... I haven't seen this chunk of text 


Thanks,
Guilherme

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V2 1/2] rust: module: change author to be a array
  2025-02-15 13:52     ` Guilherme Giacomo Simoes
@ 2025-02-15 14:34       ` Benno Lossin
  0 siblings, 0 replies; 10+ messages in thread
From: Benno Lossin @ 2025-02-15 14:34 UTC (permalink / raw)
  To: Guilherme Giacomo Simoes
  Cc: a.hindborg, alex.gaynor, aliceryhl, apw, arnd, aswinunni01, axboe,
	bhelgaas, bjorn3_gh, boqun.feng, dakr, dwaipayanray1,
	ethan.twardy, fujita.tomonori, gary, gregkh, joe, linux-kernel,
	lukas.bulwahn, miguel.ojeda.sandonis, ojeda, pbonzini,
	rust-for-linux, tmgross, walmeida

On 15.02.25 14:52, Guilherme Giacomo Simoes wrote:
> Benno Lossin <benno.lossin@proton.me> wrote:
>> As already mentioned above, I think it makes sense to also rename the
>> field to `authors`.
> When I maked this change, I thinked in change the field name from author to
> authors, but I don't doing this because anothers fields that is a array in
> module! macro (firmware and alias) are in singular too.

Hmm IMO the `alias` field should also probably be called `aliases`
(though not in this patch). And I believe that the plural of `firmware`
also is `firmware`.
So I still think it's a good idea to rename it to `authors`.

>> 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.
> Yes, you is right... I haven't seen this chunk of text

No worries, that's what review is for.

---
Cheers,
Benno


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V2 1/2] rust: module: change author to be a array
  2025-02-14 18:45 ` [PATCH V2 1/2] rust: module: change author to " Guilherme Giacomo Simoes
  2025-02-14 20:19   ` Charalampos Mitrodimas
  2025-02-15  1:39   ` Benno Lossin
@ 2025-02-16 12:35   ` kernel test robot
  2 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2025-02-16 12:35 UTC (permalink / raw)
  To: Guilherme Giacomo Simoes, a.hindborg, alex.gaynor, aliceryhl, apw,
	arnd, aswinunni01, axboe, benno.lossin, bhelgaas, bjorn3_gh,
	boqun.feng, dakr, dwaipayanray1, ethan.twardy, fujita.tomonori,
	gary, gregkh, joe, lukas.bulwahn, ojeda, pbonzini, tmgross,
	walmeida
  Cc: llvm, oe-kbuild-all, trintaeoitogc, rust-for-linux, linux-kernel,
	Miguel Ojeda

Hi Guilherme,

kernel test robot noticed the following build errors:

[auto build test ERROR on rust/rust-next]
[also build test ERROR on pci/next pci/for-linus char-misc/char-misc-testing char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.14-rc2 next-20250214]
[cannot apply to rust/rust-block-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Guilherme-Giacomo-Simoes/rust-module-change-author-to-be-a-array/20250215-024906
base:   https://github.com/Rust-for-Linux/linux rust-next
patch link:    https://lore.kernel.org/r/20250214184550.120775-2-trintaeoitogc%40gmail.com
patch subject: [PATCH V2 1/2] rust: module: change author to be a array
config: x86_64-randconfig-006-20250216 (https://download.01.org/0day-ci/archive/20250216/202502162042.nKMk0Rff-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250216/202502162042.nKMk0Rff-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202502162042.nKMk0Rff-lkp@intel.com/

All errors (new ones prefixed by >>):

>> error: proc macro panicked
   --> samples/rust/rust_driver_platform.rs:43:1
   |
   43 | / kernel::module_platform_driver! {
   44 | |     type: SampleDriver,
   45 | |     name: "rust_driver_platform",
   46 | |     author: "Danilo Krummrich",
   47 | |     description: "Rust Platform driver",
   48 | |     license: "GPL v2",
   49 | | }
   | |_^
   |
   = help: message: Expected Group
   = note: this error originates in the macro `$crate::module_driver` which comes from the expansion of the macro `kernel::module_platform_driver` (in Nightly builds, run with -Z macro-backtrace for more info)
--
>> error[E0277]: the trait bound `DriverModule: ModuleMetadata` is not satisfied
   --> samples/rust/rust_driver_platform.rs:43:1
   |
   43 | / kernel::module_platform_driver! {
   44 | |     type: SampleDriver,
   45 | |     name: "rust_driver_platform",
   46 | |     author: "Danilo Krummrich",
   47 | |     description: "Rust Platform driver",
   48 | |     license: "GPL v2",
   49 | | }
   | |_^ the trait `ModuleMetadata` is not implemented for `DriverModule`
   |
   = note: this error originates in the macro `$crate::module_driver` which comes from the expansion of the macro `kernel::module_platform_driver` (in Nightly builds, run with -Z macro-backtrace for more info)
--
>> error: proc macro panicked
   --> drivers/net/phy/ax88796b_rust.rs:14:1
   |
   14 | / kernel::module_phy_driver! {
   15 | |     drivers: [PhyAX88772A, PhyAX88772C, PhyAX88796B],
   16 | |     device_table: [
   17 | |         DeviceId::new_with_driver::<PhyAX88772A>(),
   ...  |
   24 | |     license: "GPL",
   25 | | }
   | |_^
   |
   = help: message: Expected Group
   = note: this error originates in the macro `kernel::module_phy_driver` (in Nightly builds, run with -Z macro-backtrace for more info)

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-02-16 12:36 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-14 18:45 [PATCH V2 0/2] author field in module! macro should be a array Guilherme Giacomo Simoes
2025-02-14 18:45 ` [PATCH V2 1/2] rust: module: change author to " Guilherme Giacomo Simoes
2025-02-14 20:19   ` Charalampos Mitrodimas
2025-02-15  1:11     ` Guilherme Giacomo Simoes
2025-02-15  1:39   ` Benno Lossin
2025-02-15 13:52     ` Guilherme Giacomo Simoes
2025-02-15 14:34       ` Benno Lossin
2025-02-16 12:35   ` kernel test robot
2025-02-14 18:45 ` [PATCH V2 2/2] checkpatch: throw error in malformed arrays Guilherme Giacomo Simoes
2025-02-15  5:24   ` Joe Perches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).