Skip to contentSkip to conversation

kbuild: two trivial fixes for check headers

PATCH In review Amerigo Wang 4 participants 8 messages Jun 4, 2009 – Jun 5, 2009 Kbuild

lore
Share & tools
lore

Amerigo Wang

author
#0 lore raw

PATCH 1/2 kbuild: two trivial fixes for check headers

Remove a useless 'extern' in an exported header.
Fix a comment headers_check.pl.

Signed-off-by WANG Cong <[email protected]>
Cc Sam Ravnborg <[email protected]>

------

include/mtd/jffs2-user.h +1 -1
index fa94b0e..10dc7ef 100644
@@ -18,7 +18,7 @@
 #undef je32_to_cpu
 #undef jemode_to_cpu
 
-extern int target_endian;
+int target_endian;
 
 #define t16(x) ({ __u16 __b = (x); (target_endian==__BYTE_ORDER)?__b:bswap_16(__b); })
 #define t32(x) ({ __u32 __b = (x); (target_endian==__BYTE_ORDER)?__b:bswap_32(__b); })
scripts/headers_check.pl +1 -1
index 56f90a4..ccdbace 100644
@@ -2,7 +2,7 @@
 #
 # headers_check.pl execute a number of trivial consistency checks
 #
-# Usage: headers_check.pl dir [files...]
+# Usage: headers_check.pl dir arch [files...]
 # dir:   dir to look for included files
 # arch:  architecture
 # files: list of files to check
author
#1 lore raw

PATCH 2/2 kbuild: clean up scripts/headers.sh

'drop' variable is unused.

'ppc' and 'sparc64' directories don't exist in arch/, and I think their headers can be well exported, so just remove them.

Signed-off-by WANG Cong <[email protected]>
Cc Sam Ravnborg <[email protected]>

------

scripts/headers.sh +0 -7
index d33426f..0308ecc 100755
@@ -15,19 +15,12 @@ do_command()
 	fi
 }
 
-# Do not try this architecture
-drop="generic um ppc sparc64 cris"
-
 archs=$(ls ${srctree}/arch)
 
 for arch in ${archs}; do
 	case ${arch} in
 	um)        # no userspace export
 		;;
-	ppc)       # headers exported by powerpc
-		;;
-	sparc64)   # headers exported by sparc
-		;;
 	cris)      # headers export are known broken
 		;;
 	*)
#2 lore raw
In reply to Amerigo Wang
Quoted 3 lines · Amerigo Wang

Remove a useless 'extern' in an exported header.
Fix a comment headers_check.pl.

These two patches has nothing in commom.

Quoted 10 lines
Signed-off-by WANG Cong <[email protected]>
Cc Sam Ravnborg <[email protected]>

------

include/mtd/jffs2-user.h +0 -0
index fa94b0e..10dc7ef 100644

include/mtd/jffs2-user.h line 21

Quoted 5 lines
include/mtd/jffs2-user.h +1 -1
@@ -18,7 +18,7 @@
 #undef je32_to_cpu
 #undef jemode_to_cpu
 
-extern int target_endian;
+int target_endian;

You now declare a global variable named target_endian each time you include this file - which is wrong.

author
#3 lore raw
Quoted 8 lines · Sam Ravnborg
Quoted 3 lines · Amerigo Wang

Remove a useless 'extern' in an exported header.
Fix a comment headers_check.pl.

These two patches has nothing in commom.

They are both about kernel headers stuff.

Quoted 13 lines
Quoted 13 lines
Signed-off-by WANG Cong <[email protected]>
Cc Sam Ravnborg <[email protected]>

------

include/mtd/jffs2-user.h +0 -0
index fa94b0e..10dc7ef 100644
@@ -18,7 +18,7 @@
 #undef je32_to_cpu
 #undef jemode_to_cpu
Quoted 6 lines

include/mtd/jffs2-user.h line 22

Quoted 3 lines
include/mtd/jffs2-user.h +1 -1
-extern int target_endian;
+int target_endian;

You now declare a global variable named target_endian each time you include this file - which is wrong.

Why? 'make headers_check' complains about it.

#4 lore raw
Quoted 25 lines · Amerigo Wang
Quoted 8 lines · Sam Ravnborg
Quoted 3 lines · Amerigo Wang

Remove a useless 'extern' in an exported header.
Fix a comment headers_check.pl.

These two patches has nothing in commom.

