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, 5 Dec 2023 23:53:38 +0200
From: Andy Shevchenko <andy@...nel.org>
To: Nick Desaulniers <ndesaulniers@...gle.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>, tanzirh@...gle.com,
	Kees Cook <keescook@...omium.org>, linux-hardening@...r.kernel.org,
	linux-kernel@...r.kernel.org, Nick DeSaulniers <nnn@...gle.com>,
	llvm@...ts.linux.dev
Subject: Re: [PATCH] lib/string: shrink lib/string.i via IWYU

On Tue, Dec 05, 2023 at 01:39:47PM -0800, Nick Desaulniers wrote:
> On Tue, Dec 5, 2023 at 1:24 PM Andrew Morton <akpm@...ux-foundation.org> wrote:
> > On Tue, 5 Dec 2023 13:14:16 -0800 Nick Desaulniers <ndesaulniers@...gle.com> wrote:
> > > > The preferred way to import bit-fiddling stuff is to include
> > > > <linux/bits.h>.  Under the hood this may include asm/bitsperlong.h.  Or
> > > > it may not, depending on Kconfig settings (particularly architecture).
> > >
> > > Just triple checking my understanding; it looks like
> > > include/linux/bits.h unconditionally includes asm/bitsperlong.h (which
> > > is implemented per arch) most of which seem to include
> > > asm-generic/bitsperlong.h.
> > >
> > > include/linux/bits.h also defines a few macros (BIT_MASK, BIT_WORD,
> > > BITS_PER_BYTE, GENMASK, etc).  If lib/string.c is not using any of
> > > those, why can't we go straight to #including asm/bitsperlong.h?  That
> > > should resolve to the arch specific impl which may include
> > > asm-generic/bitsperlong.h?
> >
> > It's just a general rule.  If the higher-level include is present, use
> > that.  Because of the above, plus I guess things might change in the
> > future.
> 
> Hmm...how does one know that linux/bits.h is the higher-level include
> of asm/bitsperlong.h?
> 
> Do we mention these conventions anywhere under Documentation?

Unfortunately it comes with an experience. The problem here is the absence of
Documentation for the headers that guarantee inclusion of other headers. In
general linux/* are preferred over asm/* but sometimes things are complicated
(when some asm ones include linux ones via architectural code and circling
around).

> > We've been getting better about irregular asm/include files.
> >
> > But bits.h is a poor example.  A better case to study is spinlock.h.
> > If this tool recommended including asm/spinlock.h then that won't work
> > on any architecture which doesn't implement SMP (there is no
> > arch/nios2/include/asm/spinlock.h).
> 
> The tooling Tanzir is working on does wrap IWYU, and does support such
> mapping (of 'low level' to 'high level' headers; more so, if it
> recommends X you can override to suggest Y instead).
> 
> arch/nios/ also doesn't provide a bug.h, which this patch is
> suggesting we include directly.  I guess the same goes for
> asm/rwonce.h.

Have you checked Ingo Molnar's gigantic series (2k+ patches) for the header
hell clean up? Perhaps we need to apply that first.

-- 
With Best Regards,
Andy Shevchenko



Powered by blists - more mailing lists