linux-kernel - Re: [PATCH v1 05/11] cpufreq: tegra20: Clean up included headers

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]

Message-ID: <20180518085714.GE14500@ulmo>
Date:   Fri, 18 May 2018 10:57:14 +0200
From:   Thierry Reding <thierry.reding@...il.com>
To:     Dmitry Osipenko <digetx@...il.com>
Cc:     Viresh Kumar <viresh.kumar@...aro.org>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Jonathan Hunter <jonathanh@...dia.com>,
        linux-tegra@...r.kernel.org, linux-pm@...r.kernel.org,
        linux-kernel@...r.kernel.org, Peter Geis <pgwipeout@...il.com>
Subject: Re: [PATCH v1 05/11] cpufreq: tegra20: Clean up included headers

On Fri, May 18, 2018 at 11:05:58AM +0300, Dmitry Osipenko wrote:
> On 18.05.2018 04:58, Viresh Kumar wrote:
> > On 17-05-18, 21:00, Dmitry Osipenko wrote:
> >> Remove unused/unneeded headers and sort them in the alphabet order.
> >>
> >> Signed-off-by: Dmitry Osipenko <digetx@...il.com>
> >> ---
> >>  drivers/cpufreq/tegra20-cpufreq.c | 11 ++---------
> >>  1 file changed, 2 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/cpufreq/tegra20-cpufreq.c b/drivers/cpufreq/tegra20-cpufreq.c
> >> index 69f033d297e1..61f00d1cba26 100644
> >> --- a/drivers/cpufreq/tegra20-cpufreq.c
> >> +++ b/drivers/cpufreq/tegra20-cpufreq.c
> >> @@ -16,16 +16,9 @@
> >>   *
> >>   */
> >>  
> >> -#include <linux/kernel.h>
> >> -#include <linux/module.h>
> >> -#include <linux/types.h>
> >> -#include <linux/sched.h>
> >> -#include <linux/cpufreq.h>
> >> -#include <linux/delay.h>
> >> -#include <linux/init.h>
> >> -#include <linux/err.h>
> >>  #include <linux/clk.h>
> >> -#include <linux/io.h>
> >> +#include <linux/cpufreq.h>
> >> +#include <linux/module.h>
> > 
> > Ideally you should keep all the headers whose declarations your code
> > is using directly. It may happen that removing above still compiles
> > because cpufreq.h has included the headers indirectly for you. But
> > that will break the day cpufreq.h doesn't need those headers anymore.
> > 
> > So just make sure you aren't using any of them in your code. For
> > example you are using bool in your code and so you shouldn't remove
> > types.h ? Same for init.h as you are using __init and __exit.
> 
> The preference on includes seems to vary among maintainers. I've seen other
> opinion that encouraged to minimize included headers and only add the headers
> when compilation breaks.

I think this probably depends on the type of include. For headers in
include/linux the reasoning is that they can change often as part of
some rework and this has in the past caused unrelated files to break
builds because suddenly a file no longer has all required definitions
available.

It's slightly different for header files that have a narrower exposure
because the chances of breaking things randomly are much lower.

> I'll revisit this patch and keep init.h and others in v2 since you prefer that
> way. Thank you for the review.

With those changes, this is:

Acked-by: Thierry Reding <treding@...dia.com>

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)