[PATCH] checkpatch: warn about incorrect __initdata placement

[PATCH] checkpatch: warn about incorrect __initdata placement

[Bartlomiej Zolnierkiewicz]

__initdata tag should be placed between the variable name and equal
sign for the variable to be placed in the intended .init.data section.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 scripts/checkpatch.pl | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 23d55bf..88d07a6 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -4275,6 +4275,12 @@ sub process {
 			WARN("EXPORTED_WORLD_WRITABLE",
 			     "Exporting world writable files is usually an error. Consider more restrictive permissions.\n" . $herecurr);
 		}
+
+# check for incorrect __initdata placement
+		if ($line =~ /\bstruct\s+__initdata.*\=/) {
+			WARN("INITDATA_PLACEMENT",
+			     "__initdata tag should be placed between the variable name and equal sign\n" . $herecurr);
+		}
 	}
 
 	# If we have no input at all, then there is nothing to report on
-- 
1.8.2.3

Re: [PATCH] checkpatch: warn about incorrect __initdata placement

[Joe Perches]

On Mon, 2013-09-30 at 15:23 +0200, Bartlomiej Zolnierkiewicz wrote:
> __initdata tag should be placed between the variable name and equal
> sign for the variable to be placed in the intended .init.data section.
[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -4275,6 +4275,12 @@ sub process {
[]
> +# check for incorrect __initdata placement
> +		if ($line =~ /\bstruct\s+__initdata.*\=/) {
> +			WARN("INITDATA_PLACEMENT",
> +			     "__initdata tag should be placed between the variable name and equal sign\n" . $herecurr);
> +		}

Hello Bartlomiej

I believe that -next commit 12de1c1ad0df
("checkpatch: add rules to check init attribute and const defects")

which adds $InitAttribute already does this test.

Anyway, this should be:
	if ($line =~ /\b(struct|union)\s+$InitAttribute.*=/) {

and please add this test adjacent to the other $InitAttribute
tests only if it's not already covered by the first bit added
by that commit below (again, I believe it is):

------------------------

# 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*[=;]/) {

Re: [PATCH] checkpatch: warn about incorrect __initdata placement

[Bartlomiej Zolnierkiewicz]

Hi,

On Monday, September 30, 2013 08:50:23 AM Joe Perches wrote:
> On Mon, 2013-09-30 at 15:23 +0200, Bartlomiej Zolnierkiewicz wrote:
> > __initdata tag should be placed between the variable name and equal
> > sign for the variable to be placed in the intended .init.data section.
> []
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
> > @@ -4275,6 +4275,12 @@ sub process {
> []
> > +# check for incorrect __initdata placement
> > +		if ($line =~ /\bstruct\s+__initdata.*\=/) {
> > +			WARN("INITDATA_PLACEMENT",
> > +			     "__initdata tag should be placed between the variable name and equal sign\n" . $herecurr);
> > +		}
> 
> Hello Bartlomiej
> 
> I believe that -next commit 12de1c1ad0df
> ("checkpatch: add rules to check init attribute and const defects")
> 
> which adds $InitAttribute already does this test.
> 
> Anyway, this should be:
> 	if ($line =~ /\b(struct|union)\s+$InitAttribute.*=/) {
> 
> and please add this test adjacent to the other $InitAttribute
> tests only if it's not already covered by the first bit added
> by that commit below (again, I believe it is):
> 
> ------------------------
> 
> # 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*[=;]/) {

It seems that a bit earlier patch which was merged for v3.12-rc1 (commit
8716de3 "checkpatch: add test for positional misuse of section specifiers
like __initdata" from September 11) already covers detection of the wrong
placement of __initdata (it was even inspired by the same EXYNOS4 code
issues as mine patch). I originally did my patch on August 30 so commit
8716de3 wasn't there yet, now it is all covered up nicely in the upstream.
Thanks for the work on this and sorry for the noise.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics