All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dmitry Ilvokhin <d@ilvokhin.com>
To: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Dan Williams <djbw@kernel.org>,
	Vishal Verma <vishal.l.verma@intel.com>,
	Dave Jiang <dave.jiang@intel.com>,
	Ira Weiny <ira.weiny@intel.com>, Miguel Ojeda <ojeda@kernel.org>,
	Thomas Gleixner <tglx@kernel.org>,
	Christian Brauner <brauner@kernel.org>,
	Marco Elver <elver@google.com>, "H. Peter Anvin" <hpa@zytor.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	nvdimm@lists.linux.dev, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, kernel-team@meta.com
Subject: Re: [PATCH v3 3/4] cleanup: Annotate guard constructors with __nonnull()
Date: Tue, 19 May 2026 11:54:49 +0000	[thread overview]
Message-ID: <agxPiQKt2pykEIA4@shell.ilvokhin.com> (raw)
In-Reply-To: <CANiq72mG-EpBWbW_hZYPgtV_R1vyUBsn0ytaz2X2Zw9fr0keOA@mail.gmail.com>

On Mon, May 18, 2026 at 08:19:35PM +0200, Miguel Ojeda wrote:
> On Mon, May 18, 2026 at 5:22 PM Dmitry Ilvokhin <d@ilvokhin.com> wrote:
> >
> > Add __nonnull() to unconditional guard constructors so the compiler
> > verifies at each call site that NULL is never passed:
> 
> > This provides automated, compiler-enforced verification that no
> > unconditional guard constructor receives NULL.
> 
> I wouldn't say "verify", since the compiler does a best-effort here
> with the information it has statically.
> 
> In other words, the attribute does not prevent NULL pointers to be passed.

Fair enough.

I'll re-word this paragraph as "Add __nonnull() to unconditional guard
constructors so the compiler warns when NULL is statically known to be
passed" and drop the "compiler-enforced verification" paragraph.

> 
> > + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-nonnull-function-attribute
> 
> Hmm... It appears GCC has changed the docs in commit 6e3c137f5dbb
> ("doc: Merge function, variable, type, and statement attribute
> sections [PR88472]"), dropping the per-kind attribute pages.
> 
> So the right link would need to be now:
> 
>   https://gcc.gnu.org/onlinedocs/gcc/Common-Attributes.html#index-nonnull
> 
> I will need to send a patch to fix the other links.

Fixed locally. Thanks!

> 
> > + * clang: https://clang.llvm.org/docs/AttributeReference.html#nonnull
> 
> I think this link goes to `_Nonnull` -- the GNU one is instead:
> 
>   https://clang.llvm.org/docs/AttributeReference.html#id10
> 
> (I don't love the numeric IDs, though, since they break, so I think it
> is fine either way -- the `_Nonnull` is fairly close to the one we
> want and I hope that one doesn't break)

I don't quite like numeric IDs either. There is only one #id reference
in include/linux/compiler_attributes.h and link is already dead. I'll
keep current link since it gives at least some clue what to look for on
the page.

> 
> > + */
> > +#define __nonnull(x...)                        __attribute__((__nonnull__(x)))
> 
> This is indeed available for a long time, and we already use it
> elsewhere in the kernel tree (which would be nice to clean up
> separately).

> 
> If you don't mind, please place it before `__nonstring__` (the file is
> meant to be sorted by the actual attribute name -- there are a few
> instances where this is not the case anymore, which I will eventually
> clean up)

Thanks, fixed locally.

> 
> Thanks!
> 
> Cheers,
> Miguel


  reply	other threads:[~2026-05-19 11:54 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-18 15:21 [PATCH v3 0/4] cleanup: Remove NULL check from unconditional guards Dmitry Ilvokhin
2026-05-18 15:21 ` [PATCH v3 1/4] nvdimm: Convert nvdimm_bus guard to class Dmitry Ilvokhin
2026-05-18 15:45   ` Dave Jiang
2026-05-18 15:21 ` [PATCH v3 2/4] genirq: Move NULL check into irqdesc_lock guard unlock expression Dmitry Ilvokhin
2026-05-18 15:21 ` [PATCH v3 3/4] cleanup: Annotate guard constructors with __nonnull() Dmitry Ilvokhin
2026-05-18 18:19   ` Miguel Ojeda
2026-05-19 11:54     ` Dmitry Ilvokhin [this message]
2026-05-19 12:45       ` Miguel Ojeda
2026-05-18 15:21 ` [PATCH v3 4/4] cleanup: Remove NULL check from unconditional guards Dmitry Ilvokhin

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=agxPiQKt2pykEIA4@shell.ilvokhin.com \
    --to=d@ilvokhin.com \
    --cc=akpm@linux-foundation.org \
    --cc=brauner@kernel.org \
    --cc=dave.jiang@intel.com \
    --cc=djbw@kernel.org \
    --cc=elver@google.com \
    --cc=hpa@zytor.com \
    --cc=ira.weiny@intel.com \
    --cc=kernel-team@meta.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=miguel.ojeda.sandonis@gmail.com \
    --cc=nvdimm@lists.linux.dev \
    --cc=ojeda@kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@kernel.org \
    --cc=vishal.l.verma@intel.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.