* [PATCH V7 0/2] author field in module! macro should be a array
@ 2025-03-07 13:13 Guilherme Giacomo Simoes
2025-03-07 13:14 ` [PATCH V7 1/2] rust: module: change author to an array Guilherme Giacomo Simoes
2025-03-07 13:14 ` [PATCH V7 2/2] checkpatch: check format of Vec<String> in modules Guilherme Giacomo Simoes
0 siblings, 2 replies; 11+ messages in thread
From: Guilherme Giacomo Simoes @ 2025-03-07 13:13 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, charmitro
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!
---
V7 changes
- Add in checkpatch a check for vertical arrays not align
---
Guilherme Giacomo Simoes (2):
rust: module: change author to an array
checkpatch: check format of Vec<String> in modules
drivers/block/rnull.rs | 2 +-
drivers/net/phy/ax88796b_rust.rs | 2 +-
drivers/net/phy/qt2025.rs | 2 +-
rust/kernel/net/phy.rs | 4 +-
rust/kernel/pci.rs | 2 +-
rust/kernel/platform.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 | 67 ++++++++++++++++++++++++++++
15 files changed, 90 insertions(+), 21 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH V7 1/2] rust: module: change author to an array
2025-03-07 13:13 [PATCH V7 0/2] author field in module! macro should be a array Guilherme Giacomo Simoes
@ 2025-03-07 13:14 ` Guilherme Giacomo Simoes
2025-03-07 13:26 ` Alice Ryhl
2025-03-07 13:14 ` [PATCH V7 2/2] checkpatch: check format of Vec<String> in modules Guilherme Giacomo Simoes
1 sibling, 1 reply; 11+ messages in thread
From: Guilherme Giacomo Simoes @ 2025-03-07 13:14 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, charmitro
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 +-
drivers/net/phy/ax88796b_rust.rs | 2 +-
drivers/net/phy/qt2025.rs | 2 +-
rust/kernel/net/phy.rs | 4 ++--
rust/kernel/pci.rs | 2 +-
rust/kernel/platform.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 +-
14 files changed, 23 insertions(+), 21 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/drivers/net/phy/ax88796b_rust.rs b/drivers/net/phy/ax88796b_rust.rs
index 8c7eb009d9fc..bc73ebccc2aa 100644
--- a/drivers/net/phy/ax88796b_rust.rs
+++ b/drivers/net/phy/ax88796b_rust.rs
@@ -19,7 +19,7 @@
DeviceId::new_with_driver::<PhyAX88796B>()
],
name: "rust_asix_phy",
- author: "FUJITA Tomonori <fujita.tomonori@gmail.com>",
+ authors: ["FUJITA Tomonori <fujita.tomonori@gmail.com>"],
description: "Rust Asix PHYs driver",
license: "GPL",
}
diff --git a/drivers/net/phy/qt2025.rs b/drivers/net/phy/qt2025.rs
index 1ab065798175..520daeb42089 100644
--- a/drivers/net/phy/qt2025.rs
+++ b/drivers/net/phy/qt2025.rs
@@ -26,7 +26,7 @@
phy::DeviceId::new_with_driver::<PhyQT2025>(),
],
name: "qt2025_phy",
- author: "FUJITA Tomonori <fujita.tomonori@gmail.com>",
+ authors: ["FUJITA Tomonori <fujita.tomonori@gmail.com>"],
description: "AMCC QT2025 PHY driver",
license: "GPL",
firmware: ["qt2025-2.0.3.3.fw"],
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/kernel/platform.rs b/rust/kernel/platform.rs
index 50e6b0421813..1297f5292ba9 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -101,7 +101,7 @@ fn of_id_table() -> Option<of::IdTable<Self::IdInfo>> {
/// kernel::module_platform_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] 11+ messages in thread
* [PATCH V7 2/2] checkpatch: check format of Vec<String> in modules
2025-03-07 13:13 [PATCH V7 0/2] author field in module! macro should be a array Guilherme Giacomo Simoes
2025-03-07 13:14 ` [PATCH V7 1/2] rust: module: change author to an array Guilherme Giacomo Simoes
@ 2025-03-07 13:14 ` Guilherme Giacomo Simoes
2025-03-08 18:30 ` Daniel Sedlak
1 sibling, 1 reply; 11+ messages in thread
From: Guilherme Giacomo Simoes @ 2025-03-07 13:14 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, charmitro
Cc: trintaeoitogc, rust-for-linux, linux-kernel
Implement a check to ensure that the author, firmware, and alias fields
of the module! macro are properly formatted.
* If the array contains more than one value, enforce vertical
formatting.
* If the array contains only one value, it may be formatted on a single
line
Signed-off-by: Guilherme Giacomo Simoes <trintaeoitogc@gmail.com>
---
scripts/checkpatch.pl | 67 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 67 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7b28ad331742..7c42c1a0ea6b 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2775,6 +2775,12 @@ sub process {
$realcnt = 0;
$linenr = 0;
$fixlinenr = -1;
+
+ my %array_parse_module;
+ my $expected_spaces;
+ my $spaces;
+ my $herevet_space_add;
+
foreach my $line (@lines) {
$linenr++;
$fixlinenr++;
@@ -3567,6 +3573,67 @@ sub process {
# ignore non-hunk lines and lines being removed
next if (!$hunk_line || $line =~ /^-/);
+# check if the field is about author, firmware or alias from module! macro and find malformed arrays
+ my $inline = 0;
+ my $key = "";
+ my $add_line = $line =~ /^\+/;
+
+ if ($line =~ /\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 (!$expected_spaces && (!$add_line && $key && !$inline)) {
+ if ($line =~ /^([\t ]+)(\s)/) {
+ $expected_spaces = $1;
+ }
+ }
+
+ 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);
+ }
+
+ if ($line =~ /^\+\s*([\t ]+)(\S)/) {
+ $spaces = $1;
+ $herevet_space_add = $herevet;
+ }
+ }
+
+ if ($expected_spaces && $spaces) {
+ if (length($spaces) != length($expected_spaces)) {
+ WARN("ARRAY_MODULE_MACRO",
+ "Prefer a align parameters\n" . $herevet_space_add);
+ }
+
+ $spaces = undef;
+ }
+
+ #END OF ANALYZE FIELD
+ if ($line =~ /\]/) {
+ delete $array_parse_module{$key};
+ $expected_spaces = undef;
+ }
+
#trailing whitespace
if ($line =~ /^\+.*\015/) {
my $herevet = "$here\n" . cat_vet($rawline) . "\n";
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH V7 1/2] rust: module: change author to an array
2025-03-07 13:14 ` [PATCH V7 1/2] rust: module: change author to an array Guilherme Giacomo Simoes
@ 2025-03-07 13:26 ` Alice Ryhl
2025-03-07 15:43 ` Guilherme Giacomo Simoes
0 siblings, 1 reply; 11+ messages in thread
From: Alice Ryhl @ 2025-03-07 13:26 UTC (permalink / raw)
To: Guilherme Giacomo Simoes
Cc: a.hindborg, alex.gaynor, 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, charmitro,
rust-for-linux, linux-kernel, Miguel Ojeda
On Fri, Mar 07, 2025 at 10:14:00AM -0300, Guilherme Giacomo Simoes wrote:
> 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>
I believe I reviewed this on the previous version. Did you forget to
pick up my tag, or did you change something in this patch that requires
a re-review?
Alice
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V7 1/2] rust: module: change author to an array
2025-03-07 13:26 ` Alice Ryhl
@ 2025-03-07 15:43 ` Guilherme Giacomo Simoes
2025-03-07 16:24 ` Miguel Ojeda
0 siblings, 1 reply; 11+ messages in thread
From: Guilherme Giacomo Simoes @ 2025-03-07 15:43 UTC (permalink / raw)
To: aliceryhl
Cc: a.hindborg, alex.gaynor, apw, arnd, aswinunni01, axboe,
benno.lossin, bhelgaas, bjorn3_gh, boqun.feng, charmitro, dakr,
dwaipayanray1, ethan.twardy, fujita.tomonori, gary, gregkh, joe,
linux-kernel, lukas.bulwahn, miguel.ojeda.sandonis, ojeda,
pbonzini, rust-for-linux, tmgross, trintaeoitogc, walmeida
Alice Ryhl <aliceryhl@google.com> wrote:
> I believe I reviewed this on the previous version. Did you forget to
> pick up my tag, or did you change something in this patch that requires
> a re-review?
I'm sorry, I will pick up your tag and send a v8
Too, I will pick up Charalampos and Andreas tags
Thanks,
Guilherme
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V7 1/2] rust: module: change author to an array
2025-03-07 15:43 ` Guilherme Giacomo Simoes
@ 2025-03-07 16:24 ` Miguel Ojeda
2025-03-08 0:40 ` Guilherme Giacomo Simoes
0 siblings, 1 reply; 11+ messages in thread
From: Miguel Ojeda @ 2025-03-07 16:24 UTC (permalink / raw)
To: Guilherme Giacomo Simoes
Cc: aliceryhl, a.hindborg, alex.gaynor, apw, arnd, aswinunni01, axboe,
benno.lossin, bhelgaas, bjorn3_gh, boqun.feng, charmitro, dakr,
dwaipayanray1, ethan.twardy, fujita.tomonori, gary, gregkh, joe,
linux-kernel, lukas.bulwahn, ojeda, pbonzini, rust-for-linux,
tmgross, walmeida
On Fri, Mar 7, 2025 at 4:43 PM Guilherme Giacomo Simoes
<trintaeoitogc@gmail.com> wrote:
>
> I'm sorry, I will pick up your tag and send a v8
>
> Too, I will pick up Charalampos and Andreas tags
In general, if it is the last version, you don't need to pick up tags
-- maintainers will do that for you.
i.e. in this case, it would have been fine telling Alice that you
didn't change anything (if that is the case -- is it?)
Thanks!
Cheers,
Miguel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V7 1/2] rust: module: change author to an array
2025-03-07 16:24 ` Miguel Ojeda
@ 2025-03-08 0:40 ` Guilherme Giacomo Simoes
2025-03-09 21:03 ` Miguel Ojeda
0 siblings, 1 reply; 11+ messages in thread
From: Guilherme Giacomo Simoes @ 2025-03-08 0:40 UTC (permalink / raw)
To: miguel.ojeda.sandonis
Cc: a.hindborg, alex.gaynor, aliceryhl, apw, arnd, aswinunni01, axboe,
benno.lossin, bhelgaas, bjorn3_gh, boqun.feng, charmitro, dakr,
dwaipayanray1, ethan.twardy, fujita.tomonori, gary, gregkh, joe,
linux-kernel, lukas.bulwahn, ojeda, pbonzini, rust-for-linux,
tmgross, trintaeoitogc, walmeida
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:
> In general, if it is the last version, you don't need to pick up tags
> -- maintainers will do that for you.
>
> i.e. in this case, it would have been fine telling Alice that you
> didn't change anything (if that is the case -- is it?)
Yes, this is the case, didn't change anything in this V8 in the rust side
Well, now I already add the reviewers tag, I think that is not a problmem here,
right?
Thanks,
Guilherme
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V7 2/2] checkpatch: check format of Vec<String> in modules
2025-03-07 13:14 ` [PATCH V7 2/2] checkpatch: check format of Vec<String> in modules Guilherme Giacomo Simoes
@ 2025-03-08 18:30 ` Daniel Sedlak
2025-03-09 17:47 ` Guilherme Giacomo Simoes
0 siblings, 1 reply; 11+ messages in thread
From: Daniel Sedlak @ 2025-03-08 18:30 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, charmitro
Cc: rust-for-linux, linux-kernel
On 3/7/25 2:14 PM, Guilherme Giacomo Simoes wrote:
> Implement a check to ensure that the author, firmware, and alias fields
> of the module! macro are properly formatted.
>
> * If the array contains more than one value, enforce vertical
> formatting.
> * If the array contains only one value, it may be formatted on a single
> line
>
> Signed-off-by: Guilherme Giacomo Simoes <trintaeoitogc@gmail.com>
> ---
> scripts/checkpatch.pl | 67 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 67 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 7b28ad331742..7c42c1a0ea6b 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2775,6 +2775,12 @@ sub process {
> $realcnt = 0;
> $linenr = 0;
> $fixlinenr = -1;
> +
> + my %array_parse_module;
> + my $expected_spaces;
> + my $spaces;
> + my $herevet_space_add;
> +
> foreach my $line (@lines) {
> $linenr++;
> $fixlinenr++;
> @@ -3567,6 +3573,67 @@ sub process {
> # ignore non-hunk lines and lines being removed
> next if (!$hunk_line || $line =~ /^-/);
>
> +# check if the field is about author, firmware or alias from module! macro and find malformed arrays
> + my $inline = 0;
> + my $key = "";
> + my $add_line = $line =~ /^\+/;
> +
> + if ($line =~ /\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 (!$expected_spaces && (!$add_line && $key && !$inline)) {
> + if ($line =~ /^([\t ]+)(\s)/) {
> + $expected_spaces = $1;
> + }
> + }
> +
> + 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);
Small grammar nit. "Prefer to declare…"
> + } 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);
> + }
> +
> + if ($line =~ /^\+\s*([\t ]+)(\S)/) {
> + $spaces = $1;
> + $herevet_space_add = $herevet;
> + }
> + }
> +
> + if ($expected_spaces && $spaces) {
> + if (length($spaces) != length($expected_spaces)) {
> + WARN("ARRAY_MODULE_MACRO",
> + "Prefer a align parameters\n" . $herevet_space_add);
Small grammar nit. Shouldn't this be rather: "Prefer to align
parameters" or "Prefer aligned parameters"?
> + }
> +
> + $spaces = undef;
> + }
> +
> + #END OF ANALYZE FIELD
> + if ($line =~ /\]/) {
> + delete $array_parse_module{$key};
> + $expected_spaces = undef;
> + }
> +
> #trailing whitespace
> if ($line =~ /^\+.*\015/) {
> my $herevet = "$here\n" . cat_vet($rawline) . "\n";
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V7 2/2] checkpatch: check format of Vec<String> in modules
2025-03-08 18:30 ` Daniel Sedlak
@ 2025-03-09 17:47 ` Guilherme Giacomo Simoes
0 siblings, 0 replies; 11+ messages in thread
From: Guilherme Giacomo Simoes @ 2025-03-09 17:47 UTC (permalink / raw)
To: daniel
Cc: a.hindborg, alex.gaynor, aliceryhl, apw, arnd, aswinunni01, axboe,
benno.lossin, bhelgaas, bjorn3_gh, boqun.feng, charmitro, dakr,
dwaipayanray1, ethan.twardy, fujita.tomonori, gary, gregkh, joe,
linux-kernel, lukas.bulwahn, ojeda, pbonzini, rust-for-linux,
tmgross, trintaeoitogc, walmeida
Daniel Sedlak <daniel@sedlak.dev> wrote:
> ThanksSmall grammar nit. "Prefer to declare…"
Okay, I will fix this and send a v9
> Small grammar nit. Shouldn't this be rather: "Prefer to align
> parameters" or "Prefer aligned parameters"?
"Prefere aligned parameters" seems better to me
Thanks,
Guilherme
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V7 1/2] rust: module: change author to an array
2025-03-08 0:40 ` Guilherme Giacomo Simoes
@ 2025-03-09 21:03 ` Miguel Ojeda
2025-03-10 11:53 ` Guilherme Giacomo Simoes
0 siblings, 1 reply; 11+ messages in thread
From: Miguel Ojeda @ 2025-03-09 21:03 UTC (permalink / raw)
To: Guilherme Giacomo Simoes
Cc: a.hindborg, alex.gaynor, aliceryhl, apw, arnd, aswinunni01, axboe,
benno.lossin, bhelgaas, bjorn3_gh, boqun.feng, charmitro, dakr,
dwaipayanray1, ethan.twardy, fujita.tomonori, gary, gregkh, joe,
linux-kernel, lukas.bulwahn, ojeda, pbonzini, rust-for-linux,
tmgross, walmeida
On Sat, Mar 8, 2025 at 1:40 AM Guilherme Giacomo Simoes
<trintaeoitogc@gmail.com> wrote:
>
> Yes, this is the case, didn't change anything in this V8 in the rust side
I think what Alice meant is if you changed something from v6 to v7
(not v8) -- from a quick look, I don't see any changes, so that is
good.
> Well, now I already add the reviewers tag, I think that is not a problmem here,
> right?
Yes, it is OK, no worries about it.
Please also avoid sending new versions for very small tweaks or very
frequently, since it gets confusing, e.g. I had in my queue to reply
to this message, which is v7 but v9 is already in the list... :)
Generally, you should wait a week, or at least a couple days, between
revisions (there are exceptions, of course, but this is not an urgent
series).
Thanks!
Cheers,
Miguel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V7 1/2] rust: module: change author to an array
2025-03-09 21:03 ` Miguel Ojeda
@ 2025-03-10 11:53 ` Guilherme Giacomo Simoes
0 siblings, 0 replies; 11+ messages in thread
From: Guilherme Giacomo Simoes @ 2025-03-10 11:53 UTC (permalink / raw)
To: miguel.ojeda.sandonis
Cc: a.hindborg, alex.gaynor, aliceryhl, apw, arnd, aswinunni01, axboe,
benno.lossin, bhelgaas, bjorn3_gh, boqun.feng, charmitro, dakr,
dwaipayanray1, ethan.twardy, fujita.tomonori, gary, gregkh, joe,
linux-kernel, lukas.bulwahn, ojeda, pbonzini, rust-for-linux,
tmgross, trintaeoitogc, walmeida
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:
> Please also avoid sending new versions for very small tweaks or very
> frequently, since it gets confusing, e.g. I had in my queue to reply
> to this message, which is v7 but v9 is already in the list... :)
>
> Generally, you should wait a week, or at least a couple days, between
> revisions (there are exceptions, of course, but this is not an urgent
> series).
Hm, ok. I will wait more in the next time.
Thanks,
Guilherme
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-03-10 11:54 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-07 13:13 [PATCH V7 0/2] author field in module! macro should be a array Guilherme Giacomo Simoes
2025-03-07 13:14 ` [PATCH V7 1/2] rust: module: change author to an array Guilherme Giacomo Simoes
2025-03-07 13:26 ` Alice Ryhl
2025-03-07 15:43 ` Guilherme Giacomo Simoes
2025-03-07 16:24 ` Miguel Ojeda
2025-03-08 0:40 ` Guilherme Giacomo Simoes
2025-03-09 21:03 ` Miguel Ojeda
2025-03-10 11:53 ` Guilherme Giacomo Simoes
2025-03-07 13:14 ` [PATCH V7 2/2] checkpatch: check format of Vec<String> in modules Guilherme Giacomo Simoes
2025-03-08 18:30 ` Daniel Sedlak
2025-03-09 17:47 ` 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).