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

From: Miguel Ojeda
Date: Tue Jun 17 2025 - 13:24:27 EST


On Tue, Jun 17, 2025 at 6:58 PM Boqun Feng <boqun.feng@xxxxxxxxx> wrote:
>
> This actually helped me find a bug in the current implementation: I
> should use core::ffi::CStr::to_bytes_with_nul() instead of to_bytes().
> Please see below for the update "Examples" section:

Yeah, writing examples can force us to find issues :)

I guess we could conditionally (`cfg`) assert in the "otherwise" case,
since we already had one case, but I didn't suggest it earlier because
it is a bit heavy, and the interesting case is the other one anyway so
it wouldn't have caught the issue. I guess we could assert it ends
with `.rs` for the interesting one.

By the way, I would avoid the actual filename, i.e. I would give a
more "normal" example instead of the `doctests_kernel_generated` one
of the example itself. Something like:

// Output:
// - A path like `rust/kernel/example.rs` if `file_with_nul()` is available.
// - `<Location::file_with_nul() not supported>` otherwise.

It could make sense to have an intermediate variable (especially if
you end up asserting anything), then you could put the comment on top
of that instead. Then the `pr_*` call could then perhaps show a
"realistic" example, and could inline the variable name, e.g.
something like `{caller_file}: my message\n`.

Anyway, no big deal either way, what you had is also OK.

By the way, I noticed a typo in "with a NUL terminated." above.

Cheers,
Miguel