mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@intel.com>
To: Nathan Chancellor <nathan@kernel.org>,
	Randy Dunlap <rdunlap@infradead.org>
Cc: linux-kernel@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Nicolas Schier <nsc@kernel.org>, Jason Gunthorpe <jgg@nvidia.com>,
	Masahiro Yamada <masahiroy@kernel.org>,
	linux-kbuild@vger.kernel.org
Subject: Re: [PATCH] kbuild: add header check facility as a manually run static analyzer
Date: Thu, 17 Sep 2026 11:14:03 +0300	[thread overview]
Message-ID: <67e8d510f7ea3e79035197c53ac215c23c8633ce@intel.com> (raw)
In-Reply-To: <20260916231319.GA550816@ax162>

On Wed, 16 Sep 2026, Nathan Chancellor <nathan@kernel.org> wrote:
> On Wed, Sep 16, 2026 at 03:24:09PM -0700, Randy Dunlap wrote:
>> On 9/15/26 3:43 AM, Jani Nikula wrote:
>> > There have been various attempts at adding a header test or check
>> > mechanism in the kernel build system. The header check primarily
>> > consists of ensuring headers are self-contained, have include guards,
>> > and, in some cases, pass kernel-doc.
>> > 
>> > The main problems have been:
>> > 
>> > - The dependency tracking creates undesirable artefacts (infamously also
>> >   known as disgusting turds) in the build directory.
>> > 
>> > - Gating the feature behind a kconfig option is complicated due to
>> >   allyesconfig builds. It's possible, but requires a verbose and
>> >   confusing negative proxy config option.
>> > 
>> > - Naming the dependency tracking files with a dot prefix or placing them
>> >   in a dot prefixed subdirectory in the build directory to hide them has
>> >   been exceedingly difficult to achieve. (In part due to some Makefiles
>> >   building files in subdirectory hierarchies.)
>> > 
>> > - The debate which headers, if any, should really be self-contained is
>> >   virtually open-ended.
>> 
>> I would expect that headers in include/uapi/*.h should be self-contained,
>> but apparently that's just a pipe dream on my part, or maybe it's just
>> a maintainer option.
>> 
>> With 'make HEADER_CHECK="include/uapi/" headercheck'
>> I see over 70 errors (mostly typedefs or defined constants, but not only
>> those), such as:
>> 
>> In file included from <command-line>:
>> ./../include/uapi/linux/hdlc/ioctl.h:74:21: error: ‘IFNAMSIZ’ undeclared here (not in a function)
>>    74 |         char master[IFNAMSIZ];  /* Name of master FRAD device */
>>       |                     ^~~~~~~~
>> In file included from <command-line>:
>> ./../include/uapi/linux/input.h:29:6: warning: ‘__BITS_PER_LONG’ is not defined, evaluates to ‘0’ [-Wundef]
>>    29 | #if (__BITS_PER_LONG != 32 || !defined(__USE_TIME_BITS64)) && !defined(__KERNEL__)
>>       |      ^~~~~~~~~~~~~~~
>> ./../include/uapi/linux/input.h:34:9: error: unknown type name ‘__kernel_ulong_t’
>>    34 |         __kernel_ulong_t __sec;
>>       |         ^~~~~~~~~~~~~~~~
>> In file included from ./../include/uapi/linux/papr_pdsm.h:14,
>>                  from <command-line>:
>> ../include/linux/ndctl.h:19:33: error: ‘PAGE_SIZE’ undeclared here (not in a function)
>>    19 |         ND_MIN_NAMESPACE_SIZE = PAGE_SIZE,
>>       |                                 ^~~~~~~~~
>> In file included from <command-line>:
>> ./../include/uapi/linux/patchkey.h:15:2: error: #error "patchkey.h included directly"
>>    15 | #error "patchkey.h included directly"
>>       |  ^~~~~
>> In file included from <command-line>:
>> include/uapi/xen/gntdev.h:159:25: error: unknown type name ‘grant_ref_t’
>>   159 |                         grant_ref_t ref;
>>       |                         ^~~~~~~~~~~
>> include/uapi/xen/gntdev.h:161:25: error: unknown type name ‘domid_t’
>>   161 |                         domid_t domid;
>>       |                         ^~~~~~~
>
> include/uapi already has its own header checking infrastructure under
> CONFIG_UAPI_HEADER_TEST and usr/include/Makefile, which avoids this with
> a no-header-test list that includes many of the files listed in these
> messages. To be honest, we should probably forbid HEADER_CHECK from
> including 'include/uapi' and refer people to use CONFIG_UAPI_HEADER_TEST
> instead, as there are other differences like being built under a
> different C standard or C++ and such that the existing infrastructure
> handles.

Something like this on top would fail if there are any include/uapi
headers in there:

diff --git a/Makefile b/Makefile
index 4851a4407149..77253506975e 100644
--- a/Makefile
+++ b/Makefile
@@ -1554,6 +1554,7 @@ header-check-targets := $(patsubst %.h,%.header-check,$(sort $(header-check-file
 
 headercheck:
 	$(if $(header-check-targets),,$(error $@ found no headers in HEADER_CHECK="$(HEADER_CHECK)"))
+	$(if $(filter include/uapi/%,$(header-check-targets)),$(error $@ found include/uapi headers in HEADER_CHECK="$(HEADER_CHECK)"))
 	$(Q)$(MAKE) $(header-check-targets)
 else
 headercheck:


I'll wait a bit for more feedback before sending a v2.

> That said, I think the overall idea seems fine and relatively clean, at
> least from my Kbuild perspective, as it is completely opt in, so the
> previous objection to CONFIG_HEADER_CHECK_DISABLE and widely exposing
> this to builds is pretty much moot.

Thanks Nathan and Randy, this feels encouraging. :)

BR,
Jani.

-- 
Jani Nikula, Intel

  parent reply	other threads:[~2026-09-17  8:14 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-15 10:43 Jani Nikula
2026-09-16 22:24 ` Randy Dunlap
2026-09-16 23:13   ` Nathan Chancellor
2026-09-17  2:26     ` Randy Dunlap
2026-09-17  8:14     ` Jani Nikula [this message]
2026-09-17  9:05     ` Thomas Weißschuh
2026-09-17 11:58       ` Jason Gunthorpe
2026-09-17 17:17       ` Nathan Chancellor

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=67e8d510f7ea3e79035197c53ac215c23c8633ce@intel.com \
    --to=jani.nikula@intel.com \
    --cc=jgg@nvidia.com \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masahiroy@kernel.org \
    --cc=nathan@kernel.org \
    --cc=nsc@kernel.org \
    --cc=rdunlap@infradead.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®