Date: Wed, 10 Aug 2005 16:31:22 +1000 (EST) From: Bruce Evans <bde@zeta.org.au> To: Craig Rodrigues <rodrigc@crodrigues.org> Cc: freebsd-arch@FreeBSD.org Subject: Re: [RFC] -Wredundant-decls: keep it or remove it? Message-ID: <20050810151806.I5966@epsplex.bde.org> In-Reply-To: <20050810005323.GA42721@crodrigues.org> References: <20050810005323.GA42721@crodrigues.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 9 Aug 2005, Craig Rodrigues wrote:
> In recent days, while trying to get the kernel in shape to
> compile with GCC 4.0, I encountered some examples such as the
> following in net/rtsock.c:
>
> extern struct domain routedomain; /* or at least forward */
>
> static struct protosw routesw[] = {
> { SOCK_RAW, &routedomain, 0, PR_ATOMIC|PR_ADDR,
> 0, route_output, raw_ctlinput, 0,
> 0,
> raw_init, 0, 0, 0,
> &route_usrreqs
> }
> };
>
> static struct domain routedomain =
> { PF_ROUTE, "route", 0, 0, 0,
> routesw, &routesw[sizeof(routesw)/sizeof(routesw[0])] };
>
> It is illegal in ISO C to declare a struct as extern (implying external linkage)
> , and then declare it as static (implying internal linkage).
gcc[1-4] has warned about this for at least 15 years, but only with
-pedantic. It has always been fatal in C compilers (e.g., TenDRA).
The extern declaration is apparently a hack to support K&R C where it wasn't
clear that static declarations could be repeated.
> I have two options to fix this.
>
> OPTION 1:
>
> Change routedomain to not be static:
>
> extern struct domain routedomain;
>
> ....
> struct domain routedomain = { ...... }
Not good. It should be static since it is only used in 1 object file.
> OPTION 2:
>
> Forward declare routedomain as static, but remove -Wredundant-decls
> from kernel makefiles:
>
> static struct domain routedomain;
>
> ....
> static struct domain routedomain = { ..... }
Not good. -Wredundant-decls was useful for finding and removing private
declarations of public interfaces. Now it is useful for preventing them
coming back.
> For OPTION 2, it is necessary to remove -Wredundant-decls
> because you will get a new compiler warning:
>
> warning: redundant redeclaration of 'routedomain'
> warnig: previous declaration was here ...
It is necessary to fix -Wredundant-decls so that it doesn't warn about
non-redundant declarations.
In ISO C, it is possible to build up a type by successive incomplete
declarations (see section 6.2.7). Then the later declarations needed
to complete the type are manifestly non-redundant, but "gcc -Wredundant"
decls complains about all the later ones but not about the first one
which might be redundant. Most uses of this feature are just obfuscations,
but ones like the above where the non-reduundant info is an initializer
are useful.
Example of a useless use of this feature (adapted from the example in
section 6.2.7):
%%%
int foo1();
int foo1(int foo2());
int foo1(int foo2(int foo3()));
int foo1(int foo2(int foo3(int foo4())));
%%%
This example can be extended to take any number of non-redundant
declarations to complete the declaration of foo1().
gnu C provides many more ways to do silly things like the above.
C's type rules prevent adding type qualifiers in separate declarations,
but gnu C attributes can be added separately. The behaviour for this
is buggy, and fixing it might require keeping track of which declarations
are actually redundant. Example:
%%%
static int x __attribute__((__aligned__(4)));
static int x __attribute__((__aligned__(16)));
static int x = 1;
%%%
Here the second attribute statement works reasonably by silently
overriding the first attribute; however, if the initializer is moved
before the second attribute statment, then it has no effect on the
alignment. gcc apparently emits the code for the initializer before
seeing all the declarations. I thought that C's support for composite
types don't allow it to do this. Perhaps the type+linkage rules do
actually allow it but the attribute rules should allow it but currently
don't. The type+linkage rules have very confusing parts for static
linkage which may be mainly to support 1-pass compilation.
> To fix this problem, is it better to go with OPTION 1
> or OPTION 2? I am a bit hesitant to remove -Wredundant-decls
> from the kernel Makefiles, because it has been there for a long time.
> Are there cases where the warnings are useful?
See above.
> It seems to warn against legitimate C code in
> the GCC documentation:
>
> `-Wredundant-decls'
> Warn if anything is declared more than once in the same scope,
> even in cases where multiple declaration is valid and changes
> nothing.
Many gcc warnings are for legitimate C code that is considered bad
practice. All should be (ones about illegitimate C code should be
errors).
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20050810151806.I5966>