Re: [PATCH v4 3/4] cleanup: Annotate guard constructors with __nonnull()
From: Dmitry Ilvokhin
Date: Tue May 26 2026 - 11:50:12 EST
On Sat, May 23, 2026 at 10:49:01AM +0200, Peter Zijlstra wrote:
> On Thu, May 21, 2026 at 07:18:03AM +0000, Dmitry Ilvokhin wrote:
> > Add __nonnull() to unconditional guard constructors so the compiler
> > warns when NULL is statically known to be passed:
> >
> > - DEFINE_GUARD(): re-declare the constructor with __nonnull().
> > - __DEFINE_LOCK_GUARD_1(): annotate the constructor directly.
> >
> > DEFINE_LOCK_GUARD_0() needs no annotation: its constructor takes no
> > pointer arguments (.lock is hardcoded to (void *)1).
> >
> > Define the __nonnull() macro in compiler_attributes.h, following the
> > existing convention for attribute wrappers.
> >
> > Signed-off-by: Dmitry Ilvokhin <d@xxxxxxxxxxxx>
> > Acked-by: Miguel Ojeda <ojeda@xxxxxxxxxx>
>
> The build robot found something to hate in this one. I think you're on
> Cc there. It looks to me like clang-23 is confused somehow, but who
> knows.
Seems like clang is not confused here, I was able to reproduce the problem
with GCC 11 as well.
There is a conflict with glibc's own __nonnull macro
https://elixir.bootlin.com/glibc/glibc-2.43/source/misc/sys/cdefs.h#L560-L562
which doesn't match the one from include/linux/compiler_attributes.h.
They usually don't collide, except for User Mode Linux builds, which
include both kernel and userspace headers.
Options are:
1. Drop the __nonnull() macro from include/linux/compiler_attributes.h
and use __attribute__((__nonnull__())) directly in
include/linux/cleanup.h. This is a bit unfortunate, since __nonnull()
seems like a useful shortcut, but seems like the simplest solution.
2. Rename __nonnull() to __nonnull_args() to avoid the conflict.
A returns_nonnull attribute is supported by compilers, so the name
fits, but it diverges from the existing naming convention in
include/linux/compiler_attributes.h.
3. #define __nonnull(params) __attribute__((__nonnull__ params)). This
keeps the name and is compatible with both kernel and glibc usage.
Current call sites use __nonnull() with no arguments, which works
identically. Future callers with specific parameter numbers would use
__nonnull((1, 2)) with double parens, matching glibc's convention. I
don't like this option, listed it here for the sake of completeness.
I am leaning towards option 1 so far.