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

[Julia Lawall <julia.lawall@inria.fr>]

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