Re: [PATCH 8/8] cfg80211: remove unnecessary include clauses

[Johannes Berg <johannes@sipsolutions.net>]

On Wed, 2014-04-09 at 14:36 +0200, Johannes Berg wrote:

> At the current point in time. If some of the headers that you rely on
> including something no longer does in the future because it no longer
> needs that, then you just broke everything.

Take the first of those patches for example. You say

<linux/bug.h> and <linux/net.h> is included by <linux/skbuff.h>.

However, this is a side effect of some implementation detail in
skbuff.h. If, in the future, some function in skbuff.h is no longer
inlined, then skbuff.h will no longer have to include bug.h and can
remove it. That change would break the build due to your patches.

The way you should think about this isn't the mechanic include chain,
it's the API that each file defines. skbuff.h includes bug.h due to an
implementation detail, but it doesn't intentionally re-export all the
bug.h API, that's not the purpose of skbuff.h. The purpose of skbuff.h
is to capture the SKB related APIs and structures, so that's the only
thing you should rely on getting from it.

Similarly, you should include bug.h if you need the APIs from that,
rather than relying on it being included more or less accidentally
through something else.

Obviously, the lack of namespaces etc. in the compiler makes it
impossible to actually enforce this, and as a result we often *miss*
includes that should be there, which sometimes gets found later when
things change or on different platforms if the recursive include was
platform specific, but we shouldn't exacerbate this problem by actively
removing correct includes.

Now, of course, if you find includes that aren't actually *needed* (e.g.
bug.h in a file that doesn't use BUG() or WARN() variants) then those
should be removed.

johannes