On Wed, 2014-04-09 at 14:36:59 +0200, Johannes Berg wrote: > On Wed, 2014-04-09 at 20:25 +0800, Zhao, Gang wrote: >> On Wed, 2014-04-09 at 09:49:30 +0200, Johannes Berg wrote: >> > I don't like those last four patches, I'd rather have more includes than >> > rely on other headers including headers that we need - that could change >> > after all. >> >> As I said in commit log, duplicate including could hide some warnings. > > Well, the commit logs for these patches didn't have any such > information, but that's not really the point. Besides, what does "could > hide some warnings" even mean? This isn't a meaningful thing, if you > include the right headers then you should get what you need, and you > should include the headers for what you need. I said it in those four "fix" patches, not here. I thought it's consensus to exclude directly included headers if it's indirectly included in other headers, but apparently I'm wrong. What I mean the duplicate including supressed the warnings is: #include <a.h> #include <b.h> #include <c.h> <c.h> includes <a.h>, so I thought <a.h> is "duplicate". BTW now I'm not sure my defination of duplicate is equal to `make includecheck` command. If <b.h> needs to includes <a.h> but forgot to do it, it will not generate warning, since <a.h> is above, and do the work. If we remove <a.h>, the warning will show up, and we can fix it. That's what I think is the benifit of removing duplicate include. > >> In theory, the possibility of change is equal, either it's a directly >> included header or a indirectly included header, and it's more likely to >> change to include more, not less. So the change may not cause any >> problem. > > 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. That's the point you > seemingly didn't consider, and I think it's much better to include what > you need rather than rely on somebody else doing it for you. The latter > can even be architecture-specific. I see your point. The benifit of removing duplicate including is above, the drawback is that we may more depend on other parts of the code than we should be. > >> In other way, the local directory header files may change more >> frequently than the header files in include/ directory. Whether it's a >> total win to apply the last four patches may be a question, but it's >> just amusing to see that lots of lines can be deleted. :-) > > # of lines isn't a relevant metric though. If you were removing includes > that we didn't need that's certainly a useful thing, but removing > includes because they're indirectly already included is IMHO practically > always bad. > > johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
