From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail3-relais-sop.national.inria.fr (mail3-relais-sop.national.inria.fr [192.134.164.104]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1C25E3DA5CC for ; Mon, 10 Aug 2026 13:00:37 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=192.134.164.104 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786366842; cv=none; b=HFjAU7WbNuziR6gZbB5LsPiKy3cB/p3Ug07jMDzZ+A+WWAmkyrWvr0ijbysoJ3k2o//QdkOI+LbSDoIHrDk9gOv6yPCTU4hBeUKmhMvEfrv4dgLSXo0c1nvYPwlBRNqf7ZULQYYrx1WYMlTfGf09KC9ubC3LahD33F3TAJcPnZM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786366842; c=relaxed/simple; bh=fRzUD3xWe5xR/zXKBJnhBmEFmLgwjsz9wYnGEIIm3NM=; h=Date:From:To:cc:Subject:In-Reply-To:Message-ID:References: MIME-Version:Content-Type; b=E2BuN6fxBWrcynrPccyqVD+SYaN2hTsUOQFbyFlmbBD5CPDt6ZIdjJtcaUkhLg+lotvDLpb0xUO89oL21p/hLmZ5Ckj4u28OPW9/mzUEiCBVcq1VYI3mTPSJ5yCH/PlycnQ+A2fbSgWuw5bowJIertXFtWEEEHoalOjDR6qE8zo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=inria.fr; spf=pass smtp.mailfrom=inria.fr; dkim=pass (1024-bit key) header.d=inria.fr header.i=@inria.fr header.b=PiiPRbhj; arc=none smtp.client-ip=192.134.164.104 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=inria.fr Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=inria.fr Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=inria.fr header.i=@inria.fr header.b="PiiPRbhj" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=inria.fr; s=dc; h=date:from:to:cc:subject:in-reply-to:message-id: references:mime-version; bh=SK0uVMXKBvLDqzCJ0FaLrmDzbUdUESbnQv00OPAi1PI=; b=PiiPRbhjpu32yA9tebRFbIPXgmjDVDPhaTS4SeWgqoLnZ8uMBVk1JYd2 XPu7wATRUrQbN1bcz8Wl40OhFy4IsqP4jdwhOuV9YZqj9HQ+9SflHx6Bw s7aGAlvz6o+pu7k+vC//rJv0j04dO03+e8PC9zLBoKUa2Bi2TM/PPggRO E=; X-CSE-ConnectionGUID: FQ8tflZuTHia0pzZKhEHKw== X-CSE-MsgGUID: ANVVolDBQC20/UPNcLuo6Q== Authentication-Results: mail3-relais-sop.national.inria.fr; dkim=none (message not signed) header.i=none; spf=SoftFail smtp.mailfrom=julia.lawall@inria.fr; dmarc=fail (p=none dis=none) d=inria.fr X-IronPort-AV: E=Sophos;i="6.25,215,1779141600"; d="scan'208";a="153973074" Received: from dt-lawall.paris.inria.fr ([128.93.67.65]) by mail3-relais-sop.national.inria.fr with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Aug 2026 15:00:35 +0200 Date: Mon, 10 Aug 2026 15:00:34 +0200 (CEST) From: Julia Lawall To: Sang-Heon Jeon cc: Nicolas Palix , Jani Nikula , cocci@inria.fr, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] coccinelle: misc: add cond_return_no_effect.cocci In-Reply-To: <20260806174953.401869-1-ekffu200098@gmail.com> Message-ID: <429fd2aa-3447-e16c-63b9-20f501940c1@inria.fr> References: <20260806174953.401869-1-ekffu200098@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII 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 > --- > 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 > >