Extern variables in *.c files
I grepped kernel *.c (not *.h!) files for extern variable definitions. Much to my surprize, I found ~1500 such defs.
Isn't that bad C code style? What will happen if/when type of variable gets changed? (int->long).
I thought external definitions ought to be in *.h files. Maybe I'm missing something here...
--
vda
Quoted 5 lines · vda
I grepped kernel *.c (not *.h!) files for extern variable definitions. Much to my surprize, I found ~1500 such defs.
Isn't that bad C code style? What will happen if/when type of variable gets changed? (
int->long).
Yes; Int->long won't change anything on 32-bit machines and will break silently on 64-bit ones. The trick is finding appropriate places to put such definitions so that all the things that need them can include them without circular dependencies.
Signature · 1 line
"Love the dolphins," she advised him. "Write by W.A.S.T.E.."
Quoted 6 lines · Oliver Xymoron
Quoted 3 lines · vda
I grepped kernel *.c (not *.h!) files for extern variable definitions. Much to my surprize, I found ~1500 such defs.
Quoted 8 lines
Quoted 2 lines
Isn't that bad C code style? What will happen if/when type of variable gets changed? (
int->long).Yes;
Int->longwon't change anything on 32-bit machines and will break silently on 64-bit ones. The trick is finding appropriate places to put such definitions so that all the things that need them can include them without circular dependencies.
Isn't there some way to get the linker to detect the differing sizes?
-
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Andrew" == Andrew Morton <[email protected]> writes:
Quoted 6 lines · Andrew> Oliver Xymoron wrote:
Quoted 6 lines · vda
Quoted 4 lines
I grepped kernel *.c (not *.h!) files for extern variable definitions. Much to my surprize, I found ~1500 such defs.
Quoted 8 lines
Quoted 8 lines
Quoted 2 lines
Isn't that bad C code style? What will happen if/when type of variable gets changed? (
int->long).Yes;
Int->longwon't change anything on 32-bit machines and will break silently on 64-bit ones. The trick is finding appropriate places to put such definitions so that all the things that need them can include them without circular dependencies.
Andrew> Isn't there some way to get the linker to detect the differing Andrew> sizes?
`--warn-common'
Warn when a common symbol is combined with another common symbol or with a symbol definition. Unix linkers allow this somewhat sloppy practice, but linkers on some other operating systems do not. This option allows you to find potential problems from combining global symbols. Unfortunately, some C libraries use this practice, so you may get some warnings about symbols in the libraries as well as in your programs.
Regards,
-velco
Quoted 19 lines · Momchil Velikov
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
"Andrew" == Andrew Morton <[email protected]> writes:
Quoted 6 lines · Andrew> Oliver Xymoron wrote:
Quoted 6 lines · vda
Quoted 4 lines
I grepped kernel *.c (not *.h!) files for extern variable definitions. Much to my surprize, I found ~1500 such defs.
Quoted 8 lines
Quoted 8 lines
Quoted 2 lines
Isn't that bad C code style? What will happen if/when type of variable gets changed? (
int->long).Yes;
Int->longwon't change anything on 32-bit machines and will break silently on 64-bit ones. The trick is finding appropriate places to put such definitions so that all the things that need them can include them without circular dependencies.
Quoted 11 lines
Andrew> Isn't there some way to get the linker to detect the differing Andrew> sizes?
`--warn-common'
Warn when a common symbol is combined with another common symbol or with a symbol definition. Unix linkers allow this somewhat sloppy practice, but linkers on some other operating systems do not. This option allows you to find potential problems from combining global symbols. Unfortunately, some C libraries use this practice, so you may get some warnings about symbols in the libraries as well as in your programs.
Alas, it doesn't quite work as we'd like:
akpm-1:/home/akpm> cat a.c
int a;
main()
{}
akpm-1:/home/akpm> cat b.c
extern char a;
int b()
{return a;}
akpm-1:/home/akpm> gcc -fno-common -Wl,--warn-common a.c b.c akpm-1:/home/akpm>
No warnings emitted. If b.c doesn't use `extern' it will fail to link because of -fno-common.
What we'd *like* to happen is for the linker to determine that the extern char' and the int' declarations are a mismatch. Maybe the object file doesn't have the size info for `extern'
variables. The compiler emits it though.
We can actually get what we want by disabling -fno-common' and enabling --warn-common', but that's rather awkward.
Perhaps HJ can suggest a solution?
-
Quoted 16 lines · Andrew Morton
Alas, it doesn't quite work as we'd like:
akpm-1:/home/akpm> cat
a.c
int a;
main()
{}
akpm-1:/home/akpm> catb.c
extern char a;int
b()
{return a;}
akpm-1:/home/akpm> gcc -fno-common -Wl,--warn-commona.cb.cakpm-1:/home/akpm>
Quoted 8 lines
No warnings emitted. If
b.cdoesn't use `extern' it will fail to link because of -fno-common.What we'd *like* to happen is for the linker to determine that the
extern char' and theint' declarations are a mismatch. Maybe the object file doesn't have the size info for `extern'
variables. The compiler emits it though.
Compile doesn't emit the size info for
extern char a;
One way to fix it is to remove
extern char a;
and put
extern int a;
in a header file which is included by everyone.
H.J.
Quoted 5 lines · H . J . Lu
...
Compile doesn't emit the size info forextern char a;
You're right. I goofed.
Quoted 10 lines
One way to fix it is to remove
extern char a;
and put
extern int a;
in a header file which is included by everyone.
Yup. Problem is, we have about 1500 instances in the kernel :(
(Wouldn't it be nice if int a;' generated a compiler error if a declaration extern int a;' was not in scope?)
Oh well. Seems that disabling -fno-common and enabling --warn-common is the only way to autodetect bugs such as this.
-
Quoted 1 line · Andrew Morton
Yup. Problem is, we have about 1500 instances in the kernel :(
You can ignore the ~250 entries in *syms.c files. EXPORT_SYMBOL only needs to know if a symbol is a function or anything else, it does not care about types at all. You can define variables and functions with invalid types in *syms.c without doing any damage.
Quoted 2 lines · Andrew Morton
Oh well. Seems that disabling -fno-common and enabling --warn-common is the only way to autodetect bugs such as this.
You open another can of worms with variables in drivers that should be static that aren't - you end up with no way to detect these without -fno-common.
So, you have a choice:
1. Enable -fno-common
- detect variables that should be marked static which aren't
- don't detect size differences
2. Disable -fno-common
- don't detect variables that should be marked static
- detect size differences as long as the variables aren't marked extern
As soon as someone has int foo in one file, and extern char foo in another, you've lost no matter which option you take.
The header file approach is the most reliable (and imho correct) method to solve this problem.
Signature · 2 lines
Russell King ([email protected]) The developer of ARM Linux http://www.arm.linux.org.uk/personal/aboutme.html
Re: Extern variables in *.c files (maintainers pls read this)
In reply to vdaQuoted 8 lines
So, you have a choice:
1. Enable -fno-common
- detect variables that should be marked static which aren't
- don't detect size differences
2. Disable -fno-common
- don't detect variables that should be marked static
- detect size differences as long as the variables aren't marked extern
Quoted 5 lines
As soon as someone has int foo in one file, and extern char foo in another, you've lost no matter which option you take.
The header file approach is the most reliable (and imho correct) method to solve this problem.
And this method is traditional for C. We have struct declarations and fn propotypes in *.h, we should place extern vars there too. Always.
If you are a kernel subsystem or driver maintainer, you may wish to check whether *your* part of kernel has any extern variable defs. Just run this hunter script in top dir of kernel source:
-----------------------
#!/bin/sh
function do_grep() {
pattern="$1"
dir="$2"
shift;shift
for i in $dir/$*; do
if ! test -d "$i"; then
if test -e "$i"; then
grep -E "$pattern" "$i" /dev/null
fi
fi
done
for i in $dir/*; do
if test -d "$i"; then
do_grep "$pattern" "$i" $*
fi
done
}do_grep 'extern [^()]*;' . "*.c" 2>&1 | tee ../extern.log
---------------------------------
Output is not attached here, it's too big: ~100 KB, ~1500 lines for 2.5.1-pre8 --
vda
Re: Extern variables in *.c files (maintainers pls read this)
In reply to vdaQuoted 2 lines · "[email protected]" <[email protected]>…
And this method is traditional for C. We have struct declarations and fn propotypes in *.h, we should place extern vars there too. Always.
Agreed.
Quoted 25 lines
If you are a kernel subsystem or driver maintainer, you may wish to check whether *your* part of kernel has any extern variable defs. Just run this hunter script in top dir of kernel source:
-----------------------
#!/bin/shfunction do_grep() { pattern="$1" dir="$2" shift;shift for i in $dir/$*; do if ! test -d "$i"; then if test -e "$i"; then grep -E "$pattern" "$i" /dev/null fi fi done for i in $dir/*; do if test -d "$i"; then do_grep "$pattern" "$i" $* fi done }
Quoted 2 lines
do_grep'extern [^()]*;' . "*.c" 2>&1 | tee ../extern.log
---------------------------------
FWIW, I suppose you meant:
$ find . -name '*.c' | xargs grep -E 'extern [^()]*;' 2>&1 | tee extern.log
Regards, Olaf.