Re: [PATCH] rust: kernel: introduce `unsafe_precondition_assert!` macro

[Ritvik Gupta <ritvikfoss@gmail.com>]

On Mon, Jul 21, 2025 at 11:51:31PM +0200, Miguel Ojeda wrote:
> On Wed, Jul 16, 2025 at 6:58 AM Ritvik Gupta <ritvikfoss@gmail.com> wrote:
> >
> > macro. It is a wrapper around `debug_assert!`, intended for validating
> > pre-conditions of unsafe code blocks and functions.
> 
> It isn't really a wrapper, since it doesn't call it, no?

It was intended to work the same way as `debug_assert!` [1]
with 'unsafe precondition(s) violated' message, if assertion fails.

> > +//! This module contains the kernel APIs for verifying invariants
> > +//! required by the unsafe code.
> 
> We normally call these preconditions, and normally I think "unsafe
> code" is associated with unsafe blocks, rather than e.g. `unsafe fn`.
> 
> In addition, we may want to use this module for more things, so I
> would probably just say something more general, e.g. "Safety-related
> APIs" or similar. Probably Benno can chime in here.
> 
> > +/// Checks that preconditions of an unsafe code are followed.
> 
> Same as above -- I would say unsafe function (if we really want to use
> this only for that).
> 
> > +/// are enabled. In release builds, this macro is no-op.
> 
> I would say "Otherwise, ...". Same in the commit message.
>
> > +/// // SAFETY: The caller ensures the size and alignment
> 
> This should be a `# Safety` section, not a `SAFETY` comment. Please
> see how we usually write those in other files.

I'll fix the docs.
Yes, @Benno's input would be super helpful as well :)

> > +        if cfg!(debug_assertions) {
> > +            crate::pr_err!("unsafe precondition(s) violated");
> > +            ::core::assert!($($arg)*);
> > +        }
> 
> Doesn't this print an error every time debug assertions are enabled,
> whether or not the expression is true?

You're right! I messed up the testing part (explained later).

> Also, shouldn't that be `$crate`?

I think, in this case, the compiler automatically resolved it to correct path.
Nevertheless, I will use `$crate`.

> This leads me to believe the patch wasn't tested -- please test
> patches before submitting them! (Or, if it couldn't be tested for some
> reason, please say so in the message)

The code did compile, but during testing I only verified the failing case
and overlooked the passing one.

> Finally, could we actually wrap/forward the call to `debug_assert!`?
> If we want to add a custom message, could we match the parameters so
> that we can then prefix it?

Yes, I'll do this way in v2.

Thanks for the feedback :)

[1]: https://doc.rust-lang.org/src/core/macros/mod.rs.html#306-312