Re: [PATCH v4] rust: introduce sfile macro for succinct code tracing

["Benno Lossin" <lossin@kernel.org>]

On Mon Jun 16, 2025 at 11:03 PM CEST, Miguel Ojeda wrote:
> On Mon, Jun 16, 2025 at 9:35 PM Timur Tabi <ttabi@nvidia.com> wrote:
>> +#[macro_export]
>> +macro_rules! sfile {
>> +    () => {{
>> +        const {
>
> Could we avoid the double block?

We should be able to.

>> +            let start = match ::kernel::str::rfind_const(FILE, '/') {
>> +                Some(slash) => slash + 1,
>> +                None => 0,
>> +            };
>> +            let end = match ::kernel::str::rfind_const(FILE, '.') {
>> +                Some(dot) => dot,
>> +                None => FILE.len(),
>> +            };
>
> I wonder if we should just panic if they are not found. Maybe I am not
> creative enough, but do you have in mind a use case for either?

Sounds like a good idea.

> We already remove some cases anyway with the `assert!` below, so it is
> not usable in every case anyway.

>> +            let ptr: *const u8 = unsafe { base_ptr.add(start) };
>
> Benno: do you want here (and above) qualified calls too? (I would move
> everything to a `const fn` though -- please see below)

Yeah we should do that. Some of my macros don't do it (eg use `Ok` and
`Err` without qualifying them), but we should make all macros always
exclusively use absolute paths for everything. (`Ok` and `Err` are
probably never going to be different ones in scope, but you never know)

@Timur for this case, you can qualify the call like this:

    let ptr = unsafe { <*const ::core::primitives::u8>::add(base_ptr, start) };

Maybe the stdlib should have a re-export of `*const` and `*mut` in
primitives...

> I would just call both pointers `p`, by the way. No need to keep the
> base one around.
>
>> +            // that FILE is ASCII (via is_ascii() above).
>
> `FILE`, `is_ascii()`
>
>> +/// Returns the index of the last occurrence of `needle` in `haystack`, or None.
>
> [`None`]
>
>> +/// use kernel::print::rfind_const;
>
> Consider hiding this one too.
>
>> +/// let l = rfind_const("when will then be now?", 'l');
>> +/// assert!(l == Some(8));
>
> Please add an example for the `None` case too.
>
> Finally, should we put most of this logic in a `const fn` too for the
> usual benefits (docs and tests)? Then the macro would just be
> essentially a const `basename(file!())` call, possibly fallible, and
> you can even avoid having all those qualified calls :)

A function sounds like a good idea.

But I don't see the value in the macro being fallible. Then the macro is
less convenient to use. The function can be fallible, but then the macro
should use `Result::expect()`.

---
Cheers,
Benno