From owner-freebsd-arch@FreeBSD.ORG Wed Aug 10 06:31:31 2005 Return-Path: X-Original-To: freebsd-arch@FreeBSD.org Delivered-To: freebsd-arch@FreeBSD.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id E347A16A427 for ; Wed, 10 Aug 2005 06:31:31 +0000 (GMT) (envelope-from bde@zeta.org.au) Received: from mailout2.pacific.net.au (mailout2.pacific.net.au [61.8.0.115]) by mx1.FreeBSD.org (Postfix) with ESMTP id 5414F43D45 for ; Wed, 10 Aug 2005 06:31:31 +0000 (GMT) (envelope-from bde@zeta.org.au) Received: from mailproxy2.pacific.net.au (mailproxy2.pacific.net.au [61.8.0.87]) by mailout2.pacific.net.au (8.13.4/8.13.4/Debian-3) with ESMTP id j7A6VOln030791; Wed, 10 Aug 2005 16:31:24 +1000 Received: from epsplex.bde.org (katana.zip.com.au [61.8.7.246]) by mailproxy2.pacific.net.au (8.13.4/8.13.4/Debian-3) with ESMTP id j7A6VM0Y024953; Wed, 10 Aug 2005 16:31:23 +1000 Date: Wed, 10 Aug 2005 16:31:22 +1000 (EST) From: Bruce Evans X-X-Sender: bde@epsplex.bde.org To: Craig Rodrigues In-Reply-To: <20050810005323.GA42721@crodrigues.org> Message-ID: <20050810151806.I5966@epsplex.bde.org> References: <20050810005323.GA42721@crodrigues.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: freebsd-arch@FreeBSD.org Subject: Re: [RFC] -Wredundant-decls: keep it or remove it? X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 10 Aug 2005 06:31:32 -0000 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