lkml.org 
[lkml]   [2008]   [Mar]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: whose job is it to include various header files?
On Thu, Mar 13, 2008 at 09:53:01PM -0400, Robert P. J. Day wrote:
> On Fri, 14 Mar 2008, Jan Engelhardt wrote:
>
> >
> > On Mar 14 2008 00:53, Jesper Juhl wrote:
> > >I agree with you completely. A file should explicitly include
> > >headers for the stuff it uses and not rely on implicit includes
> > >done elsewhere. Cleaning that up is going to touch a lot of files
> > >though for no real short term gain (there is a long term gain of
> > >maintainability though), so it's going to be a loveless job :(
> >
> > But straightforward. You nuke the complete #include list of a .h/.c
> > file and "rebuild" it by hand, by looking at the code the .h/.c file
> > provides/uses and selecting appropriate #includes.

Which is generally wrong thing to do because you won't notice that
moving parts to another header will make list of necessary headers much
shorter.

> well, yes, that would certainly do it, but i'm not that ambitious.
> :-) actually, what i'm testing now is deleting the two superfluous
> includes from <linux/percpu.h>:
>
> #include <linus/slab.h>

This needed for kzalloc().

> #include <linux/string.h>

That looks unnecessary, yes.

> doing "make allyesconfig" on x86, watching where the compile fails,
> fixing that file, and noticing that errors fall into a fairly small
> set of localized clumps, so i'll just collect them and submit them.
> there's actually not that many.

Sir, why on earth do you breed so many patches?

If you're going to delete string.h from percpu.h (which is good idea)
you a) delete it, and b) fix all compile breakages, and c) submit as one
patch.

By "all compile" breakages I mean cross-compilation on archs other than
i386 and on many configs on each of them. If you can't do that, sorry,
stay far away from headers, especially core.

As for many preparatory patches, a) by itself they're useless -- A is
still including B and headers are protected against double inclusion,
b) late in game you can discover that, oops, B is needed in A and all
those patches were useless, c) submitting as one patch which removes B
from A means other people can compile-test that you haven't broken their
usual config, with many small patches scattered over mailing lists they
won't be able do that. Remember, A is still including B.

I suggest to add headers when there is compilation breakage and remove
headers when it's safe thing to do.

P.S.: Some common sense is also required when touching header
dependencies. gfp.h is not included left and right by drivers,
because it comes from slab.h. Some common sense is also required
when touching header dependencies.



\
 
 \ /
  Last update: 2008-03-14 06:51    [W:0.037 / U:0.556 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site