Changes in v3: - `#![expect(internal_features)]` is reverted from `rust/compiler_builtins.rs` which was done in the first 2 versions.
Onur Özkan (3): replace `#[allow(...)]` with `#[expect(...)]` rust: remove `#[allow(clippy::unnecessary_cast)]` rust: remove `#[allow(clippy::non_send_fields_in_send_ty)]`
drivers/gpu/nova-core/regs.rs | 2 +- rust/kernel/alloc/allocator_test.rs | 2 +- rust/kernel/cpufreq.rs | 1 - rust/kernel/devres.rs | 2 +- rust/kernel/driver.rs | 2 +- rust/kernel/drm/ioctl.rs | 8 ++++---- rust/kernel/error.rs | 3 +-- rust/kernel/init.rs | 6 +++--- rust/kernel/kunit.rs | 2 +- rust/kernel/opp.rs | 4 ++-- rust/kernel/types.rs | 2 +- rust/macros/helpers.rs | 2 +- 12 files changed, 17 insertions(+), 19 deletions(-)
-- 2.50.0
This makes it clear that the warning is expected not just ignored, so we don't end up having various unnecessary linting rules in the codebase.
Some parts of the codebase already use this approach, this patch just applies it more broadly.
Signed-off-by: Onur Özkan work@onurozkan.dev --- drivers/gpu/nova-core/regs.rs | 2 +- rust/kernel/alloc/allocator_test.rs | 2 +- rust/kernel/devres.rs | 2 +- rust/kernel/driver.rs | 2 +- rust/kernel/drm/ioctl.rs | 8 ++++---- rust/kernel/error.rs | 2 +- rust/kernel/init.rs | 6 +++--- rust/kernel/kunit.rs | 2 +- rust/kernel/opp.rs | 4 ++-- rust/kernel/types.rs | 2 +- rust/macros/helpers.rs | 2 +- 11 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs index 5a1273230306..87e5963f1ebb 100644 --- a/drivers/gpu/nova-core/regs.rs +++ b/drivers/gpu/nova-core/regs.rs @@ -2,7 +2,7 @@
// Required to retain the original register names used by OpenRM, which are all capital snake case // but are mapped to types. -#![allow(non_camel_case_types)] +#![expect(non_camel_case_types)]
#[macro_use] mod macros; diff --git a/rust/kernel/alloc/allocator_test.rs b/rust/kernel/alloc/allocator_test.rs index d19c06ef0498..844197d7194e 100644 --- a/rust/kernel/alloc/allocator_test.rs +++ b/rust/kernel/alloc/allocator_test.rs @@ -7,7 +7,7 @@ //! `Cmalloc` allocator within the `allocator_test` module and type alias all kernel allocators to //! `Cmalloc`. The `Cmalloc` allocator uses libc's `realloc()` function as allocator backend.
-#![allow(missing_docs)] +#![expect(missing_docs)]
use super::{flags::*, AllocError, Allocator, Flags}; use core::alloc::Layout; diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index 57502534d985..0e9510cf4625 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -157,7 +157,7 @@ fn remove_action(this: &Arc<Self>) -> bool { success }
- #[allow(clippy::missing_safety_doc)] + #[expect(clippy::missing_safety_doc)] unsafe extern "C" fn devres_callback(ptr: *mut kernel::ffi::c_void) { let ptr = ptr as *mut DevresInner<T>; // Devres owned this memory; now that we received the callback, drop the `Arc` and hence the diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs index ec9166cedfa7..fc7bd65b01f1 100644 --- a/rust/kernel/driver.rs +++ b/rust/kernel/driver.rs @@ -168,7 +168,7 @@ fn of_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> { }
#[cfg(not(CONFIG_OF))] - #[allow(missing_docs)] + #[expect(missing_docs)] fn of_id_info(_dev: &device::Device) -> Option<&'static Self::IdInfo> { None } diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs index 445639404fb7..3ae8d2d8263f 100644 --- a/rust/kernel/drm/ioctl.rs +++ b/rust/kernel/drm/ioctl.rs @@ -9,28 +9,28 @@ const BASE: u32 = uapi::DRM_IOCTL_BASE as u32;
/// Construct a DRM ioctl number with no argument. -#[allow(non_snake_case)] +#[expect(non_snake_case)] #[inline(always)] pub const fn IO(nr: u32) -> u32 { ioctl::_IO(BASE, nr) }
/// Construct a DRM ioctl number with a read-only argument. -#[allow(non_snake_case)] +#[expect(non_snake_case)] #[inline(always)] pub const fn IOR<T>(nr: u32) -> u32 { ioctl::_IOR::<T>(BASE, nr) }
/// Construct a DRM ioctl number with a write-only argument. -#[allow(non_snake_case)] +#[expect(non_snake_case)] #[inline(always)] pub const fn IOW<T>(nr: u32) -> u32 { ioctl::_IOW::<T>(BASE, nr) }
/// Construct a DRM ioctl number with a read-write argument. -#[allow(non_snake_case)] +#[expect(non_snake_case)] #[inline(always)] pub const fn IOWR<T>(nr: u32) -> u32 { ioctl::_IOWR::<T>(BASE, nr) diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs index 3dee3139fcd4..a59613918d4c 100644 --- a/rust/kernel/error.rs +++ b/rust/kernel/error.rs @@ -263,7 +263,7 @@ fn from(e: core::convert::Infallible) -> Error { /// [`samples/rust/rust_minimal.rs`]): /// /// ``` -/// # #[allow(clippy::single_match)] +/// # #[expect(clippy::single_match)] /// fn example() -> Result { /// let mut numbers = KVec::new(); /// diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs index 8d228c237954..288b1c2a290d 100644 --- a/rust/kernel/init.rs +++ b/rust/kernel/init.rs @@ -30,7 +30,7 @@ //! ## General Examples //! //! ```rust,ignore -//! # #![allow(clippy::disallowed_names)] +//! # #![expect(clippy::disallowed_names)] //! use kernel::types::Opaque; //! use pin_init::pin_init_from_closure; //! @@ -67,11 +67,11 @@ //! ``` //! //! ```rust,ignore -//! # #![allow(unreachable_pub, clippy::disallowed_names)] +//! # #![expect(unreachable_pub, clippy::disallowed_names)] //! use kernel::{prelude::*, types::Opaque}; //! use core::{ptr::addr_of_mut, marker::PhantomPinned, pin::Pin}; //! # mod bindings { -//! # #![allow(non_camel_case_types)] +//! # #![expect(non_camel_case_types)] //! # pub struct foo; //! # pub unsafe fn init_foo(_ptr: *mut foo) {} //! # pub unsafe fn destroy_foo(_ptr: *mut foo) {} diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs index 4b8cdcb21e77..91710a1d7b87 100644 --- a/rust/kernel/kunit.rs +++ b/rust/kernel/kunit.rs @@ -280,7 +280,7 @@ macro_rules! kunit_unsafe_test_suite { static mut KUNIT_TEST_SUITE: ::kernel::bindings::kunit_suite = ::kernel::bindings::kunit_suite { name: KUNIT_TEST_SUITE_NAME, - #[allow(unused_unsafe)] + #[expect(unused_unsafe)] // SAFETY: `$test_cases` is passed in by the user, and // (as documented) must be valid for the lifetime of // the suite (i.e., static). diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs index a566fc3e7dcb..aaeefe84861a 100644 --- a/rust/kernel/opp.rs +++ b/rust/kernel/opp.rs @@ -600,9 +600,9 @@ extern "C" fn config_regulators( pub struct Table { ptr: *mut bindings::opp_table, dev: ARef<Device>, - #[allow(dead_code)] + #[expect(dead_code)] em: bool, - #[allow(dead_code)] + #[expect(dead_code)] of: bool, cpus: Option<CpumaskVar>, } diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs index 22985b6f6982..a5d5a4737a41 100644 --- a/rust/kernel/types.rs +++ b/rust/kernel/types.rs @@ -600,5 +600,5 @@ pub enum Either<L, R> { /// constructed. /// /// [`NotThreadSafe`]: type@NotThreadSafe -#[allow(non_upper_case_globals)] +#[expect(non_upper_case_globals)] pub const NotThreadSafe: NotThreadSafe = PhantomData; diff --git a/rust/macros/helpers.rs b/rust/macros/helpers.rs index e2602be402c1..6fd1462ff01a 100644 --- a/rust/macros/helpers.rs +++ b/rust/macros/helpers.rs @@ -98,7 +98,7 @@ pub(crate) fn file() -> String { }
#[cfg(CONFIG_RUSTC_HAS_SPAN_FILE)] - #[allow(clippy::incompatible_msrv)] + #[expect(clippy::incompatible_msrv)] { proc_macro::Span::call_site().file() } -- 2.50.0
Hi Onur,
kernel test robot noticed the following build warnings:
[auto build test WARNING on driver-core/driver-core-next] [also build test WARNING on driver-core/driver-core-linus rust/alloc-next shuah-kselftest/kunit shuah-kselftest/kunit-fixes rafael-pm/linux-next rafael-pm/bleeding-edge linus/master v6.16-rc3] [cannot apply to rust/rust-next driver-core/driver-core-testing rust/pin-init-next amd-pstate/linux-next amd-pstate/bleeding-edge next-20250627] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Onur-zkan/replace-allow-with-... base: driver-core/driver-core-next patch link: https://lore.kernel.org/r/20250628040956.2181-2-work%40onurozkan.dev patch subject: [PATCH v3 1/3] replace `#[allow(...)]` with `#[expect(...)]` config: x86_64-randconfig-008-20250629 (https://download.01.org/0day-ci/archive/20250629/202506291507.M9eg5kic-lkp@i...) compiler: clang version 20.1.7 (https://github.com/llvm/llvm-project 6146a88f60492b520a36f8f8f3231e15f3cc6082) rustc: rustc 1.78.0 (9b00956e5 2024-04-29) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250629/202506291507.M9eg5kic-lkp@i...)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot lkp@intel.com | Closes: https://lore.kernel.org/oe-kbuild-all/202506291507.M9eg5kic-lkp@intel.com/
All warnings (new ones prefixed by >>):
*** *** Rust bindings generator 'bindgen' < 0.69.5 together with libclang >= 19.1 *** may not work due to a bug (https://github.com/rust-lang/rust-bindgen/pull/2824), *** unless patched (like Debian's). *** Your bindgen version: 0.65.1 *** Your libclang version: 20.1.7 *** *** *** Please see Documentation/rust/quick-start.rst for details *** on how to set up the Rust support. ***
warning: this lint expectation is unfulfilled
--> rust/kernel/opp.rs:603:14 | 603 | #[expect(dead_code)] | ^^^^^^^^^ | = note: `#[warn(unfulfilled_lint_expectations)]` on by default --
warning: this lint expectation is unfulfilled
--> rust/kernel/opp.rs:605:14 | 605 | #[expect(dead_code)] | ^^^^^^^^^
This isn't needed anymore since `kernel::ffi::c_int` type is always `i32` which differs from `err` (isize).
Signed-off-by: Onur Özkan work@onurozkan.dev --- rust/kernel/error.rs | 1 - 1 file changed, 1 deletion(-)
diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs index a59613918d4c..05c6e71c0afb 100644 --- a/rust/kernel/error.rs +++ b/rust/kernel/error.rs @@ -413,7 +413,6 @@ pub fn from_err_ptr<T>(ptr: *mut T) -> Result<*mut T> { // SAFETY: The FFI function does not deref the pointer. let err = unsafe { bindings::PTR_ERR(const_ptr) };
- #[allow(clippy::unnecessary_cast)] // CAST: If `IS_ERR()` returns `true`, // then `PTR_ERR()` is guaranteed to return a // negative value greater-or-equal to `-bindings::MAX_ERRNO`, -- 2.50.0
Clippy no longer complains about this lint.
Signed-off-by: Onur Özkan work@onurozkan.dev --- rust/kernel/cpufreq.rs | 1 - 1 file changed, 1 deletion(-)
diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs index 11b03e9d7e89..97de9b0573da 100644 --- a/rust/kernel/cpufreq.rs +++ b/rust/kernel/cpufreq.rs @@ -907,7 +907,6 @@ fn register_em(_policy: &mut Policy) { /// or CPUs, so it is safe to share it. unsafe impl<T: Driver> Sync for Registration<T> {}
-#[allow(clippy::non_send_fields_in_send_ty)] /// SAFETY: Registration with and unregistration from the cpufreq subsystem can happen from any /// thread. unsafe impl<T: Driver> Send for Registration<T> {} -- 2.50.0
On Sat, Jun 28, 2025 at 6:10 AM Onur Özkan work@onurozkan.dev wrote:
Clippy no longer complains about this lint.
Do you have more context? For instance, do you know since when it no longer complains, or why was the reason for the change? i.e. why we had the `allow` in the first place, so that we know we don't need it anymore?
For instance, please how I reasoned about it in commit 5e7c9b84ad08 ("rust: sync: remove unneeded `#[allow(clippy::non_send_fields_in_send_ty)]`").
(It may happen to be the same reason, or not.)
Thanks!
Cheers, Miguel
On Sat, 28 Jun 2025 09:13:50 +0200 Miguel Ojeda miguel.ojeda.sandonis@gmail.com wrote:
On Sat, Jun 28, 2025 at 6:10 AM Onur Özkan work@onurozkan.dev wrote:
Clippy no longer complains about this lint.
Do you have more context? For instance, do you know since when it no longer complains, or why was the reason for the change? i.e. why we had the `allow` in the first place, so that we know we don't need it anymore?
For instance, please how I reasoned about it in commit 5e7c9b84ad08 ("rust: sync: remove unneeded `#[allow(clippy::non_send_fields_in_send_ty)]`").
(It may happen to be the same reason, or not.)
Thanks!
Cheers, Miguel
It doesn't seem to be the same reason. I rebased over c6af9a1191d042839e56abff69e8b0302d117988 (the exact commit where that lint was added) but still Clippy did not complain about it on the MSRV. So it was either a leftover, or there is a version between 1.78 and the current stable where Clippy did complain. I can dig into it more during the week if you would like.
IMO, we should require people to add a comment explaining the reason for adding these lint rules to the codebase. It would make both reading and modifying the code much simpler and clearer.
Regards, Onur
On Sat, Jun 28, 2025 at 12:30 PM Onur work@onurozkan.dev wrote:
It doesn't seem to be the same reason. I rebased over c6af9a1191d042839e56abff69e8b0302d117988 (the exact commit where that lint was added) but still Clippy did not complain about it on the MSRV. So it was either a leftover, or there is a version between 1.78 and the current stable where Clippy did complain. I can dig into it more during the week if you would like.
Are you sure? The lint is actually disabled, as I mention in 5e7c9b84ad08.
From a quick test, I enabled it in that file, and I get the warning.
Thus it seems to me Clippy would still complain about it just fine.
It doesn't mean we shouldn't remove it, though.
IMO, we should require people to add a comment explaining the reason for adding these lint rules to the codebase. It would make both reading and modifying the code much simpler and clearer.
Do you mean using the lint reasons feature? IIRC we discussed at some point doing that when the feature was added (we enabled it for the `expect` side of things).
For non-obvious cases or uncommon lints, it would be definitely nice (a comment is also OK). I am not sure if it is worth enforcing it for every single case, though.
It would be nice if `clippy::allow_attributes_without_reason` could be enabled just for `allow`, or ignore it for certain lints.
Cheers, Miguel
On Sat, 28 Jun 2025 14:18:53 +0200 Miguel Ojeda miguel.ojeda.sandonis@gmail.com wrote:
On Sat, Jun 28, 2025 at 12:30 PM Onur work@onurozkan.dev wrote:
It doesn't seem to be the same reason. I rebased over c6af9a1191d042839e56abff69e8b0302d117988 (the exact commit where that lint was added) but still Clippy did not complain about it on the MSRV. So it was either a leftover, or there is a version between 1.78 and the current stable where Clippy did complain. I can dig into it more during the week if you would like.
Are you sure? The lint is actually disabled, as I mention in 5e7c9b84ad08.
From a quick test, I enabled it in that file, and I get the warning.
Thus it seems to me Clippy would still complain about it just fine.
Yes, I am sure. Just to clarify, I am not testing 5e7c9b84ad08. I am testing c6af9a1191d042839e56abff69e8b0302d117988 where `#[allow(clippy::non_send_fields_in_send_ty)]` was added on `unsafe impl<T: Driver> Send for Registration<T> {}`.
Switching from `allow` to `expect` produced the following result on my end:
``` $ make LLVM=1 -j $(nproc) CLIPPY=1 DESCEND objtool CALL scripts/checksyscalls.sh INSTALL libsubcmd_headers CLIPPY L rust/kernel.o error: this lint expectation is unfulfilled --> rust/kernel/cpufreq.rs:908:10 | 908 | #[expect(clippy::non_send_fields_in_send_ty)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `-D unfulfilled-lint-expectations` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(unfulfilled_lint_expectations)]`
error: aborting due to 1 previous error
make[2]: *** [rust/Makefile:534: rust/kernel.o] Error 1 make[1]: *** [/home/nimda/devspace/onur-ozkan/linux/Makefile:1283: prepare] Error 2 make: *** [Makefile:248: __sub-make] Error 2
$ git log -1 commit c6af9a1191d042839e56abff69e8b0302d117988 (HEAD -> master) Author: Viresh Kumar viresh.kumar@linaro.org Date: Wed Jan 24 12:36:33 2024 +0530
rust: cpufreq: Extend abstractions for driver registration
Extend the cpufreq abstractions to support driver registration from Rust.
Reviewed-by: Danilo Krummrich dakr@kernel.org Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
$ rustc --version --verbose rustc 1.78.0 (9b00956e5 2024-04-29) binary: rustc commit-hash: 9b00956e56009bab2aa15d7bff10916599e3d6d6 commit-date: 2024-04-29 host: x86_64-unknown-linux-gnu release: 1.78.0 LLVM version: 18.1.2 ```
Regards, Onur
On Sat, Jun 28, 2025 at 2:42 PM Onur work@onurozkan.dev wrote:
Yes, I am sure. Just to clarify, I am not testing 5e7c9b84ad08. I am testing c6af9a1191d042839e56abff69e8b0302d117988 where `#[allow(clippy::non_send_fields_in_send_ty)]` was added on `unsafe impl<T: Driver> Send for Registration<T> {}`.
Switching from `allow` to `expect` produced the following result on my end:
Yes, of course it does -- what I am telling you (and what 5e7c9b84ad08 says) is that the lint is disabled.
And since it is disabled, if you change the line to `expect`, then it will obviously complain.
If you actually enabled the lint with e.g.
#![warn(clippy::non_send_fields_in_send_ty)]
at the top of the file, and then used `expect`, it will build fine.
Cheers, Miguel
On Sat, 28 Jun 2025 14:51:15 +0200 Miguel Ojeda miguel.ojeda.sandonis@gmail.com wrote:
On Sat, Jun 28, 2025 at 2:42 PM Onur work@onurozkan.dev wrote:
Yes, I am sure. Just to clarify, I am not testing 5e7c9b84ad08. I am testing c6af9a1191d042839e56abff69e8b0302d117988 where `#[allow(clippy::non_send_fields_in_send_ty)]` was added on `unsafe impl<T: Driver> Send for Registration<T> {}`.
Switching from `allow` to `expect` produced the following result on my end:
Yes, of course it does -- what I am telling you (and what 5e7c9b84ad08 says) is that the lint is disabled.
And since it is disabled, if you change the line to `expect`, then it will obviously complain.
If you actually enabled the lint with e.g.
#![warn(clippy::non_send_fields_in_send_ty)]at the top of the file, and then used `expect`, it will build fine.
Aha, I see. I missed that. I guess `allow` was added when the author had this lint enabled on their checkout, but their work was merged when lint removal was merged before that.
Do you want me to update the patch description by including 5e7c9b84ad08 ref and send v4?
Sorry for misunderstanding by the way!
Many thanks, Onur
On Sat, Jun 28, 2025 at 3:11 PM Onur work@onurozkan.dev wrote:
Aha, I see. I missed that. I guess `allow` was added when the author had this lint enabled on their checkout, but their work was merged when lint removal was merged before that.
Yeah, some of the code going around was written years ago, so sometimes this sort of thing happens. :)
Do you want me to update the patch description by including 5e7c9b84ad08 ref and send v4?
Sure -- maybe wait a few days, to see if anyone says anything else. Then we will need to wait for Acked-bys from other maintainers.
Or, actually, if you are sending a new version and you are willing to do it, then it would be easier to land if you split the first patch also by subsystem -- that way each maintainer can take their patches on their own time instead. Since each patch is independent, you can send them in independent patch series, that makes it even easier for maintainers to track.
For instance, you could do this grouping:
rust/kernel/error.rs | 2 +- rust/kernel/types.rs | 2 +- rust/macros/helpers.rs | 2 +- (+ this patch #2)
rust/kernel/init.rs | 6 +++---
rust/kernel/kunit.rs | 2 +-
drivers/gpu/nova-core/regs.rs | 2 +- rust/kernel/drm/ioctl.rs | 8 ++++----
rust/kernel/devres.rs | 2 +- rust/kernel/driver.rs | 2 +-
rust/kernel/alloc/allocator_test.rs | 2 +-
rust/kernel/opp.rs | 4 ++-- (+ this patch #3)
So e.g. the top one (Rust) would be a series of 2 patches, then the next one (pin-init) a single independent patch, and so on.
Sorry for misunderstanding by the way!
No worries at all! It happens :)
And thanks for doing these cleanups! They are appreciated.
Cheers, Miguel
On Sat, 28 Jun 2025 15:28:29 +0200 Miguel Ojeda miguel.ojeda.sandonis@gmail.com wrote:
On Sat, Jun 28, 2025 at 3:11 PM Onur work@onurozkan.dev wrote:
Aha, I see. I missed that. I guess `allow` was added when the author had this lint enabled on their checkout, but their work was merged when lint removal was merged before that.
Yeah, some of the code going around was written years ago, so sometimes this sort of thing happens. :)
Do you want me to update the patch description by including 5e7c9b84ad08 ref and send v4?
Sure -- maybe wait a few days, to see if anyone says anything else. Then we will need to wait for Acked-bys from other maintainers.
Or, actually, if you are sending a new version and you are willing to do it, then it would be easier to land if you split the first patch also by subsystem -- that way each maintainer can take their patches on their own time instead. Since each patch is independent, you can send them in independent patch series, that makes it even easier for maintainers to track.
I don't have enough time to do it right now, but I would be happy to do it in ~3 days during the week (assuming it's still not being reviewed by the maintainers by then).
Regards, Onur
On Sat, Jun 28, 2025 at 3:42 PM Onur work@onurozkan.dev wrote:
I don't have enough time to do it right now, but I would be happy to do it in ~3 days during the week (assuming it's still not being reviewed by the maintainers by then).
No worries, there is no rush on these cleanups. Even if it gets reviewed, you can still split, keeping their tag. Thanks!
Cheers, Miguel
On Sat, 28 Jun 2025 14:18:53 +0200 Miguel Ojeda miguel.ojeda.sandonis@gmail.com wrote:
IMO, we should require people to add a comment explaining the reason for adding these lint rules to the codebase. It would make both reading and modifying the code much simpler and clearer.
Do you mean using the lint reasons feature? IIRC we discussed at some point doing that when the feature was added (we enabled it for the `expect` side of things).
Yeah, I meant that it't taking more effort than it should, like digging through historical changes in the relevant parts of the source code, trying to figuring out whether it was just a false positive or if there was a specific reason behind it, etc.
On Sat, Jun 28, 2025 at 3:04 PM Onur work@onurozkan.dev wrote:
Yeah, I meant that it't taking more effort than it should, like digging through historical changes in the relevant parts of the source code, trying to figuring out whether it was just a false positive or if there was a specific reason behind it, etc.
Yeah, that is a big part of kernel development, especially on the maintenance side :)
I definitely agree that a good comment in the source code is better than going through Git history, and the kernel sometimes has had some things documented in the Git log that should have been in the source code instead. It happens.
However, in some cases like this one it is not clear it would help. For instance, here the lint reason message could have been something that made sense back then when the lint was enabled, and yet we would still have had to notice the lint got disabled later on, so we would end up still going into the Git log.
`expect` is great to mitigate some of that -- sadly we cannot use it as much as we would like due to sometimes being conditional to an arch or the kernel config or the Rust version. (And your first patch may have some cases that perhaps we cannot convert due to that -- I didn't check)
Cheers, Miguel
linux-kselftest-mirror@lists.linaro.org
-
kernel test robot -
Miguel Ojeda -
Onur -
Onur Özkan