linux-kernel - Re: whose job is it to include various header files?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 14 Mar 2008 08:47:44 +0300
From:	Alexey Dobriyan <adobriyan@...il.com>
To:	"Robert P. J. Day" <rpjday@...shcourse.ca>
Cc:	Jan Engelhardt <jengelh@...putergmbh.de>,
	Jesper Juhl <jesper.juhl@...il.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: 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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/