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

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

On Tue Jun 10, 2025 at 12:36 AM CEST, Timur Tabi wrote:
> Introduce the sfile (short file) macro that returns the stem of
> the current source file filename.
>
> Rust provides a file!() macro that is similar to C's __FILE__ predefined
> macro.  Unlike __FILE__, however, file!() returns a full path, which is
> klunky when used for debug traces such as
>
> 	pr_info!("{}:{}\n", file!(), line!());
>
> sfile!() can be used in situations instead, to provide a more compact
> print.  For example, if file!() returns "rust/kernel/print.rs", sfile!()
> returns just "print".
>
> The macro avoids str::rfind() because currently, that function is not
> const.  The compiler emits a call to memrchr, even when called on string
> literals.  Instead, the macro implements its own versions of rfind(),
> allowing the compiler to generate the slice at compile time.
>
> The macro also uses some unsafe functions in order to avoid indexing
> into a str, which is necessary to fully support const contexts.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
>  rust/kernel/print.rs | 66 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
>
> diff --git a/rust/kernel/print.rs b/rust/kernel/print.rs
> index 9783d960a97a..5b0c36481e95 100644
> --- a/rust/kernel/print.rs
> +++ b/rust/kernel/print.rs
> @@ -423,3 +423,69 @@ macro_rules! pr_cont (
>          $crate::print_macro!($crate::print::format_strings::CONT, true, $($arg)*)
>      )
>  );
> +/// Returns the index of the last occurrence of `needle` in `haystack`, or zero.
> +///
> +/// Identical to [`str::rfind()`], but unlike that function, this one is const.
> +/// That is, the compiler can inline it when called with a string literal.

The last sentence is incorrect. The compiler can always inline the
function. `const` means that the function can be called from const
evaluation such as in constant or static definitions, const blocks etc.

I would just remove the second sentence.

Another difference between this and `str::rfind` is that `str::rfind`
takes an `impl Pattern` and not a char. Additionally, I presume that
`str::rfind` is more efficiently implemented (as it's more complex, see
[1], but I didn't benchmark the two versions :).

[1]: https://doc.rust-lang.org/src/core/str/pattern.rs.html#478

> +#[inline]
> +pub const fn rfind_const(haystack: &str, needle: char) -> Option<usize> {
> +    let bytes = haystack.as_bytes();
> +    let mut i = haystack.len();
> +    while i > 0 {
> +        i -= 1;
> +        if bytes[i] == needle as u8 {
> +            return Some(i);
> +        }
> +    }
> +    None
> +}

This function should not be in this module, `str.rs` is probably a
better place for it.

> +
> +/// Returns just the base filename of the current file.
> +///
> +/// This differs from the built-in [`file!()`] macro, which returns the full
> +/// path of the current file.
> +///
> +/// Using the base filename gives you more succinct logging prints.

I would remove this sentence or replace it with something like "Useful
for succinct logging purposes.".

> +///
> +/// # Examples
> +///
> +/// ```
> +/// use kernel::sfile
> +/// pr_err!("{}:{}\n", sfile!(), line!());
> +/// ```
> +#[macro_export]
> +macro_rules! sfile {
> +    () => {{
> +        use kernel::print::rfind_const;

Please don't use `use` in macros.

> +
> +        const FILE: &str = file!();

Instead use absolute paths: `::core::file!()`.

> +
> +        // [`.as_bytes()`] does not support non-ASCII filenames
> +        assert!(FILE.is_ascii());

Also here.

> +
> +        let start = match rfind_const(FILE, '/') {

And here `::kernel::print::rfind_const` (more below).

> +            Some(slash) => slash + 1,
> +            None => 0,
> +        };
> +        let end = match rfind_const(FILE, '.') {
> +            Some(dot) => dot,
> +            None => FILE.len(),
> +        };
> +
> +        // The following code the equivalent of &FILE[start..start+len],
> +        // except that it allows for const contexts.  For example,
> +        //   const SFILE: &'static str = sfile!();
> +
> +        let base_ptr: *const u8 = FILE.as_ptr();
> +
> +        // SAFETY: We know that `start` is <= the length of FILE, because FILE
> +        // never ends in a slash.

I don't think that we should rely on that fact for unsafe code.

There is another argument for `start <= length`: when `rfind_const`
returns `Some(res)`, then `res < length`.

> +        let ptr: *const u8 = unsafe { base_ptr.add(start) };
> +        // SAFETY: We also know that `end` < the length of FILE.
> +        // If `end` < `start`, this will generate a compiler error.

No it won't generate a compiler error if used like in the example, it
will panic at runtime.

Maybe we should make this macro return a const block?

---
Cheers,
Benno

> +        let slice = unsafe { core::slice::from_raw_parts(ptr, end - start) };
> +        // SAFETY: We know that the slice is valid UTF-8, because we checked
> +        // that FILE is ASCII (via is_ascii() above).
> +        unsafe { core::str::from_utf8_unchecked(slice) }
> +    }};
> +}
>
> base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
> prerequisite-patch-id: 4bfda16a333b9f00a139050b7c6875922961a15d