[PATCH v2] coccinelle: misc: add cond_return_no_effect.cocci

[PATCH v2] coccinelle: misc: add cond_return_no_effect.cocci

[Sang-Heon Jeon]

Add a new Coccinelle script which removes a conditional return that
has no effect:

	if (ret)
		return ret;
	return ret;

Both branches return the same value, so the check can be removed.
The condition can also be a negation or a comparison with a
constant. Such code is usually a leftover from removing a
statement between the two returns.

When a local variable is assigned right before the check, the
assignment and the two returns turn into a single return of the
assigned expression, and the declaration is dropped if nothing
else uses the variable. Otherwise only the check is removed.

The fold can delete comments between the check and the final
return, so the generated patch should be reviewed.

Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
---
Changes from v1 [1]
- fix unexpected removal of global or static declarations, as Julia
  suggested
- send the patch separately from the treewide series, as Mark
  suggested

[1] https://lore.kernel.org/all/20260723184538.3888637-1-ekffu200098@gmail.com/
---
In the v1 thread, Jani shared the history of removing a similar
script that matched an explicit return 0 at the end [1].

Current status of the cleanup patches, two weeks after v1:
- 17/35 merged (2 sites changed to explicit return 0 as requested)
- 3/35 reviewed
- 15/35 no response yet

[1] https://lore.kernel.org/all/0ee1ef4aa7daa908bf28397ccc639c89b6aabd9c@intel.com/
---
 .../misc/cond_return_no_effect.cocci          | 143 ++++++++++++++++++
 1 file changed, 143 insertions(+)
 create mode 100644 scripts/coccinelle/misc/cond_return_no_effect.cocci

diff --git a/scripts/coccinelle/misc/cond_return_no_effect.cocci b/scripts/coccinelle/misc/cond_return_no_effect.cocci
new file mode 100644
index 000000000000..334a9bcd2d6e
--- /dev/null
+++ b/scripts/coccinelle/misc/cond_return_no_effect.cocci
@@ -0,0 +1,143 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Remove a conditional return that has no effect:
+///
+///	if (ret)
+///		return ret;
+///	return ret;
+///
+/// Both branches return the same variable, so the check has no
+/// effect. It can also be a negation or a comparison with a
+/// constant.
+///
+/// When a local variable is assigned right before the check, the
+/// assignment and the two returns turn into a single return of the
+/// assigned expression, and the declaration is dropped if nothing
+/// else uses the variable. Otherwise only the check is removed.
+///
+// Such code is usually a leftover from removing a statement between
+// the two returns.
+//
+// Confidence: High
+// Copyright: (C) 2026 Sang-Heon Jeon
+// Comments: The fold can delete comments between the check and the
+//           final return, so review the generated patch.
+// Options: --no-includes --include-headers
+
+virtual patch
+virtual context
+virtual org
+virtual report
+
+//----------------------------------------------------------
+//  For patch mode
+//----------------------------------------------------------
+
+@collect depends on patch@
+identifier ret;
+expression E;
+binary operator cmp = {<, <=, >, >=, ==, !=};
+constant C;
+@@
+	ret = E;
+	if (\(ret \| !ret \| ret cmp C\))
+		return ret;
+	return ret;
+
+@depends on patch@
+local idexpression ret;
+expression E;
+binary operator cmp = {<, <=, >, >=, ==, !=};
+constant C;
+@@
+-	ret = E;
+-	if (\(ret \| !ret \| ret cmp C\))
+-		return ret;
+-	return ret;
++	return E;
+
+@depends on patch@
+idexpression ret;
+binary operator cmp = {<, <=, >, >=, ==, !=};
+constant C;
+@@
+-	if (\(ret \| !ret \| ret cmp C\))
+-		return ret;
+	return ret;
+
+@depends on patch disable optional_storage@
+type T;
+identifier collect.ret;
+declaration D;
+statement S;
+@@
+(
+-	T ret;
+(
+	D
+|
+	S
+)
+&
+	T ret;
+	... when != ret
+	    when strict
+)
+
+@depends on patch disable optional_storage@
+type T;
+identifier collect.ret;
+constant C;
+declaration D;
+statement S;
+@@
+(
+-	T ret = C;
+(
+	D
+|
+	S
+)
+&
+	T ret = C;
+	... when != ret
+	    when strict
+)
+
+
+//----------------------------------------------------------
+//  For context mode
+//----------------------------------------------------------
+
+@depends on context@
+idexpression ret;
+binary operator cmp = {<, <=, >, >=, ==, !=};
+constant C;
+@@
+*	if (\(ret \| !ret \| ret cmp C\))
+*		return ret;
+	return ret;
+
+//----------------------------------------------------------
+//  For org and report mode
+//----------------------------------------------------------
+
+@r depends on org || report@
+idexpression ret;
+binary operator cmp = {<, <=, >, >=, ==, !=};
+constant C;
+position p;
+@@
+	if@p (\(ret \| !ret \| ret cmp C\))
+		return ret;
+	return ret;
+
+@script:python depends on org@
+p << r.p;
+@@
+cocci.print_main("WARNING: conditional return with no effect (both branches return the same value)", p)
+
+@script:python depends on report@
+p << r.p;
+@@
+coccilib.report.print_report(p[0], "WARNING: conditional return with no effect (both branches return the same value)")
-- 
2.43.0

Re: [PATCH v2] coccinelle: misc: add cond_return_no_effect.cocci

[Julia Lawall]

Jani,

Yes or no?  The resulting patches have mostly gotten a positive response
so far, but maybe this is something that one doesn't mind seeing from time
to time, but doesn't want to be bothered with on a daily basis?

It is possible to make all the rules depend on something that has to be
provided as a special argument on the command line.  This introduces a
small barrier to entry...

thanks,
julia

