From f9a866ad4847b0714cf8f240ad5681e2f6d1e08a Mon Sep 17 00:00:00 2001 From: Philipp Gesang Date: Wed, 5 Oct 2022 07:17:08 +0200 Subject: [PATCH] rust: macros: expose module parameters through MODPARAM struct Instead of creating a struct + constant pair for each parameter, expose the parameters as methods over a single struct `MODPARAM`. For example the parameter `foobar` becomes available at the Rust end as `MODPARAM.foobar()`. The rationale is that currently the `module!` macro generates a constant with the name of the parameter which can lead to surprising behavior when attempting to create a let binding of the same name: module! { type: ParamName, // ... params: { foobar : bool { default: true, permissions: 0o644, description: "test", }, }, } fn clash() { let foobar = 42; } This causes rustc to error out because `foobar` is treated as a pattern on the left hand side of the let expression: 10 | / module! { 11 | | type: ParamName, 12 | | name: "param_name", 13 | | author: "A. U. Thor", ... | 21 | | }, 22 | | } | |_- constant defined here ... 25 | let foobar = 42; | ^^^^^^ -- this expression has type `{integer}` | | | expected integer, found struct `__param_name_foobar` | `foobar` is interpreted as a constant, not a new binding | help: introduce a new binding instead: `other_foobar` Essentially, having a parameter `foobar` prevents the creation of a binding of the same name anywhere else in the module. Signed-off-by: Philipp Gesang Suggested-by: Gary Guo --- rust/macros/module.rs | 37 +++++++++++++++++++------- samples/rust/rust_module_parameters.rs | 10 +++---- 2 files changed, 33 insertions(+), 14 deletions(-) diff --git a/rust/macros/module.rs b/rust/macros/module.rs index 6001fd692469be..c22634efebd53c 100644 --- a/rust/macros/module.rs +++ b/rust/macros/module.rs @@ -329,6 +329,7 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream { assert_eq!(params.delimiter(), Delimiter::Brace); let mut it = params.stream().into_iter(); + let mut read_funcs = Vec::new(); loop { let param_name = match it.next() { @@ -388,7 +389,7 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream { let read_func = if permissions_are_readonly(¶m_permissions) { format!( " - fn read(&self) + fn {param_name}(&self) -> &<{param_type_internal} as kernel::module_param::ModuleParam>::Value {{ // SAFETY: Parameters do not need to be locked because they are // read only or sysfs is not enabled. @@ -406,7 +407,7 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream { } else { format!( " - fn read<'lck>(&self, lock: &'lck kernel::KParamGuard) + fn {param_name}<'lck>(&self, lock: &'lck kernel::KParamGuard) -> &'lck <{param_type_internal} as kernel::module_param::ModuleParam>::Value {{ // SAFETY: Parameters are locked by `KParamGuard`. unsafe {{ @@ -421,6 +422,8 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream { param_type_internal = param_type_internal, ) }; + read_funcs.push(read_func); + let kparam = format!( " kernel::bindings::kernel_param__bindgen_ty_1 {{ @@ -436,12 +439,6 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream { " static mut __{name}_{param_name}_value: {param_type_internal} = {param_default}; - struct __{name}_{param_name}; - - impl __{name}_{param_name} {{ {read_func} }} - - const {param_name}: __{name}_{param_name} = __{name}_{param_name}; - // Note: the C macro that generates the static structs for the `__param` section // asks for them to be `aligned(sizeof(void *))`. However, that was put in place // in 2003 in commit 38d5b085d2a0 (\"[PATCH] Fix over-alignment problem on x86-64\") @@ -483,7 +480,6 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream { ", name = info.name, param_type_internal = param_type_internal, - read_func = read_func, param_default = param_default, param_name = param_name, ops = ops, @@ -492,6 +488,29 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream { ) .unwrap(); } + + write!( + modinfo.buffer, + " + struct __MODPARAM; + + impl __MODPARAM {{ + " + ) + .unwrap(); + + read_funcs.iter().for_each(|read_func| { + writeln!(modinfo.buffer, "{}", read_func).unwrap(); + }); + + write!( + modinfo.buffer, + "}} + + const MODPARAM: __MODPARAM = __MODPARAM; + " + ) + .unwrap(); } let mut generated_array_types = String::new(); diff --git a/samples/rust/rust_module_parameters.rs b/samples/rust/rust_module_parameters.rs index 557cba7b4815d1..2804289660fa45 100644 --- a/samples/rust/rust_module_parameters.rs +++ b/samples/rust/rust_module_parameters.rs @@ -48,14 +48,14 @@ impl kernel::Module for RustModuleParameters { { let lock = module.kernel_param_lock(); pr_info!("Parameters:\n"); - pr_info!(" my_bool: {}\n", my_bool.read()); - pr_info!(" my_i32: {}\n", my_i32.read(&lock)); + pr_info!(" my_bool: {}\n", MODPARAM.my_bool()); + pr_info!(" my_i32: {}\n", MODPARAM.my_i32(&lock)); pr_info!( " my_str: {}\n", - core::str::from_utf8(my_str.read(&lock))? + core::str::from_utf8(MODPARAM.my_str(&lock))? ); - pr_info!(" my_usize: {}\n", my_usize.read(&lock)); - pr_info!(" my_array: {:?}\n", my_array.read()); + pr_info!(" my_usize: {}\n", MODPARAM.my_usize(&lock)); + pr_info!(" my_array: {:?}\n", MODPARAM.my_array()); } Ok(RustModuleParameters)