Re: [PATCH v3 1/2] rust: Introduce file_from_location()

[Boqun Feng <boqun.feng@gmail.com>]

On Mon, Jun 16, 2025 at 11:19:52PM +0200, Miguel Ojeda wrote:
> On Mon, Jun 16, 2025 at 5:36 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> >
> > introduce a file_from_location() function, which return a warning string
> 
> returns
> 

Fixed.

> >  #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(coerce_unsized))]
> >  #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(dispatch_from_dyn))]
> >  #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(unsize))]
> > +#![cfg_attr(CONFIG_RUSTC_HAS_LOCATION_FILE_WITH_NUL, feature(file_with_nul))]
> 
> I would change the config name to `CONFIG_RUSTC_HAS_FILE_WITH_NUL`
> since that is the actual name, i.e. without "location".
> 

Sounds good to me

> By the way, please add a comment on top, like the others, i.e. something like:
> 
>     //
>     // `feature(file_with_nul)` is expected to become stable. Before Rust
>     // 1.89.0, it did not exist, so enable it conditionally.
> 

Will do, one thing though: the comment lines seem to wrap at 78 or 80
chars, so do other lines for conditional features in rust/kernel/lib.rs.
However I believe in Rust code we use 100 chars text width, any
particular reason that I should keep these new lines the same (wrapping
at 80 characters)? Otherwise I will make the new lines wrap at 100.

> Alice: the tracking issue uses the wrong name, i.e. with the
> `location_*` prefix.
> 
> > +/// If `file_with_nul()` is not available, returns a string that warns about it.
> 
> Could we give a couple examples, i.e. of each case? (No need to assert anything)
> 

Sure, will do, but I'm afraid there is only case, unless I misunderstood
you:

/// # Examples
///
/// ```
/// use kernel::file_from_location;
///
/// #[track_caller]
/// fn foo() {
///     let caller = core::panic::Location::caller();
///
///     pr_info!("{}\n", file_from_location(caller));
/// }
///
/// foo();

> > +        use crate::c_str;
> 
> Do we need the `use`?
> 

No need for that, and thanks for catching it!

Regards,
Boqun

> Thanks!
> 
> Cheers,
> Miguel