rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V4 0/2] author field in module! macro should be a array
@ 2025-02-21 18:21 Guilherme Giacomo Simoes
  2025-02-21 18:21 ` [PATCH V4 1/2] rust: module: change author to an array Guilherme Giacomo Simoes
  2025-02-21 18:21 ` [PATCH V4 2/2] checkpatch: throw error for malformed arrays Guilherme Giacomo Simoes
  0 siblings, 2 replies; 5+ messages in thread
From: Guilherme Giacomo Simoes @ 2025-02-21 18:21 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 is currently of type String.
Since modules can have multiple authors, this limitation prevents
specifying more than one.

- Change the author field to Option<Vec<String>> to allow creating
  modules with multiple authors.

- rename the field from author to authors to make it explicit that it
  can refer to multiple authors.

- In modules that use the author field, update its value to an array of
  strings, and also rename it from author to authors.

- Change the checkpatch.pl to find poorly formatted arrays in the macro
  module!

---
V4 changes 

- Improved checkpatch warning messages
- Improved shiftwidth
- Fixed the regex in checkpatch
---

Guilherme Giacomo Simoes (2):
  rust: module: change author to an array
  checkpatch: throw error for malformed arrays

 drivers/block/rnull.rs               |  2 +-
 rust/kernel/net/phy.rs               |  4 +--
 rust/kernel/pci.rs                   |  2 +-
 rust/macros/lib.rs                   |  6 ++--
 rust/macros/module.rs                | 12 ++++----
 samples/rust/rust_driver_faux.rs     |  2 +-
 samples/rust/rust_driver_pci.rs      |  2 +-
 samples/rust/rust_driver_platform.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 ++++++++++++++++++++++++++++
 12 files changed, 63 insertions(+), 18 deletions(-)

-- 
2.34.1


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

* [PATCH V4 1/2] rust: module: change author to an array
  2025-02-21 18:21 [PATCH V4 0/2] author field in module! macro should be a array Guilherme Giacomo Simoes
@ 2025-02-21 18:21 ` Guilherme Giacomo Simoes
  2025-02-21 18:21 ` [PATCH V4 2/2] checkpatch: throw error for malformed arrays Guilherme Giacomo Simoes
  1 sibling, 0 replies; 5+ messages in thread
From: Guilherme Giacomo Simoes @ 2025-02-21 18:21 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 is currently of type String.
Since modules can have multiple authors, this limitation prevents
specifying more than one.
Change the author field to Option<Vec<String>> to allow creating modules
with multiple authors. Additionally, rename the field from author to
authors to make it explicit that it can refer to multiple authors.  In
modules that use the author field, update its value to an array of
strings, and also rename it from author to authors.

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                   |  6 +++---
 rust/macros/module.rs                | 12 +++++++-----
 samples/rust/rust_driver_faux.rs     |  2 +-
 samples/rust/rust_driver_pci.rs      |  2 +-
 samples/rust/rust_driver_platform.rs |  2 +-
 samples/rust/rust_minimal.rs         |  2 +-
 samples/rust/rust_misc_device.rs     |  2 +-
 samples/rust/rust_print_main.rs      |  2 +-
 11 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/drivers/block/rnull.rs b/drivers/block/rnull.rs
index ddf3629d8894..d07e76ae2c13 100644
--- a/drivers/block/rnull.rs
+++ b/drivers/block/rnull.rs
@@ -27,7 +27,7 @@
 module! {
     type: NullBlkModule,
     name: "rnull_mod",
-    author: "Andreas Hindborg",
+    authors: ["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..a59469c785e3 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",
+///     authors: ["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",
+///     authors: ["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..f7b2743828ae 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",
+///     authors: ["Author name"],
 ///     description: "Description",
 ///     license: "GPL v2",
 /// }
diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index d61bc6a56425..7ce1cb891dfb 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",
+///     authors: ["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",
+///     authors: ["Rust for Linux Contributors"],
 ///     description: "My device driver requires firmware",
 ///     license: "GPL",
 ///     firmware: ["my_device_firmware1.bin", "my_device_firmware2.bin"],
@@ -88,7 +88,7 @@
 /// # Supported argument types
 ///   - `type`: type which implements the [`Module`] trait (required).
 ///   - `name`: ASCII string literal of the name of the kernel module (required).
-///   - `author`: string literal of the author of the kernel module.
+///   - `authors`: array of ASCII string literals of the authors of the kernel module.
 ///   - `description`: string literal of the description of the kernel module.
 ///   - `license`: ASCII string literal of the license of the kernel module (required).
 ///   - `alias`: array of ASCII string literals of the alias names of the kernel module.
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index cdf94f4982df..db16f0af0855 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>,
+    authors: Option<Vec<String>>,
     description: Option<String>,
     alias: Option<Vec<String>>,
     firmware: Option<Vec<String>>,
@@ -107,7 +107,7 @@ fn parse(it: &mut token_stream::IntoIter) -> Self {
         const EXPECTED_KEYS: &[&str] = &[
             "type",
             "name",
-            "author",
+            "authors",
             "description",
             "license",
             "alias",
@@ -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)),
+                "authors" => info.authors = 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)),
@@ -183,8 +183,10 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
     let info = ModuleInfo::parse(&mut it);
 
     let mut modinfo = ModInfoBuilder::new(info.name.as_ref());