On Fri, 7 Aug 2026, Sang-Heon Jeon wrote:

> Add a new Coccinelle script which removes a conditional return that
> has no effect:
>
> 	if (ret)
> 		return ret;
> 	return ret;
>
> Both branches return the same value, so the check can be removed.
> The condition can also be a negation or a comparison with a
> constant. Such code is usually a leftover from removing a
> statement between the two returns.
>
> When a local variable is assigned right before the check, the
> assignment and the two returns turn into a single return of the
> assigned expression, and the declaration is dropped if nothing
> else uses the variable. Otherwise only the check is removed.
>
> The fold can delete comments between the check and the final
> return, so the generated patch should be reviewed.
>
> Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
> ---
> Changes from v1 [1]
> - fix unexpected removal of global or static declarations, as Julia
>   suggested
> - send the patch separately from the treewide series, as Mark
>   suggested
>
> [1] https://lore.kernel.org/all/20260723184538.3888637-1-ekffu200098@gmail.com/
> ---
> In the v1 thread, Jani shared the history of removing a similar
> script that matched an explicit return 0 at the end [1].
>
> Current status of the cleanup patches, two weeks after v1:
> - 17/35 merged (2 sites changed to explicit return 0 as requested)
> - 3/35 reviewed
> - 15/35 no response yet
>
> [1] https://lore.kernel.org/all/0ee1ef4aa7daa908bf28397ccc639c89b6aabd9c@intel.com/
> ---
>  .../misc/cond_return_no_effect.cocci          | 143 ++++++++++++++++++
>  1 file changed, 143 insertions(+)
>  create mode 100644 scripts/coccinelle/misc/cond_return_no_effect.cocci
>
> diff --git a/scripts/coccinelle/misc/cond_return_no_effect.cocci b/scripts/coccinelle/misc/cond_return_no_effect.cocci
> new file mode 100644
> index 000000000000..334a9bcd2d6e
> --- /dev/null
> +++ b/scripts/coccinelle/misc/cond_return_no_effect.cocci
> @@ -0,0 +1,143 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +///
> +/// Remove a conditional return that has no effect:
> +///
> +///	if (ret)
> +///		return ret;
> +///	return ret;
> +///
> +/// Both branches return the same variable, so the check has no
> +/// effect. It can also be a negation or a comparison with a
> +/// constant.
> +///
> +/// When a local variable is assigned right before the check, the
> +/// assignment and the two returns turn into a single return of the
> +/// assigned expression, and the declaration is dropped if nothing
> +/// else uses the variable. Otherwise only the check is removed.
> +///
> +// Such code is usually a leftover from removing a statement between
> +// the two returns.
> +//
> +// Confidence: High
> +// Copyright: (C) 2026 Sang-Heon Jeon
> +// Comments: The fold can delete comments between the check and the
> +//           final return, so review the generated patch.
> +// Options: --no-includes --include-headers
> +
> +virtual patch
> +virtual context
> +virtual org
> +virtual report
> +
> +//----------------------------------------------------------
> +//  For patch mode
> +//----------------------------------------------------------
> +
> +@collect depends on patch@
> +identifier ret;
> +expression E;
> +binary operator cmp = {<, <=, >, >=, ==, !=};
> +constant C;
> +@@
> +	ret = E;
> +	if (\(ret \| !ret \| ret cmp C\))
> +		return ret;
> +	return ret;
> +
> +@depends on patch@
> +local idexpression ret;
> +expression E;
> +binary operator cmp = {<, <=, >, >=, ==, !=};
> +constant C;
> +@@
> +-	ret = E;
> +-	if (\(ret \| !ret \| ret cmp C\))
> +-		return ret;
> +-	return ret;
> ++	return E;
> +
> +@depends on patch@
> +idexpression ret;
> +binary operator cmp = {<, <=, >, >=, ==, !=};
> +constant C;
> +@@
> +-	if (\(ret \| !ret \| ret cmp C\))
> +-		return ret;
> +	return ret;
> +
> +@depends on patch disable optional_storage@
> +type T;
> +identifier collect.ret;
> +declaration D;
> +statement S;
> +@@
> +(
> +-	T ret;
> +(
> +	D
> +|
> +	S
> +)
> +&
> +	T ret;
> +	... when != ret
> +	    when strict
> +)
> +
> +@depends on patch disable optional_storage@
> +type T;
> +identifier collect.ret;
> +constant C;
> +declaration D;
> +statement S;
> +@@
> +(
> +-	T ret = C;
> +(
> +	D
> +|
> +	S
> +)
> +&
> +	T ret = C;
> +	... when != ret
> +	    when strict
> +)
> +
> +
> +//----------------------------------------------------------
> +//  For context mode
> +//----------------------------------------------------------
> +
> +@depends on context@
> +idexpression ret;
> +binary operator cmp = {<, <=, >, >=, ==, !=};
> +constant C;
> +@@
> +*	if (\(ret \| !ret \| ret cmp C\))
> +*		return ret;
> +	return ret;
> +
> +//----------------------------------------------------------
> +//  For org and report mode
> +//----------------------------------------------------------
> +
> +@r depends on org || report@
> +idexpression ret;
> +binary operator cmp = {<, <=, >, >=, ==, !=};
> +constant C;
> +position p;
> +@@
> +	if@p (\(ret \| !ret \| ret cmp C\))
> +		return ret;
> +	return ret;
> +
> +@script:python depends on org@
> +p << r.p;
> +@@
> +cocci.print_main("WARNING: conditional return with no effect (both branches return the same value)", p)
> +
> +@script:python depends on report@
> +p << r.p;
> +@@
> +coccilib.report.print_report(p[0], "WARNING: conditional return with no effect (both branches return the same value)")
> --
> 2.43.0
>
>