Re: [PATCH v4 3/4] cleanup: Annotate guard constructors with __nonnull()

[Dmitry Ilvokhin <d@ilvokhin.com>]

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@ilvokhin.com>
> > Acked-by: Miguel Ojeda <ojeda@kernel.org>
> 
> 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.