linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@au1.ibm.com>,
	Ingo Molnar <mingo@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	"H. Peter Anvin" <hpa@zytor.com>,
	James Hogan <james.hogan@imgtec.com>,
	"James E.J. Bottomley" <jejb@parisc-linux.org>,
	Helge Deller <deller@gmx.de>,
	Martin Schwidefsky <schwidefsky@de.ibm.com>,
	Heiko Carstens <heiko.carstens@de.ibm.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: [PATCH] checkpatch: Add test for #defines of ARCH_HAS_<foo>
Date: Wed, 25 Sep 2013 19:00:54 -0700	[thread overview]
Message-ID: <1380160854.17366.49.camel@joe-AO722> (raw)
In-Reply-To: <CA+55aFycQ9XJvEOsiM3txHL5bjUc8CeKWJNR_H+MiicaddB42Q@mail.gmail.com>

On Wed, 2013-09-25 at 18:26 -0700, Linus Torvalds wrote:
> On Wed, Sep 25, 2013 at 5:40 PM, Joe Perches <joe@perches.com> wrote:
> >
> > Huh?  That matches all the ARCH_HAS_<foo> patterns.
> 
> Right. And they are all crap. lib/string.c is a prime example of
> something that should never have happened.
> 
> The ARCH_HAS_xyz pattern is totally retarded. It's wrong.
> 
> For big conceptual features, we should use Kconfig symbols.
> 
> And for smaller things - like lib/string.c - where we have
> compatibility fallback functions but want architectures able to
> override them with optimized ones one function at a time, we should
> either use weak functions (appropriate for some cases), or the symbol
> that protects them should the the SAME SYMBOL WE USE. Rather than some
> made-up crap-for-brains new ARCH_HAS_xyz symbol. That way it shows up
> in greps, and that way we don't have any question about what random
> symbol pattern we use that particular day.
> 
> So for *bad* use, see lib/string.c, and the ARCH_AS_xyz horror.
> 
> For *good* use, see lib/div64.c or lib/find_next_bit.c.
> 
> Notice how div64.c doesn't make up new ARCH_HAS_random_crap names? And
> no, you don't have to define those things as macros, you can define
> them as functions (inline or not), and then just do
> 
>     #define find_next_zero_bit find_next_zero_bit
> 
> to tell the rest of the world "Look, I have this defined".
> 
> The whole "make up a totally unrelated second name for it" means that
> we have things like __HAVE_ARCH_STRLEN but also things like
> ARCH_HAS_PREFETCHW. Ugh.
> 
>                        Linus

So, add a test for these #defines

Additionally, moved string_find_replace sub as it
screws up subsequent formatting when placed inside
another sub.

Suggested-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Joe Perches <joe@perches.com>
---
 scripts/checkpatch.pl | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index c03e427..e2e7703 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1512,6 +1512,14 @@ sub rtrim {
 	return $string;
 }
 
+sub string_find_replace {
+	my ($string, $find, $replace) = @_;
+
+	$string =~ s/$find/$replace/g;
+
+	return $string;
+}
+
 sub tabify {
 	my ($leading) = @_;
 
@@ -3731,14 +3739,6 @@ sub process {
 			}
 		}
 
-sub string_find_replace {
-	my ($string, $find, $replace) = @_;
-
-	$string =~ s/$find/$replace/g;
-
-	return $string;
-}
-
 # check for bad placement of section $InitAttribute (e.g.: __initdata)
 		if ($line =~ /(\b$InitAttribute\b)/) {
 			my $attr = $1;
@@ -4196,6 +4196,12 @@ sub string_find_replace {
 			     "usage of NR_CPUS is often wrong - consider using cpu_possible(), num_possible_cpus(), for_each_possible_cpu(), etc\n" . $herecurr);
 		}
 
+# Use of __ARCH_HAS_<FOO> or ARCH_HAVE_<BAR> is wrong.
+		if ($line =~ /\+\s*#\s*define\s+((?:__)?ARCH_(?:HAS|HAVE)\w*)\b/) {
+			ERROR("DEFINE_ARCH_HAS",
+			      "#define of '$1' is wrong - use Kconfig variables or standard guards instead\n" . $herecurr);
+		}
+
 # check for %L{u,d,i} in strings
 		my $string;
 		while ($line =~ /(?:^|")([X\t]*)(?:"|$)/g) {



  reply	other threads:[~2013-09-26  2:01 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-25 16:17 [RFC PATCH 0/7] softirq: Consolidation and stack overrun fix v2 Frederic Weisbecker
2013-09-25 16:18 ` [PATCH 1/7] irq: Force hardirq exit's softirq processing on its own stack Frederic Weisbecker
2013-09-25 16:18 ` [PATCH 2/7] irq: Consolidate do_softirq() arch overriden implementations Frederic Weisbecker
2013-09-25 16:18 ` [PATCH 3/7] irq: Optimize call to softirq on hardirq exit Frederic Weisbecker
2013-09-25 16:18 ` [PATCH 4/7] irq: Improve a bit softirq debugging Frederic Weisbecker
2013-09-25 16:18 ` [PATCH 5/7] irq: Justify the various softirq stack choices Frederic Weisbecker
2013-09-25 16:18 ` [PATCH 6/7] irq: Optimize softirq stack selection in irq exit Frederic Weisbecker
2013-09-25 23:04   ` Linus Torvalds
2013-09-26  7:12     ` Frederic Weisbecker
2013-09-25 16:18 ` [PATCH 7/7] x86: Tell about irq stack coverage Frederic Weisbecker
2013-09-25 23:08   ` Linus Torvalds
2013-09-26  0:21     ` Andrew Morton
2013-09-26  0:40       ` Joe Perches
2013-09-26  1:26         ` Linus Torvalds
2013-09-26  2:00           ` Joe Perches [this message]
2013-09-26  2:32             ` [PATCH] checkpatch: Add test for #defines of ARCH_HAS_<foo> Andrew Morton
2013-09-26  2:40               ` Joe Perches

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=1380160854.17366.49.camel@joe-AO722 \
    --to=joe@perches.com \
    --cc=akpm@linux-foundation.org \
    --cc=benh@kernel.crashing.org \
    --cc=davem@davemloft.net \
    --cc=deller@gmx.de \
    --cc=fweisbec@gmail.com \
    --cc=heiko.carstens@de.ibm.com \
    --cc=hpa@zytor.com \
    --cc=james.hogan@imgtec.com \
    --cc=jejb@parisc-linux.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=paulus@au1.ibm.com \
    --cc=peterz@infradead.org \
    --cc=schwidefsky@de.ibm.com \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).