-    if let Some(author) = info.author {
-        modinfo.emit("author", &author);
+    if let Some(authors) = info.authors {
+        for author in authors {
+            modinfo.emit("authors", &author);
+        }
     }
     if let Some(description) = info.description {
         modinfo.emit("description", &description);
diff --git a/samples/rust/rust_driver_faux.rs b/samples/rust/rust_driver_faux.rs
index 048c6cb98b29..378bab4b587d 100644
--- a/samples/rust/rust_driver_faux.rs
+++ b/samples/rust/rust_driver_faux.rs
@@ -7,7 +7,7 @@
 module! {
     type: SampleModule,
     name: "rust_faux_driver",
-    author: "Lyude Paul",
+    authors: ["Lyude Paul"],
     description: "Rust faux device sample",
     license: "GPL",
 }
diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs
index 1fb6e44f3395..364a0660a743 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",
+    authors: ["Danilo Krummrich"],
     description: "Rust PCI driver",
     license: "GPL v2",
 }
diff --git a/samples/rust/rust_driver_platform.rs b/samples/rust/rust_driver_platform.rs
index 8120609e2940..f7a0f1b29d1d 100644
--- a/samples/rust/rust_driver_platform.rs
+++ b/samples/rust/rust_driver_platform.rs
@@ -43,7 +43,7 @@ fn drop(&mut self) {
 kernel::module_platform_driver! {
     type: SampleDriver,
     name: "rust_driver_platform",
-    author: "Danilo Krummrich",
+    authors: ["Danilo Krummrich"],
     description: "Rust Platform driver",
     license: "GPL v2",
 }
diff --git a/samples/rust/rust_minimal.rs b/samples/rust/rust_minimal.rs
index 4aaf117bf8e3..1fc7a1be6b6d 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",
+    authors: ["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..d3785e7c0330 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",
+    authors: ["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..8ea95e8c2f36 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",
+    authors: ["Rust for Linux Contributors"],
     description: "Rust printing macros sample",
     license: "GPL",
 }
-- 
2.34.1


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

* [PATCH V4 2/2] checkpatch: throw error for malformed arrays
  2025-02-21 18:21 [PATCH V4 0/2] author field in module! macro should be a array Guilherme Giacomo Simoes
  2025-02-21 18:21 ` [PATCH V4 1/2] rust: module: change author to an array Guilherme Giacomo Simoes
@ 2025-02-21 18:21 ` Guilherme Giacomo Simoes
  2025-02-22  9:22   ` Joe Perches
  1 sibling, 1 reply; 5+ messages in thread
From: Guilherme Giacomo Simoes @ 2025-02-21 18:21 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

Implement a check to ensure these fields are correctly formatted. If a
line contains one of these keys that should be of type Vec<String>, use
a regex to verify whether the array holds multiple values.
* If the array contains more than one value, enforce vertical formatting
* If the array has only one value, it can remain on 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..21a5ef4fd8b8 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(authors|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 = "$here\n" . cat_vet($rawline) . "\n";
+
+			my $counter = () = $line =~ /"/g;
+			my $more_than_one = $counter > 2;
+			if ($more_than_one) {
+				WARN("ARRAY_MODULE_MACRO",
+				"Prefer each array element on a separate line\n$herevet");
+			} elsif ($inline && $line !~ /\]/ && $line !~ /,/ && $line =~ /"/) {
+				WARN("ARRAY_MODULE_MACRO",
+				"Prefer declare ] on the same line\n$herevet");
+			} elsif (!$inline && $line =~ /\]/ && $line =~ /\"/) {
+				WARN("ARRAY_MODULE_MACRO",
+				"Prefer a new line after the last value and before ]\n$herevet");
+			} elsif ($inline && $line =~ /,/ && $line !~ /\]/) {
+				WARN("ARRAY_MODULE_MACRO",
+				"Prefer a new line after [\n$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] 5+ messages in thread

* Re: [PATCH V4 2/2] checkpatch: throw error for malformed arrays
  2025-02-21 18:21 ` [PATCH V4 2/2] checkpatch: throw error for malformed arrays Guilherme Giacomo Simoes
@ 2025-02-22  9:22   ` Joe Perches
  2025-02-22 13:30     ` Guilherme Giacomo Simoes
  0 siblings, 1 reply; 5+ messages in thread
From: Joe Perches @ 2025-02-22  9:22 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-21 at 15:21 -0300, Guilherme Giacomo Simoes wrote:
> Implement a check to ensure these fields are correctly formatted. If a
> line contains one of these keys that should be of type Vec<String>, use
> a regex to verify whether the array holds multiple values.
> * If the array contains more than one value, enforce vertical formatting
> * If the array has only one value, it can remain on the same line

Malformed arrays of what type?

[]
> diff --git 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

check if what kind of array otherwise this comment is very poor.
Arrays commonly have more than one value assigned on a single line like

	int foo[] = { 1, 2, 3 };

> +		my $inline = 0;
> +		my $key = "";
> +		my $add_line = $line =~ /^\+/;
> +
> +		if ($line =~ /\s*.*\b(authors|alias|firmware)\s*:\s*\[/) {

The initial "\s*.*" portion of this regex isn't necessary
and can be removed
=
> +			$inline = 1;
> +			$array_parse_module{$1} = 1;
> +		}
> +
> +		my @keys = keys %array_parse_module;
> +		if (@keys) {
> +			$key = $keys[0];
> +		}
> +
> +		if ($add_line && $key) {
> +			my $herevet = "$here\n" . cat_vet($rawline) . "\n";
> +
> +			my $counter = () = $line =~ /"/g;
> +			my $more_than_one = $counter > 2;
> +			if ($more_than_one) {
> +				WARN("ARRAY_MODULE_MACRO",
> +				"Prefer each array element on a separate line\n$herevet");

As I wrote in an earlier submission, please align to open parenthesis
and use the common form for the message with $herevet

				WARN("ARRAY_MODULE_MACRO",
				     "Prefer each array element on a separate line\n" . $herevet);

> +			} elsif ($inline && $line !~ /\]/ && $line !~ /,/ && $line =~ /"/) {
> +				WARN("ARRAY_MODULE_MACRO",
> +				"Prefer declare ] on the same line\n$herevet");

etc...


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

* Re: [PATCH V4 2/2] checkpatch: throw error for malformed arrays
  2025-02-22  9:22   ` Joe Perches
@ 2025-02-22 13:30     ` Guilherme Giacomo Simoes
  0 siblings, 0 replies; 5+ messages in thread
From: Guilherme Giacomo Simoes @ 2025-02-22 13:30 UTC (permalink / raw)
  To: joe
  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,
	linux-kernel, lukas.bulwahn, ojeda, pbonzini, rust-for-linux,
	tmgross, trintaeoitogc, walmeida

Joe Perches <joe@perches.com> wrote:
> check if what kind of array otherwise this comment is very poor.
> Arrays commonly have more than one value assigned on a single line like
yes, you is right, I will send a v5

> The initial "\s*.*" portion of this regex isn't necessary
> and can be removed
I will make a test without "\s*.*" and working I will send this change in a v5

> As I wrote in an earlier submission, please align to open parenthesis
> and use the common form for the message with $herevet
Okay, this will be changed.

Thanks,
Guilherme

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

end of thread, other threads:[~2025-02-22 13:30 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-21 18:21 [PATCH V4 0/2] author field in module! macro should be a array Guilherme Giacomo Simoes
2025-02-21 18:21 ` [PATCH V4 1/2] rust: module: change author to an array Guilherme Giacomo Simoes
2025-02-21 18:21 ` [PATCH V4 2/2] checkpatch: throw error for malformed arrays Guilherme Giacomo Simoes
2025-02-22  9:22   ` Joe Perches
2025-02-22 13:30     ` Guilherme Giacomo Simoes

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).