All of lore.kernel.org
 help / color / mirror / Atom feed
From: Utkarsh Verma <utkarshverma294@gmail.com>
To: Dwaipayan Ray <dwaipayanray1@gmail.com>
Cc: Lukas Bulwahn <lukas.bulwahn@gmail.com>,
	Joe Perches <joe@perches.com>, Jonathan Corbet <corbet@lwn.net>,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	Utkarsh Verma <utkarshverma294@gmail.com>
Subject: [PATCH] docs: checkpatch: add UNNECESSARY_ELSE message
Date: Fri,  1 Oct 2021 17:32:18 +0530	[thread overview]
Message-ID: <20211001120218.28751-1-utkarshverma294@gmail.com> (raw)
In-Reply-To: <06f4c72fefeedb5145a940e5a78d50e610acdcc4.camel@perches.com>

Added and documented UNNECESSARY_ELSE message type.

Signed-off-by: Utkarsh Verma <utkarshverma294@gmail.com>
---
 Documentation/dev-tools/checkpatch.rst | 77 ++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)

diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
index f0956e9ea2d8..e0f1ea1a0911 100644
--- a/Documentation/dev-tools/checkpatch.rst
+++ b/Documentation/dev-tools/checkpatch.rst
@@ -1166,3 +1166,80 @@ Others
 
   **TYPO_SPELLING**
     Some words may have been misspelled.  Consider reviewing them.
+
+  **UNNECESSARY_ELSE**
+    Using an else statement just after a return or a break statement is
+    unnecessary. For example::
+
+      for (i = 0; i < 100; i++) {
+              int foo = bar();
+              if (foo < 1)
+                      break;
+              else
+                      usleep(1);
+      }
+
+    is generally better written as::
+
+      for (i = 0; i < 100; i++) {
+              int foo = bar();
+              if (foo < 1)
+                      break;
+              usleep(1);
+      }
+
+    It helps to reduce the indentation and removes the unnecessary else
+    statement. But note there can be some false positives because of the
+    way it is implemented in the checkpatch script. The checkpatch script
+    throws this warning message if it finds an else statement and the line
+    above it is a break or return statement indented at one tab more than
+    the else statement. So there can be some false positives like::
+
+      int n = 15;
+      if (n > 10)
+              n--;
+      else if (n == 10)
+              return 0;
+      else
+              n++;
+
+    Now the checkpatch will give a warning for the use of else after return
+    statement. If the else statement is removed then::
+
+      int n = 15;
+      if (n > 10)
+              n--;
+      else if (n == 10)
+              return 0;
+      n++;
+
+    Now both the n-- and n++ statements will be executed which is different
+    from the logic in the first case. Because the if block doesn't have
+    a return statement, so removing the else statement is wrong.
+
+    Always check the previous if/else if blocks, for break/return
+    statements, and do not blindly follow the checkpatch advice. One
+    patch https://lore.kernel.org/all/20200615155131.GA4563@sevic69/
+    even made it to the mainline, which was again reverted and fixed.
+    Commit 98fe05e21a6e ("staging: rtl8712: Remove unnecesary else
+    after return statement.")
+
+    Also, do not change the code if there is only a single return/break
+    statement inside if-else block, like::
+
+      if (a > b)
+              return a;
+      else
+              return b;
+
+    now if the else statement is removed::
+
+      if (a > b)
+              return a;
+      return b;
+
+    there is no considerable increase in the readability and one can argue
+    that the first form is more readable because of the indentation. So
+    do not remove the else statement in case of a single return/break
+    statements inside the if-else block.
+    See: https://lore.kernel.org/lkml/20140925032215.GK7996@ZenIV.linux.org.uk/
-- 
2.25.1


  reply	other threads:[~2021-10-01 12:03 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-25 20:17 [PATCH] Documentation: checkpatch: Document some more message types Utkarsh Verma
2021-09-27 17:43 ` Jonathan Corbet
2021-09-27 17:47   ` Utkarsh Verma
2021-09-27 17:53   ` Joe Perches
2021-10-01 12:02     ` Utkarsh Verma [this message]
2021-10-02 14:45       ` [PATCH v2] docs: checkpatch: add UNNECESSARY_ELSE message Utkarsh Verma
2021-10-03  4:38         ` Dwaipayan Ray
2021-10-03  5:08           ` Lukas Bulwahn
2021-10-03  5:19           ` Utkarsh Verma
2021-10-03  5:31             ` Lukas Bulwahn
2021-10-03  5:23         ` Lukas Bulwahn
2021-10-01 19:09     ` [PATCH] Documentation: checkpatch: Document some more message types Utkarsh Verma
2021-10-01 19:26       ` [PATCH] checkpatch: add check for continue statement in UNNECESSARY_ELSE Utkarsh Verma
2021-10-01 20:22         ` Joe Perches
2021-10-01 20:47           ` [PATCH v2] " Utkarsh Verma
2021-10-01 21:09             ` Joe Perches
2021-10-02 14:02               ` [PATCH v3] " Utkarsh Verma
2021-10-03  5:12                 ` Lukas Bulwahn
2021-09-29  5:28 ` [PATCH] Documentation: checkpatch: Document some more message types Lukas Bulwahn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211001120218.28751-1-utkarshverma294@gmail.com \
    --to=utkarshverma294@gmail.com \
    --cc=corbet@lwn.net \
    --cc=dwaipayanray1@gmail.com \
    --cc=joe@perches.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lukas.bulwahn@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.