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

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

On Sat Jun 14, 2025 at 12:46 AM CEST, Timur Tabi wrote:
> On Tue, 2025-06-10 at 10:45 +0200, Benno Lossin wrote:
>> > +            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`.
>
> The problem is the +1:
>
>     let start = match ::kernel::str::rfind_const(FILE, '/') {
>         Some(slash) => slash + 1,
>         None => 0,
>     };
>
> `start` is the first character *after* the slash.  If FILE ever did end in a slash, then `start`
> would point past the end of the string.  That didn't seem to cause a problem when I tested it, but
> technically my code just assumes it's true.
>
> I could add another assertion:
>
>             assert!(start < FILE.len());
>
> If I set FILE to "HELLO\", this causes a compiler error:
>
> 178 |         pr_info!("{}:{}\n", sfile!(), line!());
>     |                             ^^^^^^^^ the evaluated program panicked at 'assertion failed:
> start < FILE.len()', drivers/gpu/nova-core/gpu.rs:178:29
>
> or even:
>
>             assert!(start < end);
>
> This code is only ever supposed on be used on ASCII filenames, which can never end in a slash.
>
> I could do something like this:
>
>             let start = match ::kernel::str::rfind_const(FILE, '/') {
>                 Some(slash) if slash == FILE.len() => slash,
>                 Some(slash) => slash + 1,
>                 None => 0,
>             };
>
> But I don't think this is useful.  

Let's use an assert, since it shouldn't happen in practice. We need to
assert it though for the unsafe code to be correct.

---
Cheers,
Benno