All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: Christoph Jaeger <cj@linux.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Julia Lawall <julia.lawall@lip6.fr>
Cc: Alan <alan@linux.intel.com>,
	davem@davemloft.net, willemb@google.com, edumazet@google.com,
	dborkman@redhat.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH] checkpatch: Add likely/unlikely comparison misuse test
Date: Sun, 11 Jan 2015 11:49:57 -0800	[thread overview]
Message-ID: <1421005797.9233.3.camel@perches.com> (raw)
In-Reply-To: <20150111193404.GN1513@betelgeuse.hsd1.ma.comcast.net>

Add a test for probably likely/unlikely misuses where
the comparison is likely misplaced

	if (likely(foo) > 0)
vs
	if (likely(foo > 0))

Signed-off-by: Joe Perches <joe@perches.com>
---
On Sun, 2015-01-11 at 14:34 -0500, Christoph Jaeger wrote:
> > drivers/platform/goldfish/goldfish_pipe.c:285:	if (unlikely(bufflen) == 0)
> 
> Well, the conditional statement works as intended. Of course, the branch
> prediction doesn't.
> 
> Coccinelle should be able to check for this kind of likely()/unlikely() usage,
> shouldn't it?

Most likely,  checkpatch could too, but not as well.
This misuse isn't very common. (2 in current source?)

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

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 6afc24b..b8d47dc 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5219,6 +5219,13 @@ sub process {
 			      "#define of '$1' is wrong - use Kconfig variables or standard guards instead\n" . $herecurr);
 		}
 
+# likely/unlikely comparisons similar to "(likely(foo) > 0)"
+		if ($^V && $^V ge 5.10.0 &&
+		    $line =~ /\b((?:un)?likely)\s*\(\s*$FuncArg\s*\)\s*$Compare/) {
+			WARN("LIKELY_MISUSE",
+			     "Using $1 should generally have parentheses around the comparison\n" . $herecurr);
+		}
+
 # whine mightly about in_atomic
 		if ($line =~ /\bin_atomic\s*\(/) {
 			if ($realfile =~ m@^drivers/@) {



  reply	other threads:[~2015-01-11 19:50 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-11 18:01 [PATCH net] packet: bail out of packet_snd() if L2 header creation fails Christoph Jaeger
2015-01-11 18:35 ` Eric Dumazet
2015-01-11 18:49   ` Willem de Bruijn
2015-01-11 18:52 ` Joe Perches
2015-01-11 19:34   ` Christoph Jaeger
2015-01-11 19:49     ` Joe Perches [this message]
2015-01-11 21:38 ` Daniel Borkmann
2015-01-12  2:54 ` David Miller

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=1421005797.9233.3.camel@perches.com \
    --to=joe@perches.com \
    --cc=akpm@linux-foundation.org \
    --cc=alan@linux.intel.com \
    --cc=cj@linux.com \
    --cc=davem@davemloft.net \
    --cc=dborkman@redhat.com \
    --cc=edumazet@google.com \
    --cc=julia.lawall@lip6.fr \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=willemb@google.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.