Date: Tue, 21 Feb 2012 04:57:13 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Aleksandr Rybalko <ray@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r231939 - head/sys/net80211 Message-ID: <20120221041356.A1826@besplex.bde.org> In-Reply-To: <201202201505.q1KF5MNP056276@svn.freebsd.org> References: <201202201505.q1KF5MNP056276@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 20 Feb 2012, Aleksandr Rybalko wrote: > Log: > Remove redundant forward declaration of struct ieee80211com. Thanks, but it now has 1 redundant forward declarations of this instead of 2. There is another section with 3 forward declarations which are all redundant and bogus. Since programmers apparently don't understand C's scope rules for declarations of incomplete structs (these declarations declare just the struct tag), it would be useful for the compiler to warn about bogus ones. The remaining 3 bogus ones are before a struct that uses pointers to the incomplete structs for its struct members. C's scope rules work right in this case, and so any pointer declaration self-declares the same incomplete struct that a previous forward declaration would (thus any previous purely-forward declaration is bogus, and `any' applies to all 3 here), and also forward-declares the incomplete struct for the rest of the file (so any later forward declaration is bogus). > Modified: head/sys/net80211/ieee80211_node.h > ============================================================================== > --- head/sys/net80211/ieee80211_node.h Mon Feb 20 13:59:24 2012 (r231938) > +++ head/sys/net80211/ieee80211_node.h Mon Feb 20 15:05:21 2012 (r231939) > @@ -299,8 +299,6 @@ ieee80211_unref_node(struct ieee80211_no > *ni = NULL; /* guard against use */ > } > > -struct ieee80211com; > - > void ieee80211_node_attach(struct ieee80211com *); > void ieee80211_node_lateattach(struct ieee80211com *); > void ieee80211_node_detach(struct ieee80211com *); OTOH, the scope rules for prototypes (and function definitions) don't work right. Any declaration of an incomplete struct in the function's parameters is purely local to the extent that it doesn't affect the rest of the file, and to go with this the declaration must be of a struct that is incapable being passed, unless the struct has already been declared, perhaps incompletely. So some prototypes need an explicit forward declaration of the struct which is sometimes placed just before the prototype or at the beginning of a block of prototypes. But in many cases, such a declaration is bogus because the struct has already been declared (incompletely and self-referentially) as part of the declaration of a pointer struct member. Normal order puts (complete) struct declarations before prototypes and thus gives the declaration more often than other orders would, and it is good to depend on it since the need for forward declarations before prototypes is just a bug in the scope rules for prototypes so we shouldn't have to write extra code to work around it. Compilers can be relied to warn about cases where the explicit forward declarations are necessary, except in cases of header pollution where most but not all of the necessary forward declarations are provided accidentally by pollution in other headers (you start getting errors if the pollution is fixed or moved, or if the application doesn't happen to include enough headers to supply as much pollution as it used to). I usually find minimal sets of forard declarations by first fixing all pollution and then compiling the header without any other includes before it; then remove all forward declarations and add back the ones that cause compilation failures. Without fixing the pollution, you can't rely on compilation failures to find all the necessary ones. Pollution and non-pollution in other headers make it difficult for a -Wbogus-declarations option to be made usable. The -Wredundant-decls options option is only for variables and prototypes, and is buggy for non-redundant declarations where the declaration is built up, with types and other qualities being completed at each step. Considerable ifdefing was needed to make this usable in the FreeBSD kernel, and it is only enabled by WARNS >= 6 in userland. Similar ifdefing for struct tags might be even more painful. Now it is clearer that some declarations are built up -- a full struct definition is not redundant when it completes an incomplete struct declaration. The self-forward declaration of "struct foo" in "struct foo *bar" is never redundant, and it makes any previous forward declaration of "struct foo" redundant, but the compiler can only usefully warn about this if the previous one is guaranteed to be in scope (thus if it is in another header, it may or not be bogus, depending on whether that header always does it and is always included, and this is so in all other headers). Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120221041356.A1826>