[PATCH 2/3] rust: macros: change author field to array

[PATCH 2/3] rust: macros: change author field to array

[Guilherme Giacomo Simoes]

The author field don't accept multiples authors. Since this is
necessary, in commit 84703117fd98 ("rust: module: change author to be a
array") was change the author field for accept a string array.
In modules that use the author field, change its value to a string
array.

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 ++--
 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 +-
 8 files changed, 10 insertions(+), 10 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/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

[PATCH 3/3] checkpatch: throw error in malformed arrays

[Guilherme Giacomo Simoes]

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 | 68 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7b28ad331742..d45536c2c63a 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2775,6 +2775,9 @@ sub process {
 	$realcnt = 0;
 	$linenr = 0;
 	$fixlinenr = -1;
+
+	my %parse_module_arr;
+
 	foreach my $line (@lines) {
 		$linenr++;
 		$fixlinenr++;
@@ -3567,6 +3570,71 @@ 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;
+		if ($line =~ /^\+\s*.*\b(author|alias|firmware)\s*:\s*\[/) {
+			$inline = 1;
+
+			if ($line =~ /author/) {
+				$parse_module_arr{'author'} = 1;
+			}
+
+			if ($line =~ /alias/) {
+				$parse_module_arr{'alias'} = 1;
+			}
+
+			if ($line =~ /firmware/) {
+				$parse_module_arr{'firmware'} = 1;
+			}
+		}
+
+		if (keys %parse_module_arr) {
+			my @keys = keys %parse_module_arr;
+			my $key = $keys[0];
+
+			my $more_than_one_vl = qr/".+",\s*".+"/;
+
+			if ($line =~ $more_than_one_vl) {
+					ERROR("ERR_ARRAY_MODULE_MACRO",
+						"the key $key have more than one values in same line\n$here\n". cat_vet($rawline));
+			}
+			elsif ($inline && $line !~ /\]/ && $line =~ /\["/) {
+					ERROR("ERR_ARRAY_MODULE_MACRO",
+						qq~the key $key need a new line after declare the key\n$here\n~ . cat_vet($rawline) . qq~\nFor example:
+
+FROM
+'example': ['value',
+	    'value'
+
+TO
+'example': [
+	'value',
+	'value
+
+	~);
+			}
+			elsif (!$inline && $line =~ /"\]/) {
+					ERROR("ERR_ARRAY_MODULE_MACRO",
+						qq~the key $key need a new line after last value and before ]\n$here\n~ . cat_vet($rawline) . qq~\nFor example:
+
+FROM
+'last_value']
+
+TO
+'last_value'
+]
+	~);
+			}
+
+			#END OF ANALYZE FIELD
+			if ((!$inline || $line !~ $more_than_one_vl) && $line =~ /\]/) {
+				delete $parse_module_arr{author};
+				delete $parse_module_arr{alias};
+				delete $parse_module_arr{firmware};
+			}
+		}
+
+
 #trailing whitespace
 		if ($line =~ /^\+.*\015/) {
 			my $herevet = "$here\n" . cat_vet($rawline) . "\n";
-- 
2.34.1

Re: [PATCH 3/3] checkpatch: throw error in malformed arrays

[Joe Perches]

On Wed, 2025-02-12 at 16:48 -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.

Isn't this is a style desire not a parsing limitation?

> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -2775,6 +2775,9 @@ sub process {
>  	$realcnt = 0;
>  	$linenr = 0;
>  	$fixlinenr = -1;
> +
> +	my %parse_module_arr;

Probably not useful to add hungarian style naming.

> @@ -3567,6 +3570,71 @@ 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;
> +		if ($line =~ /^\+\s*.*\b(author|alias|firmware)\s*:\s*\[/) {

\s*.* could be just .*

> +			$inline = 1;
> +
> +			if ($line =~ /author/) {
> +				$parse_module_arr{'author'} = 1;
> +			}
> +
> +			if ($line =~ /alias/) {
> +				$parse_module_arr{'alias'} = 1;
> +			}
> +
> +			if ($line =~ /firmware/) {
> +				$parse_module_arr{'firmware'} = 1;
> +			}

$parse_module_arr{$1} = 1;

> +		}
> +
> +		if (keys %parse_module_arr) {
> +			my @keys = keys %parse_module_arr;
> +			my $key = $keys[0];
> +
> +			my $more_than_one_vl = qr/".+",\s*".+"/;
> +
> +			if ($line =~ $more_than_one_vl) {
> +					ERROR("ERR_ARRAY_MODULE_MACRO",
> +						"the key $key have more than one values in same line\n$here\n". cat_vet($rawline));

"Prefer one value per line\n"

ERR_ error prefixes are not useful.
These seem more like a WARN than an ERROR
Bad indentation
And please use a $herevet temporary


> +			}
> +			elsif ($inline && $line !~ /\]/ && $line =~ /\["/) {
> +					ERROR("ERR_ARRAY_MODULE_MACRO",
> +						qq~the key $key need a new line after declare the key\n$here\n~ . cat_vet($rawline) . qq~\nFor example:

Use } elsif

"Prefer declaring keys on separate lines"

Are quote characters allowed in keys?
If not, this seems unnecessarily complicated.

$herevet

I think the qq is unnecessary and the for example
should refer to some style documentation and not
be explicit in checkpatch.

Re: [PATCH 3/3] checkpatch: throw error in malformed arrays

[Guilherme Giacomo Simoes]

Joe Perches <joe@perches.com> wrotes:
> Isn't this is a style desire not a parsing limitation?
Yes.

> Probably not useful to add hungarian style naming.
hungarian style naminng ? Do you think that is better `array_parse_module` ?

> \s*.* could be just .*
> 
> > +			$inline = 1;
> > +
> > +			if ($line =~ /author/) {
> > +				$parse_module_arr{'author'} = 1;
> > +			}
> > +
> > +			if ($line =~ /alias/) {
> > +				$parse_module_arr{'alias'} = 1;
> > +			}
> > +
> > +			if ($line =~ /firmware/) {
> > +				$parse_module_arr{'firmware'} = 1;
> > +			}
> 
> $parse_module_arr{$1} = 1;
hmm, ok thanks.

> > +			if ($line =~ $more_than_one_vl) {
> > +					ERROR("ERR_ARRAY_MODULE_MACRO",
> > +						"the key $key have more than one values in same line\n$here\n". cat_vet($rawline));
> 
> "Prefer one value per line\n"
> 
> ERR_ error prefixes are not useful.
> These seem more like a WARN than an ERROR
> Bad indentation
> And please use a $herevet temporary
> 
> 
> > +			}
> > +			elsif ($inline && $line !~ /\]/ && $line =~ /\["/) {
> > +					ERROR("ERR_ARRAY_MODULE_MACRO",
> > +						qq~the key $key need a new line after declare the key\n$here\n~ . cat_vet($rawline) . qq~\nFor example:
> 
> Use } elsif
> 
> "Prefer declaring keys on separate lines"
> 
> Are quote characters allowed in keys?
> If not, this seems unnecessarily complicated.
> 
> $herevet
> 
> I think the qq is unnecessary and the for example
> should refer to some style documentation and not
> be explicit in checkpatch.
Okay, I will submit a v2 with your hints

Thanks,
Guilherme

Re: [PATCH 2/3] rust: macros: change author field to array

[Gary Guo]

On Wed, 12 Feb 2025 16:48:56 -0300
Guilherme Giacomo Simoes <trintaeoitogc@gmail.com> wrote:

> The author field don't accept multiples authors. Since this is
> necessary, in commit 84703117fd98 ("rust: module: change author to be a
> array") was change the author field for accept a string array.
> In modules that use the author field, change its value to a string
> array.

Spelling out commit SHA doesn't work for patches because it will change
when applied.

Also, this patch needs to be the same commit as the one changing
macros, otherwise the tree won't compile when bisecting.

Best,
Gary

> 
> 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 ++--
>  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 +-
>  8 files changed, 10 insertions(+), 10 deletions(-)

Re: [PATCH 2/3] rust: macros: change author field to array

[Guilherme Giacomo Simoes]

Gary Guo <gary@garyguo.net> wrotes:
> Spelling out commit SHA doesn't work for patches because it will change
> when applied.
Hmm, I was think that they would stayed the same

> Also, this patch needs to be the same commit as the one changing
> macros, otherwise the tree won't compile when bisecting.
This really make sense, I will send a v2

Thanks,
Guilherme