Date: Tue, 21 Feb 2012 01:51:46 +0200 From: Aleksandr Rybalko <ray@freebsd.org> To: Bruce Evans <brde@optusnet.com.au> 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: <20120221015146.48c3e882.ray@freebsd.org> In-Reply-To: <20120221041356.A1826@besplex.bde.org> References: <201202201505.q1KF5MNP056276@svn.freebsd.org> <20120221041356.A1826@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 21 Feb 2012 04:57:13 +1100 (EST) Bruce Evans <brde@optusnet.com.au> wrote: > 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. Seems me too :) > > 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). So I will try to remove another when complete with understanding of that paragraph of ‘K&R C’ :) > > > 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 Thanks for good comments Bruce! WBW -- Aleksandr Rybalko <ray@freebsd.org>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120221015146.48c3e882.ray>