* [PATCH] checkpatch: Add test for positional misuse of section specifiers like __initdata
@ 2013-08-28 2:08 Joe Perches
2013-08-31 14:31 ` Andi Kleen
0 siblings, 1 reply; 4+ messages in thread
From: Joe Perches @ 2013-08-28 2:08 UTC (permalink / raw)
To: Andrew Morton
Cc: Andy Whitcroft, Russell King - ARM Linux, Guenter Roeck,
Jean Delvare, Andi Kleen, LKML, linux-arm-kernel
As discussed recently on the arm [1] and lm-sensors [2] lists, it is
possible to use section markers on variables in a way which gcc doesn't
understand (or at least not the way the developer intended):
static struct __initdata samsung_pll_clock exynos4_plls[nr_plls] = {
does NOT put exynos4_plls in the .initdata section. The __initdata
marker can be virtually anywhere on the line, EXCEPT right after
"struct". The preferred location is before the "=" sign if there is
one, or before the trailing ";" otherwise.
[1] http://permalink.gmane.org/gmane.linux.ports.arm.kernel/258149
[2] http://lists.lm-sensors.org/pipermail/lm-sensors/2013-August/039836.html
So, update checkpatch to find these misuses and report an
error when it's immediately after struct or union, and a
warning when it's otherwise not immediately before the ; or =.
A similar patch was suggested by Andi Kleen
https://lkml.org/lkml/2013/8/5/648
Suggested-by: Jean Delvare <khali@linux-fr.org>
Signed-off-by: Joe Perches <joe@perches.com>
Tested-by: Guenter Roeck <linux@roeck-us.net>
---
scripts/checkpatch.pl | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 46 insertions(+), 1 deletion(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9ba4fc4..47016c3 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -242,6 +242,8 @@ our $Sparse = qr{
__rcu
}x;
+our $InitAttribute = qr{__(?:mem|cpu|dev|net_|)(?:initdata|initconst|init\b)};
+
# Notes to $Attribute:
# We need \b after 'init' otherwise 'initconst' will cause a false positive in a check
our $Attribute = qr{
@@ -262,7 +264,7 @@ our $Attribute = qr{
__deprecated|
__read_mostly|
__kprobes|
- __(?:mem|cpu|dev|)(?:initdata|initconst|init\b)|
+ $InitAttribute|
____cacheline_aligned|
____cacheline_aligned_in_smp|
____cacheline_internodealigned_in_smp|
@@ -292,6 +294,7 @@ our $Operators = qr{
}x;
our $NonptrType;
+our $NonptrTypeWithAttr;
our $Type;
our $Declare;
@@ -354,6 +357,12 @@ our @typeList = (
qr{${Ident}_handler},
qr{${Ident}_handler_fn},
);
+our @typeListWithAttr = (
+ @typeList,
+ qr{struct\s+$InitAttribute\s+$Ident},
+ qr{union\s+$InitAttribute\s+$Ident},
+);
+
our @modifierList = (
qr{fastcall},
);
@@ -367,6 +376,7 @@ our $allowed_asm_includes = qr{(?x:
sub build_types {
my $mods = "(?x: \n" . join("|\n ", @modifierList) . "\n)";
my $all = "(?x: \n" . join("|\n ", @typeList) . "\n)";
+ my $allWithAttr = "(?x: \n" . join("|\n ", @typeListWithAttr) . "\n)";
$Modifier = qr{(?:$Attribute|$Sparse|$mods)};
$NonptrType = qr{
(?:$Modifier\s+|const\s+)*
@@ -377,6 +387,15 @@ sub build_types {
)
(?:\s+$Modifier|\s+const)*
}x;
+ $NonptrTypeWithAttr = qr{
+ (?:$Modifier\s+|const\s+)*
+ (?:
+ (?:typeof|__typeof__)\s*\([^\)]*\)|
+ (?:$typeTypedefs\b)|
+ (?:${allWithAttr}\b)
+ )
+ (?:\s+$Modifier|\s+const)*
+ }x;
$Type = qr{
$NonptrType
(?:(?:\s|\*|\[\])+\s*const|(?:\s|\*|\[\])+|(?:\s*\[\s*\])+)?
@@ -3706,6 +3725,32 @@ 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;
+ if ($line =~ /^\+\s*static\s+(?:const\s+)?(?:$attr\s+)?($NonptrTypeWithAttr)\s+(?:$attr\s+)?($Ident(?:\[[^]]*\])?)\s*[=;]/) {
+ my $ptr = $1;
+ my $var = $2;
+ if ((($ptr =~ /\b(union|struct)\s+$attr\b/ &&
+ ERROR("MISPLACED_INIT",
+ "$attr should be placed after $var\n" . $herecurr)) ||
+ ($ptr !~ /\b(union|struct)\s+$attr\b/ &&
+ WARN("MISPLACED_INIT",
+ "$attr should be placed after $var\n" . $herecurr))) &&
+ $fix) {
+ $fixed[$linenr - 1] =~ s/(\bstatic\s+(?:const\s+)?)(?:$attr\s+)?($NonptrTypeWithAttr)\s+(?:$attr\s+)?($Ident(?:\[[^]]*\])?)\s*([=;])\s*/"$1" . trim(string_find_replace($2, "\\s*$attr\\s*", " ")) . " " . trim(string_find_replace($3, "\\s*$attr\\s*", "")) . " $attr" . ("$4" eq ";" ? ";" : " = ")/e;
+ }
+ }
+ }
+
# prefer usleep_range over udelay
if ($line =~ /\budelay\s*\(\s*(\d+)\s*\)/) {
# ignore udelay's < 10, however
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] checkpatch: Add test for positional misuse of section specifiers like __initdata
2013-08-28 2:08 [PATCH] checkpatch: Add test for positional misuse of section specifiers like __initdata Joe Perches
@ 2013-08-31 14:31 ` Andi Kleen
2013-08-31 16:04 ` Joe Perches
0 siblings, 1 reply; 4+ messages in thread
From: Andi Kleen @ 2013-08-31 14:31 UTC (permalink / raw)
To: Joe Perches
Cc: Andrew Morton, Andy Whitcroft, Russell King - ARM Linux,
Guenter Roeck, Jean Delvare, Andi Kleen, LKML, linux-arm-kernel
> A similar patch was suggested by Andi Kleen
> https://lkml.org/lkml/2013/8/5/648
My patch checked for const <-> initdata / non const initconst mistakes.
I don't think your patch does that?
-Andi
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] checkpatch: Add test for positional misuse of section specifiers like __initdata
2013-08-31 14:31 ` Andi Kleen
@ 2013-08-31 16:04 ` Joe Perches
2013-08-31 16:41 ` Andi Kleen
0 siblings, 1 reply; 4+ messages in thread
From: Joe Perches @ 2013-08-31 16:04 UTC (permalink / raw)
To: Andi Kleen
Cc: Andrew Morton, Andy Whitcroft, Russell King - ARM Linux,
Guenter Roeck, Jean Delvare, LKML, linux-arm-kernel
On Sat, 2013-08-31 at 16:31 +0200, Andi Kleen wrote:
> > A similar patch was suggested by Andi Kleen
> > https://lkml.org/lkml/2013/8/5/648
>
> My patch checked for const <-> initdata / non const initconst mistakes.
>
> I don't think your patch does that?
Hi Andi.
No it doesn't.
This patch is just warning when using
struct __initdata foo bar;
instead of
struct foo bar __initdata;
I'll get around to adding tests for your
cases soonish.
cheers, Joe
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] checkpatch: Add test for positional misuse of section specifiers like __initdata
2013-08-31 16:04 ` Joe Perches
@ 2013-08-31 16:41 ` Andi Kleen
0 siblings, 0 replies; 4+ messages in thread
From: Andi Kleen @ 2013-08-31 16:41 UTC (permalink / raw)
To: Joe Perches
Cc: Andi Kleen, Andrew Morton, Andy Whitcroft,
Russell King - ARM Linux, Guenter Roeck, Jean Delvare, LKML,
linux-arm-kernel
> I'll get around to adding tests for your
> cases soonish.
You could also just apply my patch?
AFAIK it works fine.
-Andi
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-08-31 16:41 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-28 2:08 [PATCH] checkpatch: Add test for positional misuse of section specifiers like __initdata Joe Perches
2013-08-31 14:31 ` Andi Kleen
2013-08-31 16:04 ` Joe Perches
2013-08-31 16:41 ` Andi Kleen
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).