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

["Zhao\, Gang" <gamerh2o@gmail.com>]

On Wed, 2014-04-09 at 14:43:20 +0200, Johannes Berg wrote:
> 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.

I just want to reduce the possibility that duplicate including could
hide some warnings which I explained in anoother thread. The current way
I use may be too aggressive as you said. I will try to use your
suggestion to remove duplicate including, so the last four patches can
be dropped.

BTW, after I explained why I changed the parameter type in the first
patch, do you still think that is not appropriate ?

>
> johannes