On 02/11/2025 10.56, Uwe Kleine-König wrote: > Hello Daniel, > > [Adding Stephen and linux-next to Cc] > > On Sat, Nov 01, 2025 at 10:39:08PM +0100, Daniel Gomez wrote: >> On 24/09/2025 14.39, Andreas Hindborg wrote: >>> Extend the `module!` macro with support module parameters. Also add some >>> string to integer parsing functions. >>> >>> Based on the original module parameter support by Miguel [1], >>> later extended and generalized by Adam for more types [2][3]. >>> Originally tracked at [4]. >>> >>> Link: https://github.com/Rust-for-Linux/linux/pull/7 [1] >>> Link: https://github.com/Rust-for-Linux/linux/pull/82 [2] >>> Link: https://github.com/Rust-for-Linux/linux/pull/87 [3] >>> Link: https://github.com/Rust-for-Linux/linux/issues/11 [4] >>> Signed-off-by: Andreas Hindborg <[email protected]> >> >> I tested this series with rust_minimal module. They LGTM, >> >> Tested-by: Daniel Gomez <[email protected]> >> >> The patches did not apply cleanly to v6.18-rc3, at least not when using b4. >> However, when applying them to the base commit and then rebasing onto >> v6.18-rc3, >> I didn't see any conflicts. > > I don't know how you use b4, but > > git checkout v6.18-rc3 > b4 am -3 [email protected] > git am -3 > ./v18_20250924_a_hindborg_rust_extend_module_macro_with_integer_parameter_support.mbx > > works fine on my end. Using `-3` should have the same effect as applying > the series on top of the original base and rebase it.
Right, that's what I did but manually. I didn't know about that argument :). > > git fetch > https://git.kernel.org/pub/scm/linux/kernel/git/modules/linux.git > rebase/[email protected] > git range-diff FETCH_HEAD...HEAD > > confirms that. > >> I've created a temporary branch with this rebase here: >> >> https://git.kernel.org/pub/scm/linux/kernel/git/modules/linux.git/log/?h=rebase/[email protected] >> >> Can you take a look when you can? I'll merge this shortly after checking with >> Uwe, as there are some minor conflicts with his tree. >> >> + Uwe >> >> These are the conflicts I see when merging the patch series from Michal [1] >> (Introduce import_ns support for Rust). I believe these are trivial things >> that >> we will get notified from linux-next merging. But let me know what you think >> as >> you have requested in that thread. >> >> [1] Link: >> https://lore.kernel.org/all/[email protected]/ > > Yeah, I expect that Stephen will highlight the conflicts, but I prefer > to not be surprised by that and consider linux-next more a fallback > security net that I don't want to use. I like it to be the other way > round and tell Stephen about conflicts to expect :-) Please Stephen, check the proposed changes below. I plan to merge this series in modules' tree and it will conflict with Uwe's tree on rust/macros/module.rs file. > >> ... >> Applying: rust: macros: Add support for 'imports_ns' to module! >> Patch failed at 0008 rust: macros: Add support for 'imports_ns' to module! >> error: patch failed: rust/macros/module.rs:98 >> error: rust/macros/module.rs: patch does not apply >> hint: Use 'git am --show-current-patch=diff' to see the failed patch >> hint: When you have resolved this problem, run "git am --continue". >> hint: If you prefer to skip this patch, run "git am --skip" instead. >> hint: To restore the original branch and stop patching, run "git am --abort". >> hint: Disable this message with "git config set advice.mergeConflict false" >> >> git am --show-current-patch=diff > > That command shows the patch to apply, but not the conflict, let alone > your resolution. > >> --- >> rust/macros/module.rs | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> --- >> rust/macros/module.rs | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/rust/macros/module.rs b/rust/macros/module.rs >> index >> 5ee54a00c0b65699596e660b2d4d60e64be2a50c..408cd115487514c8be79724d901c676435696376 >> 100644 >> --- a/rust/macros/module.rs >> +++ b/rust/macros/module.rs >> @@ -98,6 +98,7 @@ struct ModuleInfo { >> description: Option<String>, >> alias: Option<Vec<String>>, >> firmware: Option<Vec<String>>, >> + imports_ns: Option<Vec<String>>, >> } > > So here the addition of `params` is missing. > >> [...] > > When I merge your branch mentioned above with my pwm/for-next and > resolve the merge conflicts, the resolution looks as follows. The only > non-trivial thing is that > > if let Some(imports) = info.imports_ns { > > now needs a & for `info`. Correct. In case it's necessary, I've merged your changes into the modules's -next branch and attach the diff for you and Stephen. Not sure which order trees are taken/merged, though. > > Best regards > Uwe > > diff --cc rust/macros/module.rs > index d62e9c1e2a89,408cd1154875..000000000000 > --- a/rust/macros/module.rs > +++ b/rust/macros/module.rs > @@@ -205,50 -98,7 +205,51 @@@ struct ModuleInfo > description: Option<String>, > alias: Option<Vec<String>>, > firmware: Option<Vec<String>>, > + imports_ns: Option<Vec<String>>, > + params: Option<Vec<Parameter>>, > +} > + > +#[derive(Debug)] > +struct Parameter { > + name: String, > + ptype: String, > + default: String, > + description: String, > +} > + > +fn expect_params(it: &mut token_stream::IntoIter) -> Vec<Parameter> { > + let params = expect_group(it); > + assert_eq!(params.delimiter(), Delimiter::Brace); > + let mut it = params.stream().into_iter(); > + let mut parsed = Vec::new(); > + > + loop { > + let param_name = match it.next() { > + Some(TokenTree::Ident(ident)) => ident.to_string(), > + Some(_) => panic!("Expected Ident or end"), > + None => break, > + }; > + > + assert_eq!(expect_punct(&mut it), ':'); > + let param_type = expect_ident(&mut it); > + let group = expect_group(&mut it); > + assert_eq!(group.delimiter(), Delimiter::Brace); > + assert_eq!(expect_punct(&mut it), ','); > + > + let mut param_it = group.stream().into_iter(); > + let param_default = expect_param_default(&mut param_it); > + let param_description = expect_string_field(&mut param_it, > "description"); > + expect_end(&mut param_it); > + > + parsed.push(Parameter { > + name: param_name, > + ptype: param_type, > + default: param_default, > + description: param_description, > + }) > + } > + > + parsed > } > > impl ModuleInfo { > @@@ -263,7 -113,7 +264,8 @@@ > "license", > "alias", > "firmware", > + "imports_ns", > + "params", > ]; > const REQUIRED_KEYS: &[&str] = &["type", "name", "license"]; > let mut seen_keys = Vec::new(); > @@@ -289,7 -139,7 +291,8 @@@ > "license" => info.license = expect_string_ascii(it), > "alias" => info.alias = Some(expect_string_array(it)), > "firmware" => info.firmware = Some(expect_string_array(it)), > + "imports_ns" => info.imports_ns = > Some(expect_string_array(it)), > + "params" => info.params = Some(expect_params(it)), > _ => panic!("Unknown key \"{key}\". Valid keys are: > {EXPECTED_KEYS:?}."), > } > > @@@ -329,25 -179,30 +332,30 @@@ pub(crate) fn module(ts: TokenStream) - > // Rust does not allow hyphens in identifiers, use underscore instead. > let ident = info.name.replace('-', "_"); > let mut modinfo = ModInfoBuilder::new(ident.as_ref()); > - if let Some(authors) = info.authors { > + if let Some(authors) = &info.authors { > for author in authors { > - modinfo.emit("author", &author); > + modinfo.emit("author", author); > } > } > - if let Some(description) = info.description { > - modinfo.emit("description", &description); > + if let Some(description) = &info.description { > + modinfo.emit("description", description); > } > modinfo.emit("license", &info.license); > - if let Some(aliases) = info.alias { > + if let Some(aliases) = &info.alias { > for alias in aliases { > - modinfo.emit("alias", &alias); > + modinfo.emit("alias", alias); > } > } > - if let Some(firmware) = info.firmware { > + if let Some(firmware) = &info.firmware { > for fw in firmware { > - modinfo.emit("firmware", &fw); > + modinfo.emit("firmware", fw); > } > } > - if let Some(imports) = info.imports_ns { > ++ if let Some(imports) = &info.imports_ns { > + for ns in imports { > + modinfo.emit("import_ns", &ns); > + } > + } > > // Built-in modules also export the `file` modinfo string. > let file = The resolution looks good to me. As I applied Michal's patch on top of Andreas changes, it looks like this on my side: diff --git a/rust/macros/module.rs b/rust/macros/module.rs index d62e9c1e2a89..5bf0a487de50 100644 --- a/rust/macros/module.rs +++ b/rust/macros/module.rs @@ -206,6 +206,7 @@ struct ModuleInfo { alias: Option<Vec<String>>, firmware: Option<Vec<String>>, params: Option<Vec<Parameter>>, + imports_ns: Option<Vec<String>>, } #[derive(Debug)] @@ -264,6 +265,7 @@ fn parse(it: &mut token_stream::IntoIter) -> Self { "alias", "firmware", "params", + "imports_ns", ]; const REQUIRED_KEYS: &[&str] = &["type", "name", "license"]; let mut seen_keys = Vec::new(); @@ -290,6 +292,7 @@ fn parse(it: &mut token_stream::IntoIter) -> Self { "alias" => info.alias = Some(expect_string_array(it)), "firmware" => info.firmware = Some(expect_string_array(it)), "params" => info.params = Some(expect_params(it)), + "imports_ns" => info.imports_ns = Some(expect_string_array(it)), _ => panic!("Unknown key \"{key}\". Valid keys are: {EXPECTED_KEYS:?}."), } @@ -348,6 +351,11 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream { modinfo.emit("firmware", fw); } } + if let Some(imports) = &info.imports_ns { + for ns in imports { + modinfo.emit("import_ns", &ns); + } + } // Built-in modules also export the `file` modinfo string. let file =
