linux-kernel - Re: [PATCH v3 1/2] rust: Introduce file_from

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aFFwumsjuAEIJVUF@Mac.home>
Date: Tue, 17 Jun 2025 06:42:18 -0700
From: Boqun Feng <boqun.feng@...il.com>
To: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
Cc: linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org,
	ojeda@...nel.org, alex.gaynor@...il.com, gary@...yguo.net,
	bjorn3_gh@...tonmail.com, benno.lossin@...ton.me,
	a.hindborg@...sung.com, aliceryhl@...gle.com, tmgross@...ch.edu,
	dakr@...nel.org, mingo@...hat.com, peterz@...radead.org,
	juri.lelli@...hat.com, vincent.guittot@...aro.org,
	dietmar.eggemann@....com, rostedt@...dmis.org, bsegall@...gle.com,
	mgorman@...e.de, vschneid@...hat.com, pmladek@...e.com,
	fujita.tomonori@...il.com, mingo@...nel.org
Subject: Re: [PATCH v3 1/2] rust: Introduce file_from_location()

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@...il.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