[PATCH v9 2/2] scripts: checkpatch: add RUST_UNWRAP lint

Jason Hall posted 2 patches 1 day ago
[PATCH v9 2/2] scripts: checkpatch: add RUST_UNWRAP lint
Posted by Jason Hall 1 day ago
Warn against the use of .unwrap() and .expect() unless accompanied by
a '// PANIC:' comment. This enforces safety standards in the Rust-
for-Linux project until upstream Clippy lints are integrated.

Suggested-by: Miguel Ojeda <ojeda@kernel.org>
Link: https://github.com/Rust-for-linux/linux/issues/1191
Signed-off-by: Jason Hall <jason.kei.hall@gmail.com>
---
 scripts/rust_checkpatch.pl | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/scripts/rust_checkpatch.pl b/scripts/rust_checkpatch.pl
index 56c1bc29d3f2..fa7adaed264c 100644
--- a/scripts/rust_checkpatch.pl
+++ b/scripts/rust_checkpatch.pl
@@ -9,7 +9,21 @@ use warnings;
 sub process_rust {
     my ($line, $rawline, $herecurr) = @_;
 
-    # Reserve for future Rust-specific lints
+    # Check for Rust unwrap/expect usage.
+    # We skip lines that are already comments, assert macros (common in tests),
+    # or have a '// PANIC:' justification.
+    if ($line =~ /^\+/) {
+        if ($line =~ /(?:\.|::)(?:unwrap|expect)\s*\(/ &&
+            $rawline !~ /\/\/\s*PANIC:/ &&
+            $line !~ /^\+\s*\/\// &&
+            $line !~ /^\+\s*assert/) {
+            return ("RUST_UNWRAP",
+                    "unwrap() and expect() should generally be avoided in Rust kernel code.\n" .
+                   "If the use is intended, please justify it with a '// PANIC:' comment.\n" .
+                    "See: https://rust.docs.kernel.org/kernel/error/type.Result.html#error-codes-in-c-and-rust\n" .
+                    $herecurr);
+        }
+    }
     return ();
 }
 
-- 
2.43.0
Re: [PATCH v9 2/2] scripts: checkpatch: add RUST_UNWRAP lint
Posted by Dirk Behme 15 hours ago
On 07.02.26 23:49, Jason Hall wrote:
> Warn against the use of .unwrap() and .expect() unless accompanied by
> a '// PANIC:' comment. This enforces safety standards in the Rust-
> for-Linux project until upstream Clippy lints are integrated.


I wonder if we could add some outcome from the mailing list discussion
to the commit message? E.g. what we consider to be false positives,
the handling of them and what we suppose to be fixed etc.


> Suggested-by: Miguel Ojeda <ojeda@kernel.org>
> Link: https://github.com/Rust-for-linux/linux/issues/1191
> Signed-off-by: Jason Hall <jason.kei.hall@gmail.com>
> ---
>  scripts/rust_checkpatch.pl | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/rust_checkpatch.pl b/scripts/rust_checkpatch.pl
> index 56c1bc29d3f2..fa7adaed264c 100644
> --- a/scripts/rust_checkpatch.pl
> +++ b/scripts/rust_checkpatch.pl
> @@ -9,7 +9,21 @@ use warnings;
>  sub process_rust {
>      my ($line, $rawline, $herecurr) = @_;
>  
> -    # Reserve for future Rust-specific lints
> +    # Check for Rust unwrap/expect usage.
> +    # We skip lines that are already comments, assert macros (common in tests),
> +    # or have a '// PANIC:' justification.
> +    if ($line =~ /^\+/) {
> +        if ($line =~ /(?:\.|::)(?:unwrap|expect)\s*\(/ &&


Whats about the `.expect()` topic discussed in

https://lore.kernel.org/rust-for-linux/a798e6a368639f7a1ce633a6dfecd088d6ed4123.camel@perches.com/T/#m00723ad673727036e5fcf96a35f2f231ec9de31f

https://lore.kernel.org/rust-for-linux/a798e6a368639f7a1ce633a6dfecd088d6ed4123.camel@perches.com/T/#m5604274a633ef33eb474f95b54f797843d0fe1dd

?

> +            $rawline !~ /\/\/\s*PANIC:/ &&
> +            $line !~ /^\+\s*\/\// &&
> +            $line !~ /^\+\s*assert/) {
> +            return ("RUST_UNWRAP",
> +                    "unwrap() and expect() should generally be avoided in Rust kernel code.\n" .
> +                   "If the use is intended, please justify it with a '// PANIC:' comment.\n" .
> +                    "See: https://rust.docs.kernel.org/kernel/error/type.Result.html#error-codes-in-c-and-rust\n" .
> +                    $herecurr);
> +        }
> +    }
>      return ();
>  }

Just for the logs:

Running this on e.g.

https://lore.kernel.org/rust-for-linux/20260207-binder-shrink-vec-v3-v3-3-8ff388563427@cock.li/

gives


$ ./scripts/checkpatch.pl
0001-rust-alloc-add-KUnit-tests-for-Vec-shrink-operations.patch
WARNING: unwrap() and expect() should generally be avoided in Rust
kernel code.
If the use is intended, please justify it with a '// PANIC:' comment.
See:
https://rust.docs.kernel.org/kernel/error/type.Result.html#error-codes-in-c-and-rust
#52: FILE: rust/kernel/alloc/kvec.rs:1524:
+        let mut v: VVec<u32> = VVec::with_capacity(initial_capacity,
GFP_KERNEL).unwrap();

...

total: 0 errors, 21 warnings, 189 lines checked

(note: all 21 warnings are from `unwrap()`)

I'm not sure if it makes me happy to ignore these 21 warnings as false
positives ;)

Best regards

Dirk
Re: [PATCH v9 2/2] scripts: checkpatch: add RUST_UNWRAP lint
Posted by Jason Hall 8 hours ago
Hi Dirk,

I have been playing around with the lint logic. It now identifies 
when we enter a test-related block (like mod tests, #[test], or KUnit 
macros) and suppresses the warning for that specific context. 

This handles the false positives you encountered in the Binder patch 
while still catching "lazy" unwraps in production logic.

I tested the scenarios laid out below.

Proposed Logic:

my $in_rust_test_context = 0;

sub process_rust {
    my ($line, $rawline, $herecurr) = @_;

    if ($rawline =~ /^(?:@@|diff --git)/) {
        $in_rust_test_context = 0;
    }

    if ($line =~ /^\+\s*(?:mod\s+tests|#\[.*(?:test|kunit))/) {
        $in_rust_test_context = 1;
    }

    if ($line =~ /^\+/ && !$in_rust_test_context) {
        if ($line =~ /(?:\.|::)(?:unwrap|expect)\s*\(/ &&
            $rawline !~ /\/\/\s*PANIC:/ &&
            $line !~ /^\+\s*\/\// &&
            $line !~ /^\+\s*assert/) {
            return ("RUST_UNWRAP", "...");
        }
    }
    return ();
}

Scenarios:

--- a/rust/kernel/alloc/kvec.rs
+++ b/rust/kernel/alloc/kvec.rs
@@ -42,6 +42,7 @@
     pub fn new_with_data() -> Self {
-        let x = Some(1);
+        // Scenario 1: Production code (SHOULD WARN)
+        let val = some_kernel_function().unwrap();
         Vec { ptr: NonNull::dangling(), len: 0 }
     }
 
@@ -150,6 +151,13 @@
+    // Scenario 2: Explicitly justified (SHOULD NOT WARN)
+    pub fn proof_of_concept() {
+        let x = core::ptr::NonNull::new(ptr).unwrap(); // PANIC: ptr is never null here
+    }
+
+    // Scenario 3: Already a comment (SHOULD NOT WARN)
+    // let x = something.unwrap();
+
@@ -200,10 +208,20 @@
+#[test]
+fn scenario_4_standalone_test() {
+    // SHOULD NOT WARN because of #[test] above
+    let v: KVec<u32> = KVec::with_capacity(1, GFP_KERNEL).unwrap();
+}
+
+#[macros::kunit_tests(rust_kvec)]
+mod tests {
+    use super::*;
+
+    fn test_internal_logic() {
+        // Scenario 5: Inside KUnit mod (SHOULD NOT WARN)
+        // Dirk's specific 21 warnings happened in a block like this.
+        let mut v = KVec::new();
+        v.push(1, GFP_KERNEL).unwrap();
+        assert_eq!(v.pop().unwrap(), 1);
+    }
+}

If this looks okay, I can resubmit the patch series with this.

Best regards,
Jason