They are both about kernel headers stuff.

Quoted 13 lines
Quoted 13 lines
Signed-off-by WANG Cong <[email protected]>
Cc Sam Ravnborg <[email protected]>

------

include/mtd/jffs2-user.h +0 -0
index fa94b0e..10dc7ef 100644
@@ -18,7 +18,7 @@
 #undef je32_to_cpu
 #undef jemode_to_cpu
Quoted 8 lines
Quoted 7 lines

include/mtd/jffs2-user.h line 22

Quoted 3 lines
include/mtd/jffs2-user.h +1 -1
-extern int target_endian;
+int target_endian;

You now declare a global variable named target_endian each time you include this file - which is wrong.

Why? 'make headers_check' complains about it.

Rightfully is does...
But is think it is a prototype of a function.

But think about it - it does not makes sense to have an extern declarationof a variable in an exported header. There is no way that userspace can access this variable from user-space and therefore the declaration of the variable needs to be moved to a kernel-only header file.

So if you touch this I would ask you not to consider a single line in jffs-user.h - but rather take a critiacal look at the whole file and fix any issues there is.
And then to send it to the mtd people (dwmw2).
I did not look at the file but just looking at yout patch the "undef jemode_to_cpu" looks fishy in an exported header.

Sam

author
#5 lore raw
Quoted 10 lines · Sam Ravnborg

Rightfully is does...
But is think it is a prototype of a function.

But think about it - it does not makes sense to have an extern declarationof a variable in an exported header. There is no way that userspace can access this variable from user-space and therefore the declaration of the variable needs to be moved to a kernel-only header file.

Agree, then the script looks wrong. I will fix it.

Quoted 7 lines

So if you touch this I would ask you not to consider a single line in jffs-user.h - but rather take a critiacal look at the whole file and fix any issues there is.
And then to send it to the mtd people (dwmw2).
I did not look at the file but just looking at yout patch the "undef jemode_to_cpu" looks fishy in an exported header.

Hmm, after googling a bit, I got this patch:

http://patchwork.ozlabs.org/patch/26224/

So I won't touch this.

Thanks.

#6 lore raw
In reply to Sam Ravnborg
Quoted 7 lines · Sam Ravnborg

So if you touch this I would ask you not to consider a single line in jffs-user.h - but rather take a critiacal look at the whole file and fix any issues there is.
And then to send it to the mtd people (dwmw2).
I did not look at the file but just looking at yout patch the "undef jemode_to_cpu" looks fishy in an exported header.

David already have patches to remove whole jffs-user.h file and AFAIK he already queued it. So no point of touching this file.

Thanks,
--
JSR

#7 lore raw
In reply to Sam Ravnborg
Quoted 20 lines · Sam Ravnborg
Quoted 3 lines · Amerigo Wang

Remove a useless 'extern' in an exported header.
Fix a comment headers_check.pl.

These two patches has nothing in commom.

Quoted 13 lines
Signed-off-by WANG Cong <[email protected]>
Cc Sam Ravnborg <[email protected]>

------

include/mtd/jffs2-user.h +0 -0
index fa94b0e..10dc7ef 100644
@@ -18,7 +18,7 @@
 #undef je32_to_cpu
 #undef jemode_to_cpu
Quoted 4 lines

include/mtd/jffs2-user.h line 21

Quoted 2 lines
include/mtd/jffs2-user.h +1 -1
-extern int target_endian;
+int target_endian;

You now declare a global variable named target_endian each time you include this file - which is wrong.

Hmmm, the script seems wrong, not the C code. :)
And the word 'prototype' is only for functions.

How about patch below?

Signed-off-by WANG Cong <[email protected]>

---

scripts/headers_check.pl +2 -2
index 56f90a4..11dc869 100644
@@ -2,7 +2,7 @@
 #
 # headers_check.pl execute a number of trivial consistency checks
 #
-# Usage: headers_check.pl dir [files...]
+# Usage: headers_check.pl dir arch [files...]
 # dir:   dir to look for included files
 # arch:  architecture
 # files: list of files to check

@@ -63,7 +63,7 @@ sub check_include
 
 sub check_prototypes
 {
-	if ($line =~ m/^\s*extern\b/) {
+	if ($line =~ m/^\s*extern\b.*\(.*\)/) {
 		printf STDERR "$filename:$lineno: extern's make no sense in userspace\n";
 	}
 }