linux-kernel - Re: [PATCH v3 4/8] pwm: lpss: Include headers we are direct user of

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:   Tue, 27 Sep 2022 17:55:21 +0200
From:   Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     Hans de Goede <hdegoede@...hat.com>, linux-pwm@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Thierry Reding <thierry.reding@...il.com>
Subject: Re: [PATCH v3 4/8] pwm: lpss: Include headers we are direct user of

On Tue, Sep 27, 2022 at 06:26:28PM +0300, Andy Shevchenko wrote:
> On Tue, Sep 27, 2022 at 05:10:53PM +0200, Uwe Kleine-König wrote:
> > On Tue, Sep 27, 2022 at 05:47:19PM +0300, Andy Shevchenko wrote:
> > > For the sake of integrity, include headers we are direct user of.
> > > 
> > > While at it, add missed struct pwm_lpss_boardinfo one and replace
> > > device.h with a forward declaration. The latter improves compile
> > > time due to reducing overhead of device.h parsing with entire train
> > > of dependencies.
> > 
> > Hm, I copied the cmdline for the compiler from a V=1 build and only run
> > the compiler on drivers/pwm/pwm-lpss-pci.c.
> > 
> > With #include <device.h> I got:
> > 
> > 	real	0m0.421s
> > 	user	0m0.354s
> > 	sys	0m0.066s
> > 
> > With struct device; I got:
> > 
> > 	real	0m0.431s
> > 	user	0m0.378s
> > 	sys	0m0.052s
> > 
> > Are the numbers for you considerably different?
> 
> Why Ingo created thousands of patches to do something similar? Because for
> a single user you won't see a big difference, but when amount of small pieces
> are gathered together, you definitely will.

My doubt is that for me the effect of using struct device over #include
<device.h> is even negative (looking at real and user). Is it sys which
counts in the end?

> > > +struct device;
> 
> ...
> 
> > > +struct pwm_lpss_boardinfo;
> > 
> > Hmm, I wonder why there is no compiler warning without that declaration.
> > At least in my builds. Do you see a warning? IMHO it's better to fix
> > that be swapping the order of struct pwm_lpss_chip and struct
> > pwm_lpss_boardinfo.
> 
> Have I told about warning?

No, it's just me who expected there would be a warning if a pointer to
struct pwm_lpss_boardinfo is used before struct pwm_lpss_boardinfo is
defined (or declared).

Anyhow, I stand by my opinion that swapping the order of struct
pwm_lpss_chip and struct pwm_lpss_boardinfo is a saner fix.

> It's a proper C programming style.
> You don't have a warning because all pointers are considered to be the same,
> but it is better style to explicitly point that out.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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