[PATCH v3] scripts: checkpatch: warn on Rust panicking methods

Jason Hall posted 1 patch 5 days, 7 hours ago
There is a newer version of this series
scripts/checkpatch.pl | 11 +++++++++++
1 file changed, 11 insertions(+)
[PATCH v3] scripts: checkpatch: warn on Rust panicking methods
Posted by Jason Hall 5 days, 7 hours ago
Added regex check in checkpatch.pl for common Rust panicking methods
like unwrap() and expect().

Allowed an exception if the line contains a '// PANIC:' comment.

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>

Hi, Joe,

I updated the regex, and I looked at Ryan Foster's patch.  These two PANIC
checks don't seem to interfere with each other as far as I can see.
---
v3:
 - Using non-capturing groups (?:) to optimize regex.
v2:
 - Switched from \b to (\.|::) to avoid false positives in strings.
 - Added : to PANIC comment check.
 scripts/checkpatch.pl | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index c0250244cf7a..37bdf602e7e7 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3834,6 +3834,17 @@ sub process {
 # check we are in a valid source file if not then ignore this hunk
 		next if ($realfile !~ /\.(h|c|rs|s|S|sh|dtsi|dts)$/);
 
+# check for Rust unwrap/expect
+		if ($realfile =~ /\.rs$/ && $line =~ /^\+/) {
+			if ($line =~ /(?:\.|::)(?:unwrap|expect)\s*\(/ &&
+				$rawline !~ /\/\/\s*PANIC:/ &&
+				$line !~ /^\+\s*\/\// &&
+				$line !~ /^\+\s*assert/) {
+				WARN("RUST_UNWRAP",
+					"Avoid unwrap() or expect() in Rust code; use proper error handling (Result) or justify with a '// PANIC: ...' comment.\n" . $herecurr);
+			}
+		}
+
 # check for using SPDX-License-Identifier on the wrong line number
 		if ($realline != $checklicenseline &&
 		    $rawline =~ /\bSPDX-License-Identifier:/ &&
-- 
2.43.0
Re: [PATCH v3] scripts: checkpatch: warn on Rust panicking methods
Posted by Dirk Behme 4 days, 21 hours ago
Hi Jason,

On 01/02/2026 20:57, Jason Hall wrote:
> Added regex check in checkpatch.pl for common Rust panicking methods

Describe your changes in imperative mood. Added -> Add

> like unwrap() and expect().
> 
> Allowed an exception if the line contains a '// PANIC:' comment.

I recently came across that using `.unwrap()` in `tests` seems to be 
fine. E.g.:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/alloc/kvec.rs#n1364

Is this covered here?

> 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>

Something with the formatting seems to be wrong here? `---` is missing here?

Best regards

Dirk

> Hi, Joe,
> 
> I updated the regex, and I looked at Ryan Foster's patch.  These two PANIC
> checks don't seem to interfere with each other as far as I can see.
> ---
> v3:
>   - Using non-capturing groups (?:) to optimize regex.
> v2:
>   - Switched from \b to (\.|::) to avoid false positives in strings.
>   - Added : to PANIC comment check.
>   scripts/checkpatch.pl | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index c0250244cf7a..37bdf602e7e7 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3834,6 +3834,17 @@ sub process {
>   # check we are in a valid source file if not then ignore this hunk
>   		next if ($realfile !~ /\.(h|c|rs|s|S|sh|dtsi|dts)$/);
>   
> +# check for Rust unwrap/expect
> +		if ($realfile =~ /\.rs$/ && $line =~ /^\+/) {
> +			if ($line =~ /(?:\.|::)(?:unwrap|expect)\s*\(/ &&
> +				$rawline !~ /\/\/\s*PANIC:/ &&
> +				$line !~ /^\+\s*\/\// &&
> +				$line !~ /^\+\s*assert/) {
> +				WARN("RUST_UNWRAP",
> +					"Avoid unwrap() or expect() in Rust code; use proper error handling (Result) or justify with a '// PANIC: ...' comment.\n" . $herecurr);
> +			}
> +		}
> +
>   # check for using SPDX-License-Identifier on the wrong line number
>   		if ($realline != $checklicenseline &&
>   		    $rawline =~ /\bSPDX-License-Identifier:/ &&
[PATCH v4] scripts: checkpatch: warn on Rust panicking methods
Posted by Jason Hall 4 days, 13 hours ago
Add regex check in checkpatch.pl for common Rust panicking methods
like unwrap() and expect().

Allow an exception if the line contains a '// PANIC:' comment.

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>
---
Hi Dirk,

Now using imperative mood.  I decided to keep that logic stateless
unless its agreed that we need to add state.  Adding checks for #[test]
and other test identifiers will make this much more complicated.  There
is already a check for // PANIC: that works fine.

v4:
 - Use imperative mood in commit description.
 - Fix patch formatting and placement of '---' separator.
v3:
 - Use non-capturing groups (?:) to optimize regex.
v2:
 - Switch from \b to (\.|::) to avoid false positives in strings.
 - Add : to PANIC comment check.

 scripts/checkpatch.pl | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index c0250244cf7a..37bdf602e7e7 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3834,6 +3834,17 @@ sub process {
 # check we are in a valid source file if not then ignore this hunk
 		next if ($realfile !~ /\.(h|c|rs|s|S|sh|dtsi|dts)$/);
 
+# check for Rust unwrap/expect
+		if ($realfile =~ /\.rs$/ && $line =~ /^\+/) {
+			if ($line =~ /(?:\.|::)(?:unwrap|expect)\s*\(/ &&
+				$rawline !~ /\/\/\s*PANIC:/ &&
+				$line !~ /^\+\s*\/\// &&
+				$line !~ /^\+\s*assert/) {
+				WARN("RUST_UNWRAP",
+					"Avoid unwrap() or expect() in Rust code; use proper error handling (Result) or justify with a '// PANIC: ...' comment.\n" . $herecurr);
+			}
+		}
+
 # check for using SPDX-License-Identifier on the wrong line number
 		if ($realline != $checklicenseline &&
 		    $rawline =~ /\bSPDX-License-Identifier:/ &&
-- 
2.43.0
Re: [PATCH v4] scripts: checkpatch: warn on Rust panicking methods
Posted by Dirk Behme 3 days, 21 hours ago
Hi Jason,

On 02/02/2026 14:56, Jason Hall wrote:
> Add regex check in checkpatch.pl for common Rust panicking methods
> like unwrap() and expect().
> 
> Allow an exception if the line contains a '// PANIC:' comment.
> 
> 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>
> ---
> Hi Dirk,
> 
> Now using imperative mood.  I decided to keep that logic stateless
> unless its agreed that we need to add state.  Adding checks for #[test]
> and other test identifiers will make this much more complicated.  There
> is already a check for // PANIC: that works fine.


Yes. I'm just slightly unclear what we do with existing code where using 
`unwrap()` is fine/accepted/required? And with patches like

https://lore.kernel.org/rust-for-linux/20260131154016.270385-3-shivamklr@cock.li/

I would guess with this change we would get warnings on that?

So the idea is that we would need some add-on patches on top of this to 
annotate existing code / patches with `// PANIC: ...` to stay warning clean?

Thanks,

Dirk

Btw, what's with Gary's comment to drop `expect()`?


> v4:
>   - Use imperative mood in commit description.
>   - Fix patch formatting and placement of '---' separator.
> v3:
>   - Use non-capturing groups (?:) to optimize regex.
> v2:
>   - Switch from \b to (\.|::) to avoid false positives in strings.
>   - Add : to PANIC comment check.
> 
>   scripts/checkpatch.pl | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index c0250244cf7a..37bdf602e7e7 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3834,6 +3834,17 @@ sub process {
>   # check we are in a valid source file if not then ignore this hunk
>   		next if ($realfile !~ /\.(h|c|rs|s|S|sh|dtsi|dts)$/);
>   
> +# check for Rust unwrap/expect
> +		if ($realfile =~ /\.rs$/ && $line =~ /^\+/) {
> +			if ($line =~ /(?:\.|::)(?:unwrap|expect)\s*\(/ &&
> +				$rawline !~ /\/\/\s*PANIC:/ &&
> +				$line !~ /^\+\s*\/\// &&
> +				$line !~ /^\+\s*assert/) {
> +				WARN("RUST_UNWRAP",
> +					"Avoid unwrap() or expect() in Rust code; use proper error handling (Result) or justify with a '// PANIC: ...' comment.\n" . $herecurr);
> +			}
> +		}
> +
>   # check for using SPDX-License-Identifier on the wrong line number
>   		if ($realline != $checklicenseline &&
>   		    $rawline =~ /\bSPDX-License-Identifier:/ &&
Re: [PATCH v4] scripts: checkpatch: warn on Rust panicking methods
Posted by Jkhall81 3 days, 11 hours ago
Nice, emails sent from gmail get automatically rejected.

So, Dirk.  To satisfy your concerns the current 10ish line
code update is going to slowly, after many more emails
written in nano, mutate into a franken-regex-perl beast. 
checkpatch.pl is already huge.  I'm not a fan of this 
approach.

We could just not do this. Right now we are trying to
get a warning if someone uses rust code that can cause a
panic.  Software Engineers are smart people.  What if they
just don't use rust code that causes panics inside core
files.  Problem solved.
Re: [PATCH v4] scripts: checkpatch: warn on Rust panicking methods
Posted by Onur Özkan 3 days, 11 hours ago
On Tue,  3 Feb 2026 08:25:41 -0700
Jkhall81 <jason.kei.hall@gmail.com> wrote:

> Nice, emails sent from gmail get automatically rejected.
> 
> So, Dirk.  To satisfy your concerns the current 10ish line
> code update is going to slowly, after many more emails
> written in nano, mutate into a franken-regex-perl beast. 
> checkpatch.pl is already huge.  I'm not a fan of this 
> approach.

Me neither. I wonder why we are doing this instead of using the
unwrap_used and expect_used linting rules from clippy. This would
catch the problem much earlier than checkpath since many of us build
the kernel with CLIPPY=1 flag.

Regards,
Onur

> 
> We could just not do this. Right now we are trying to
> get a warning if someone uses rust code that can cause a
> panic.  Software Engineers are smart people.  What if they
> just don't use rust code that causes panics inside core
> files.  Problem solved.
> 
>
Re: [PATCH v4] scripts: checkpatch: warn on Rust panicking methods
Posted by Miguel Ojeda 2 days, 9 hours ago
On Tue, Feb 3, 2026 at 4:49 PM Onur Özkan <work@onurozkan.dev> wrote:
>
> Me neither. I wonder why we are doing this instead of using the
> unwrap_used and expect_used linting rules from clippy. This would
> catch the problem much earlier than checkpath since many of us build
> the kernel with CLIPPY=1 flag.

Please see my reply to Dirk and the context I link there, but
regarding the existing Clippy lints: we want to have the `// TAG`
style comments, rather than attributes, so we are waiting for the
Clippy lints on this.

Cheers,
Miguel
Re: [PATCH v4] scripts: checkpatch: warn on Rust panicking methods
Posted by Gary Guo 3 days, 11 hours ago
On Tue Feb 3, 2026 at 3:49 PM GMT, Onur Özkan wrote:
> On Tue,  3 Feb 2026 08:25:41 -0700
> Jkhall81 <jason.kei.hall@gmail.com> wrote:
>
>> Nice, emails sent from gmail get automatically rejected.
>> 
>> So, Dirk.  To satisfy your concerns the current 10ish line
>> code update is going to slowly, after many more emails
>> written in nano, mutate into a franken-regex-perl beast. 
>> checkpatch.pl is already huge.  I'm not a fan of this 
>> approach.
>
> Me neither. I wonder why we are doing this instead of using the
> unwrap_used and expect_used linting rules from clippy. This would
> catch the problem much earlier than checkpath since many of us build
> the kernel with CLIPPY=1 flag.

Because it's okay to `panic` or use `expect`. checkpatch will just warn you
once when the code is introduced, not continuously in each build.

Best,
Gary

>
> Regards,
> Onur
>
>> 
>> We could just not do this. Right now we are trying to
>> get a warning if someone uses rust code that can cause a
>> panic.  Software Engineers are smart people.  What if they
>> just don't use rust code that causes panics inside core
>> files.  Problem solved.
>> 
>> 
Re: [PATCH v4] scripts: checkpatch: warn on Rust panicking methods
Posted by Onur Özkan 3 days, 10 hours ago
On Tue, 03 Feb 2026 16:02:02 +0000
"Gary Guo" <gary@garyguo.net> wrote:

> On Tue Feb 3, 2026 at 3:49 PM GMT, Onur Özkan wrote:
> > On Tue,  3 Feb 2026 08:25:41 -0700
> > Jkhall81 <jason.kei.hall@gmail.com> wrote:
> >
> >> Nice, emails sent from gmail get automatically rejected.
> >> 
> >> So, Dirk.  To satisfy your concerns the current 10ish line
> >> code update is going to slowly, after many more emails
> >> written in nano, mutate into a franken-regex-perl beast. 
> >> checkpatch.pl is already huge.  I'm not a fan of this 
> >> approach.
> >
> > Me neither. I wonder why we are doing this instead of using the
> > unwrap_used and expect_used linting rules from clippy. This would
> > catch the problem much earlier than checkpath since many of us build
> > the kernel with CLIPPY=1 flag.
> 
> Because it's okay to `panic` or use `expect`. checkpatch will just
> warn you once when the code is introduced, not continuously in each
> build.

That's interesting because it implies that it's okay for people to use
them without "// PANIC..." comments. That sounds problematic since it
means some instances will have that comment while others may not.

In my opinion, adding a clippy rule and using "#[allow(...)]" in the
places where it's acceptable to use them makes more sense. This is at
least more consistent and doesn't bloat the checkpatch file.

Thanks,
Onur

> 
> Best,
> Gary
> 
> >
> > Regards,
> > Onur
> >
> >> 
> >> We could just not do this. Right now we are trying to
> >> get a warning if someone uses rust code that can cause a
> >> panic.  Software Engineers are smart people.  What if they
> >> just don't use rust code that causes panics inside core
> >> files.  Problem solved.
> >> 
> >> 
> 
Re: [PATCH v4] scripts: checkpatch: warn on Rust panicking methods
Posted by Gary Guo 3 days, 10 hours ago
On Tue Feb 3, 2026 at 4:32 PM GMT, Onur Özkan wrote:
> On Tue, 03 Feb 2026 16:02:02 +0000
> "Gary Guo" <gary@garyguo.net> wrote:
>
>> On Tue Feb 3, 2026 at 3:49 PM GMT, Onur Özkan wrote:
>> > On Tue,  3 Feb 2026 08:25:41 -0700
>> > Jkhall81 <jason.kei.hall@gmail.com> wrote:
>> >
>> >> Nice, emails sent from gmail get automatically rejected.
>> >> 
>> >> So, Dirk.  To satisfy your concerns the current 10ish line
>> >> code update is going to slowly, after many more emails
>> >> written in nano, mutate into a franken-regex-perl beast. 
>> >> checkpatch.pl is already huge.  I'm not a fan of this 
>> >> approach.
>> >
>> > Me neither. I wonder why we are doing this instead of using the
>> > unwrap_used and expect_used linting rules from clippy. This would
>> > catch the problem much earlier than checkpath since many of us build
>> > the kernel with CLIPPY=1 flag.
>> 
>> Because it's okay to `panic` or use `expect`. checkpatch will just
>> warn you once when the code is introduced, not continuously in each
>> build.
>
> That's interesting because it implies that it's okay for people to use
> them without "// PANIC..." comments. That sounds problematic since it
> means some instances will have that comment while others may not.

My personal view has always been it's okay to not have it. It's a burden to
developers to always have to write these. In many cases, `panic()` or `expect()`
calls are needed because you have something of `Option<>` and you know it's not
going to be `None`. The C equivalent would just be a pointer and you use it
without checking for NULL; we never ask people to justify why they feel it's
okay to dereference a pointer.

Sure, if people would like to justify why they think it won't panic, brilliant,
keeping doing it. But I don't want to make it harder for people to write Rust
code compared to C.

>
> In my opinion, adding a clippy rule and using "#[allow(...)]" in the
> places where it's acceptable to use them makes more sense. This is at
> least more consistent and doesn't bloat the checkpatch file.

`#[allow()]` looks quite verbose, and also you cannot apply it everywhere (just
blocks or items).

Best,
Gary
Re: [PATCH v4] scripts: checkpatch: warn on Rust panicking methods
Posted by Dirk Behme 2 days, 11 hours ago
On 03.02.26 17:54, Gary Guo wrote:
> On Tue Feb 3, 2026 at 4:32 PM GMT, Onur Özkan wrote:
>> On Tue, 03 Feb 2026 16:02:02 +0000
>> "Gary Guo" <gary@garyguo.net> wrote:
>>
>>> On Tue Feb 3, 2026 at 3:49 PM GMT, Onur Özkan wrote:
>>>> On Tue,  3 Feb 2026 08:25:41 -0700
>>>> Jkhall81 <jason.kei.hall@gmail.com> wrote:
>>>>
>>>>> Nice, emails sent from gmail get automatically rejected.
>>>>>
>>>>> So, Dirk.  To satisfy your concerns the current 10ish line
>>>>> code update is going to slowly, after many more emails
>>>>> written in nano, mutate into a franken-regex-perl beast. 
>>>>> checkpatch.pl is already huge.  I'm not a fan of this 
>>>>> approach.
>>>>
>>>> Me neither. I wonder why we are doing this instead of using the
>>>> unwrap_used and expect_used linting rules from clippy. This would
>>>> catch the problem much earlier than checkpath since many of us build
>>>> the kernel with CLIPPY=1 flag.
>>>
>>> Because it's okay to `panic` or use `expect`. checkpatch will just
>>> warn you once when the code is introduced, not continuously in each
>>> build.
>>
>> That's interesting because it implies that it's okay for people to use
>> them without "// PANIC..." comments. That sounds problematic since it
>> means some instances will have that comment while others may not.
> 
> My personal view has always been it's okay to not have it. It's a burden to
> developers to always have to write these. In many cases, `panic()` or `expect()`
> calls are needed because you have something of `Option<>` and you know it's not
> going to be `None`. The C equivalent would just be a pointer and you use it
> without checking for NULL; we never ask people to justify why they feel it's
> okay to dereference a pointer.
> 
> Sure, if people would like to justify why they think it won't panic, brilliant,
> keeping doing it. But I don't want to make it harder for people to write Rust
> code compared to C.


The question is if we could find a way to make it *consistent*?

I mean how should a developer know if the warning (he gets once, or
even if he checks an existing file with -f always) is relevant or not?
We introduce the warning because we want to discourage the use of
`unwrap()`. At the same time there are places where its usage is
allowed or even needed. How to know what is valid? The warning or the
usage?

So do we find an acceptable way to mark the allowed ones? Or do we
drop this patch entirely and hope to catch the "forbidden" ones by
review? Or?

Best regards

Dirk


>> In my opinion, adding a clippy rule and using "#[allow(...)]" in the
>> places where it's acceptable to use them makes more sense. This is at
>> least more consistent and doesn't bloat the checkpatch file.
> 
> `#[allow()]` looks quite verbose, and also you cannot apply it everywhere (just
> blocks or items).
> 
> Best,
> Gary
> 
> 

Re: [PATCH v4] scripts: checkpatch: warn on Rust panicking methods
Posted by Miguel Ojeda 2 days, 9 hours ago
On Wed, Feb 4, 2026 at 4:56 PM Dirk Behme <dirk.behme@gmail.com> wrote:
>
> The question is if we could find a way to make it *consistent*?
>
> I mean how should a developer know if the warning (he gets once, or
> even if he checks an existing file with -f always) is relevant or not?
> We introduce the warning because we want to discourage the use of
> `unwrap()`. At the same time there are places where its usage is
> allowed or even needed. How to know what is valid? The warning or the
> usage?

I think usually developers use `checkpatch.pl` mostly on patches, not
existing files; plus it doesn't make the build fail. Thus I see
`checkpatch.pl` as a tool that can have way more false positives than
a linter that we need to keep strictly clean.

The idea with the `checkpatch.pl` warning was to have something we
could land easily before we got the new Clippy lints, and perhaps to
apply it in more cases than the eventual Clippy lint (since false
positives are not as concerning).

I have some context in
https://github.com/rust-lang/rust-clippy/issues/15895 -> "Additional
context", and a few other issues linked in
https://github.com/Rust-for-Linux/linux/issues/349 for the new lints.

Cheers,
Miguel
Re: [PATCH v4] scripts: checkpatch: warn on Rust panicking methods
Posted by Dirk Behme 1 day, 14 hours ago
On 04.02.2026 19:10, Miguel Ojeda wrote:
> On Wed, Feb 4, 2026 at 4:56 PM Dirk Behme <dirk.behme@gmail.com> wrote:
>>
>> The question is if we could find a way to make it *consistent*?
>>
>> I mean how should a developer know if the warning (he gets once, or
>> even if he checks an existing file with -f always) is relevant or not?
>> We introduce the warning because we want to discourage the use of
>> `unwrap()`. At the same time there are places where its usage is
>> allowed or even needed. How to know what is valid? The warning or the
>> usage?
> 
> I think usually developers use `checkpatch.pl` mostly on patches, not
> existing files; plus it doesn't make the build fail. Thus I see
> `checkpatch.pl` as a tool that can have way more false positives than
> a linter that we need to keep strictly clean.

And how would a developer know that a `checkpatch.pl` warning on 
`unwrap()` is a false positive or not (i.e. is to be fixed)?

Thanks

Dirk


> The idea with the `checkpatch.pl` warning was to have something we
> could land easily before we got the new Clippy lints, and perhaps to
> apply it in more cases than the eventual Clippy lint (since false
> positives are not as concerning).
> 
> I have some context in
> https://github.com/rust-lang/rust-clippy/issues/15895 -> "Additional
> context", and a few other issues linked in
> https://github.com/Rust-for-Linux/linux/issues/349 for the new lints.
> 
> Cheers,
> Miguel

Re: [PATCH v4] scripts: checkpatch: warn on Rust panicking methods
Posted by Miguel Ojeda 1 day, 6 hours ago
On Thu, Feb 5, 2026 at 2:24 PM Dirk Behme <dirk.behme@de.bosch.com> wrote:
>
> And how would a developer know that a `checkpatch.pl` warning on
> `unwrap()` is a false positive or not (i.e. is to be fixed)?

It depends on the context, like some other `checkpatch.pl`
errors/warnings, i.e. there is always some degree of developer
discretion. Or, at least, that is the way I think about
`checkpatch.pl` warnings, but maybe the maintainers think otherwise
(Cc'ing).

To give concrete advice, we could perhaps link to
https://rust.docs.kernel.org/kernel/error/type.Result.html#error-codes-in-c-and-rust,
for instance. (Please see my other reply on v5 about it).

Cheers,
Miguel
Re: [PATCH v4] scripts: checkpatch: warn on Rust panicking methods
Posted by Joe Perches 2 days, 8 hours ago
On Wed, 2026-02-04 at 19:10 +0100, Miguel Ojeda wrote:
> On Wed, Feb 4, 2026 at 4:56 PM Dirk Behme <dirk.behme@gmail.com> wrote:
> > 
> > The question is if we could find a way to make it *consistent*?
> > 
> > I mean how should a developer know if the warning (he gets once, or
> > even if he checks an existing file with -f always) is relevant or not?
> > We introduce the warning because we want to discourage the use of
> > `unwrap()`. At the same time there are places where its usage is
> > allowed or even needed. How to know what is valid? The warning or the
> > usage?
> 
> I think usually developers use `checkpatch.pl` mostly on patches, not
> existing files; plus it doesn't make the build fail. Thus I see
> `checkpatch.pl` as a tool that can have way more false positives than
> a linter that we need to keep strictly clean.
> 
> The idea with the `checkpatch.pl` warning was to have something we
> could land easily before we got the new Clippy lints, and perhaps to
> apply it in more cases than the eventual Clippy lint (since false
> positives are not as concerning).
> 
> I have some context in
> https://github.com/rust-lang/rust-clippy/issues/15895 -> "Additional
> context", and a few other issues linked in
> https://github.com/Rust-for-Linux/linux/issues/349 for the new lints.

Maybe adding something like this to checkpatch so rust
specific checks and possibly other execs could be added
relatively easily in checkpatch's process() 

	process_rust() if ($realfile =~ /\.rs$/);

---
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index e56374662ff79..bd9daa77470a5 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -20,6 +20,8 @@ my $D = dirname(abs_path($P));
 
 my $V = '0.32';
 
+require "$D/rust_checkpatch.pl";
+
 use Getopt::Long qw(:config no_auto_abbrev);
 
 my $quiet = 0;
[PATCH v5] scripts: checkpatch: move Rust-specific lints to separate file
Posted by Jason Hall 2 days, 1 hour ago
Move Rust linting logic into scripts/rust_checkpatch.pl to prevent
further growth of the main checkpatch.pl script.

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: Joe Perches <joe@perches.com>
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>

---
v5:
 - Move Rust-specific logic to scripts/rust_checkpatch.pl (Suggested by Joe Perches).
 - Add conditional loading hook in scripts/checkpatch.pl.
 - Updated regex to use house-style comments.
v4:
 - Use imperative mood in commit description.
 - Fix patch formatting and placement of '---' separator.
v3:
 - Use non-capturing groups (?:) to optimize regex.
v2:
 - Switch from \b to (\.|::) to avoid false positives in strings.
 - Add : to PANIC comment check.
---
 scripts/checkpatch.pl      | 14 ++++++++++++++
 scripts/rust_checkpatch.pl | 25 +++++++++++++++++++++++++
 2 files changed, 39 insertions(+)
 create mode 100644 scripts/rust_checkpatch.pl

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index c0250244cf7a..f75cb70ad0dd 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -20,6 +20,12 @@ my $D = dirname(abs_path($P));
 
 my $V = '0.32';
 
+my $rust_checkpatch_available = 0;
+if (-e "$D/rust_checkpatch.pl") {
+	require "$D/rust_checkpatch.pl";
+	$rust_checkpatch_available = 1;
+}
+
 use Getopt::Long qw(:config no_auto_abbrev);
 
 my $quiet = 0;
@@ -2947,6 +2953,14 @@ sub process {
 
 		$cnt_lines++ if ($realcnt != 0);
 
+# Check for Rust-specific lints
+		if ($rust_checkpatch_available && $realfile =~ /\.rs$/) {
+			my ($type, $msg) = process_rust($line, $rawline, $herecurr);
+			if ($type) {
+				WARN($type, $msg);
+			}
+		}
+
 # Verify the existence of a commit log if appropriate
 # 2 is used because a $signature is counted in $commit_log_lines
 		if ($in_commit_log) {
diff --git a/scripts/rust_checkpatch.pl b/scripts/rust_checkpatch.pl
new file mode 100644
index 000000000000..69db5ded7371
--- /dev/null
+++ b/scripts/rust_checkpatch.pl
@@ -0,0 +1,25 @@
+#!/usr/bin/env perl
+# SPDX-License-Identifier: GPL-2.0
+#
+# (c) 2026, Jason K. Hall <jason.kei.hall@gmail.com>
+
+use strict;
+use warnings;
+
+sub process_rust {
+    my ($line, $rawline, $herecurr) = @_;
+
+    # check for Rust unwrap/expect
+    if ($line =~ /^\+/) {
+        if ($line =~ /(?:\.|::)(?:unwrap|expect)\s*\(/ &&
+            $rawline !~ /\/\/\s*PANIC:/ &&
+            $line !~ /^\+\s*\/\// &&
+            $line !~ /^\+\s*assert/) {
+            return ("RUST_UNWRAP",
+                    "Avoid unwrap() or expect() in Rust code; use proper error handling (Result) or justify with a '// PANIC: ...' comment.\n" . $herecurr);
+        }
+    }
+    return ();
+}
+
+1;
\ No newline at end of file
-- 
2.43.0
Re: [PATCH v5] scripts: checkpatch: move Rust-specific lints to separate file
Posted by Miguel Ojeda 1 day, 6 hours ago
On Thu, Feb 5, 2026 at 2:42 AM Jason Hall <jason.kei.hall@gmail.com> wrote:
>
> Move Rust linting logic into scripts/rust_checkpatch.pl to prevent
> further growth of the main checkpatch.pl script.
>
> 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: Joe Perches <joe@perches.com>
> 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>

Ideally, this would be two patches -- one performing the move and the
other adding the new lint.

Either way, the file should be added to `MAINTAINERS` -- not sure if
Joe would want to maintain it or perhaps have a new sub-entry or
perhaps a co-maintainer etc.

> +                    "Avoid unwrap() or expect() in Rust code; use proper error handling (Result) or justify with a '// PANIC: ...' comment.\n" . $herecurr);

By "proper error handling", I guess you mean bubbling the error up
with `?` or similar, right?

There are also other options, like using `unsafe` sometimes etc.

I think perhaps the best is to keep the message a bit less strict
(since there are cases where it is OK) and put the details in a link
to, say, https://rust.docs.kernel.org/kernel/error/type.Result.html#error-codes-in-c-and-rust
(Cc'ing Dirk who was involved in that).

I am thinking of something like:

    `unwrap()` and `expect()` should generally be avoided in Rust
kernel code modulo some exceptions (e.g. within asserts in doctests).
If the use is intended, then please justify it with a `// PANIC:`
comment. Please see ...

But I will let others comment...

(Also, please Cc all the maintainers/reviews of Rust and checkpatch --
doing so here).

Thanks!

Cheers,
Miguel
Re: [PATCH v5] scripts: checkpatch: move Rust-specific lints to separate file
Posted by Dirk Behme 18 hours ago
On 05.02.2026 21:55, Miguel Ojeda wrote:
> On Thu, Feb 5, 2026 at 2:42 AM Jason Hall <jason.kei.hall@gmail.com> wrote:
>>
>> Move Rust linting logic into scripts/rust_checkpatch.pl to prevent
>> further growth of the main checkpatch.pl script.
>>
>> 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: Joe Perches <joe@perches.com>
>> 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>
> 
> Ideally, this would be two patches -- one performing the move and the
> other adding the new lint.
> 
> Either way, the file should be added to `MAINTAINERS` -- not sure if
> Joe would want to maintain it or perhaps have a new sub-entry or
> perhaps a co-maintainer etc.
> 
>> +                    "Avoid unwrap() or expect() in Rust code; use proper error handling (Result) or justify with a '// PANIC: ...' comment.\n" . $herecurr);
> 
> By "proper error handling", I guess you mean bubbling the error up
> with `?` or similar, right?
> 
> There are also other options, like using `unsafe` sometimes etc.
> 
> I think perhaps the best is to keep the message a bit less strict
> (since there are cases where it is OK) and put the details in a link
> to, say, https://rust.docs.kernel.org/kernel/error/type.Result.html#error-codes-in-c-and-rust
> (Cc'ing Dirk who was involved in that).
> 
> I am thinking of something like:
> 
>      `unwrap()` and `expect()` should generally be avoided in Rust


Gary had some concerns about `expect()` in

https://lore.kernel.org/rust-for-linux/DG3VYAW3TXEO.1YG8N99YWCDR6@garyguo.net/


> kernel code modulo some exceptions (e.g. within asserts in doctests

... and `tests`)

? See

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/alloc/kvec.rs?h=v6.19-rc8#n1355

).
> If the use is intended, then please justify it with a `// PANIC:`
> comment. Please see ...


I'm totally fine with this. In consequence do we agree that this will 
result in some follow up patches annotating the existing "allowed" 
exceptional / intended cases (e.g. in doctests and tests) with `// 
PANIC:` comments?

Best regards

Dirk



> But I will let others comment...
> 
> (Also, please Cc all the maintainers/reviews of Rust and checkpatch --
> doing so here).
> 
> Thanks!
> 
> Cheers,
> Miguel

Re: [PATCH v5] scripts: checkpatch: move Rust-specific lints to separate file
Posted by Miguel Ojeda 9 hours ago
On Fri, Feb 6, 2026 at 9:32 AM Dirk Behme <dirk.behme@de.bosch.com> wrote:
>
> Gary had some concerns about `expect()` in

Yeah, for `expect()` in `checkpatch.pl`, the current message does not
make much sense, i.e. perhaps we want a slightly different one,
without the `// PANIC` bit.

Or perhaps, like Gary says, we don't warn for that one, at least for
now. I think we discussed pros/cons of using `expect()` to begin with
recently -- Gary?

> I'm totally fine with this. In consequence do we agree that this will
> result in some follow up patches annotating the existing "allowed"
> exceptional / intended cases (e.g. in doctests and tests) with `//
> PANIC:` comments?

Those are the ones that would not need annotations, i.e. they would be
false positives.

As far as I remember, the idea with the Clippy lint was to allow it in
certain places like e.g. `assert()`s inside doctests for sure (and
perhaps in doctests in general). Thus no `// PANIC:` needed for
certain cases.

And here for `checkpatch.pl`, if it were to trigger in a similar
situation, then it would be a false positive, and thus the developer
should not add it. But the patch here skips if it is inside a comment
(or doctest), no?

So the remaining concern if I understand correctly is too many false
positives inside `#[test]`s but outside an `assert()`s? Yeah, we may
need to track if we have seen `kunit_tests` (and reset if we go out of
the test module), to skip a bunch of warnings.

By the way, I would prefer we expand the comment on top of the line
explaining what it is supposed to cover, in order to evaluate whether
the regexes etc. are doing what we expect or not.

Thanks Dirk et al. for diving into this -- we do need to move forward
this topic, and there have been some disagreements on how much is too
much to warn about :)

Cheers,
Miguel