Re: [PATCH v5 3/4] cleanup: Annotate guard constructors with nonnull

[Dmitry Ilvokhin <d@ilvokhin.com>]

On Fri, Jun 05, 2026 at 08:46:57AM +0300, Dan Carpenter wrote:
> On Tue, Jun 02, 2026 at 07:12:52AM +0000, Dmitry Ilvokhin wrote:
> > Add __nonnull_args() to unconditional guard constructors so the compiler
> > warns when NULL is statically known to be passed:
> > 
> > - DEFINE_GUARD(): re-declare the constructor with __nonnull_args().
> > - __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_args() macro in compiler_attributes.h, following
> > the existing convention for attribute wrappers. Deliberately not named
> > '__nonnull', to avoid clashing with glibc's __nonnull() when kernel and
> > userspace headers are combined (User Mode Linux for example).
> > 
> > Signed-off-by: Dmitry Ilvokhin <d@ilvokhin.com>
> > ---
> > Miguel, I dropped your Acked-by due to the rename. Went with
> > __nonnull_args() (over __knonnull()). Happy to restore your tag if that
> > spelling works for you.
> > 
> 
> Sparse doesn't like an empty __nonnull_args() at all.
> 
> ./include/linux/spinlock.h:608:1: error: an expression is expected before ')'
> ./include/linux/spinlock.h:619:1: error: an expression is expected before ')'
> ./include/linux/spinlock.h:631:1: error: an expression is expected before ')'
> 
> Shouldn't we specify the arguments which are non-NULL?
> __nonnull_args(1)?

Thanks for the report, Dan.

That is interesting, it seems sparse trips over the empty-parens form
the guard macro expands to, nonnull(), but is fine with
__attribute__((nonnull)) without explicit arguments, that a few places
in-tree already use. GCC and clang accept both, so this probably
deserves a fix in sparse too.

Meanwhile, spelling the argument out explicitly is a simple and
reasonable fix (and arguably clearer). Patch below.

From 50f7e7eaad1d33773cee9a278625fdb4b451f53b Mon Sep 17 00:00:00 2001
From: Dmitry Ilvokhin <d@ilvokhin.com>
Date: Fri, 5 Jun 2026 03:06:22 -0700
Subject: [PATCH] cleanup: Specify nonnull argument index

The guard constructors were annotated with an empty __nonnull_args(),
relying on __nonnull__() marking every pointer parameter as non-NULL.
Sparse cannot parse the empty argument list.

Both constructors take the lock pointer as their first parameter, so
specify the index explicitly: __nonnull_args(1).

Reported-by: Dan Carpenter <error27@gmail.com>
Closes: https://lore.kernel.org/all/aiJi0WcYE8FZt-jO@stanley.mountain/
Signed-off-by: Dmitry Ilvokhin <d@ilvokhin.com>
---
 include/linux/cleanup.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h
index 65416938e318..b1b5698cbf1b 100644
--- a/include/linux/cleanup.h
+++ b/include/linux/cleanup.h
@@ -397,7 +397,7 @@ static __maybe_unused const bool class_##_name##_is_conditional = _is_cond
 	__DEFINE_GUARD_LOCK_PTR(_name, _T)
 
 #define DEFINE_GUARD(_name, _type, _lock, _unlock) \
-	static __always_inline __nonnull_args() _type class_##_name##_constructor(_type _T); \
+	static __always_inline __nonnull_args(1) _type class_##_name##_constructor(_type _T); \
 	DEFINE_CLASS(_name, _type, _unlock, ({ _lock; _T; }), _type _T); \
 	DEFINE_CLASS_IS_GUARD(_name)
 
@@ -498,7 +498,7 @@ static __always_inline void class_##_name##_destructor(class_##_name##_t *_T) \
 __DEFINE_GUARD_LOCK_PTR(_name, &_T->lock)
 
 #define __DEFINE_LOCK_GUARD_1(_name, _type, ...)			\
-static __always_inline __nonnull_args()					\
+static __always_inline __nonnull_args(1)				\
 class_##_name##_t class_##_name##_constructor(_type *l)			\
 	__no_context_analysis						\
 {									\
-- 
2.53.0-Meta