From: sashiko-bot@kernel.org
To: "Andreas Hindborg" <a.hindborg@kernel.org>
Cc: linux-modules@vger.kernel.org, ojeda@kernel.org
Subject: Re: [PATCH] rust: module_param: return value by copy from `value`
Date: Fri, 29 May 2026 08:55:11 +0000 [thread overview]
Message-ID: <20260529085512.32BF01F00893@smtp.kernel.org> (raw)
In-Reply-To: <20260529-modules-value-ref-v1-1-a0df7e9e6f2d@kernel.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Missing `#[inline]` annotations on small abstraction methods `value` and `value_ref` in `ModuleParamAccess`.
--
commit 6103e7f86e2528da9d35cfda1b1a7d24ab1f6516
Author: Andreas Hindborg <a.hindborg@kernel.org>
rust: module_param: return value by copy from value
For Copy parameter types, this patch changes ModuleParamAccess::value
to return by copy instead of by reference, improving ergonomics. It also
renames the reference-returning accessor to value_ref and updates the
caller in rust_minimal.
> diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
> index 6a8a7a875643a..701210206bba5 100644
> --- a/rust/kernel/module_param.rs
> +++ b/rust/kernel/module_param.rs
> @@ -132,9 +132,12 @@ pub const fn new(default: T) -> Self {
> }
>
> /// Get a shared reference to the parameter value.
> + ///
> + /// Returns a reference to the value supplied at module load time, or a
> + /// reference to the default value if the parameter has not been set.
> // Note: When sysfs access to parameters are enabled, we have to pass in a
> // held lock guard here.
> - pub fn value(&self) -> &T {
> + pub fn value_ref(&self) -> &T {
[Severity: Low]
This isn't a bug, but according to the Rust subsystem coding guidelines,
functions in abstractions that are small or forward to binding calls should
be annotated with #[inline]. Should value_ref() have an #[inline] annotation
added here?
> self.value.as_ref().unwrap_or(&self.default)
> }
[ ... ]
> @@ -146,6 +149,16 @@ pub const fn as_void_ptr(&self) -> *mut c_void {
> }
> }
>
> +impl<T: Copy> ModuleParamAccess<T> {
> + /// Get a copy of the parameter value.
> + ///
> + /// Returns the value supplied at module load time, or the default value
> + /// if the parameter has not been set.
> + pub fn value(&self) -> T {
[Severity: Low]
This isn't a bug, but similar to the above, should this small abstraction
method also be marked with #[inline] to allow compiler optimizations
across crate boundaries?
> + self.value.copy().unwrap_or(self.default)
> + }
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260529-modules-value-ref-v1-1-a0df7e9e6f2d@kernel.org?part=1
next prev parent reply other threads:[~2026-05-29 8:55 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-29 8:41 [PATCH] rust: module_param: return value by copy from `value` Andreas Hindborg
2026-05-29 8:55 ` sashiko-bot [this message]
2026-05-29 12:22 ` Gary Guo
2026-06-01 10:10 ` Andreas Hindborg
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260529085512.32BF01F00893@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=a.hindborg@kernel.org \
--cc=linux-modules@vger.kernel.org \
--cc=ojeda@kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.