Re: [PATCH v6 2/5] rust: debugfs: Bind file creation for long-lived Display

[Danilo Krummrich <dakr@kernel.org>]

On Wed, Jun 18, 2025 at 02:28:14AM +0000, Matthew Maurer wrote:
> diff --git a/rust/kernel/debugfs/display_file.rs b/rust/kernel/debugfs/display_file.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..65e37e34d7b587482492dc296637a3bc517b9fe3
> --- /dev/null
> +++ b/rust/kernel/debugfs/display_file.rs
> @@ -0,0 +1,60 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2025 Google LLC.
> +
> +use crate::seq_file::SeqFile;
> +use crate::seq_print;
> +use core::fmt::Display;
> +
> +/// Implements `open` for `file_operations` via `single_open` to fill out a `seq_file`.
> +///
> +/// # Safety
> +///
> +/// * `inode`'s private pointer must point to a value of type `T` which will outlive the `inode`
> +///   and will not be mutated during this call.
> +/// * `file` must point to a live, not-yet-initialized file object.
> +pub(crate) unsafe extern "C" fn display_open<T: Display>(
> +    inode: *mut bindings::inode,
> +    file: *mut bindings::file,
> +) -> i32 {

Please use kernel::ffi::c_int instead.

> +    // SAFETY:
> +    // * `file` is acceptable by caller precondition.
> +    // * `print_act` will be called on a `seq_file` with private data set to the third argument,
> +    //   so we meet its safety requirements.
> +    // * The `data` pointer passed in the third argument is a valid `T` pointer that outlives
> +    //   this call by caller preconditions.
> +    unsafe { bindings::single_open(file, Some(display_act::<T>), (*inode).i_private) }
> +}
> +
> +/// Prints private data stashed in a seq_file to that seq file.
> +///
> +/// # Safety
> +///
> +/// `seq` must point to a live `seq_file` whose private data is a live pointer to a `T` which is
> +/// not being mutated.
> +pub(crate) unsafe extern "C" fn display_act<T: Display>(
> +    seq: *mut bindings::seq_file,
> +    _: *mut core::ffi::c_void,
> +) -> i32 {

kernel::ffi::c_int

> +    // SAFETY: By caller precondition, this pointer is live, points to a value of type `T`, and
> +    // is not being mutated.
> +    let data = unsafe { &*((*seq).private as *mut T) };
> +    // SAFETY: By caller precondition, `seq_file` points to a live `seq_file`, so we can lift
> +    // it.
> +    let seq_file = unsafe { SeqFile::from_raw(seq) };
> +    seq_print!(seq_file, "{}", data);
> +    0
> +}
> +
> +// Work around lack of generic const items.
> +pub(crate) trait DisplayFile: Display + Sized {
> +    const VTABLE: bindings::file_operations = bindings::file_operations {
> +        read: Some(bindings::seq_read),
> +        llseek: Some(bindings::seq_lseek),
> +        release: Some(bindings::single_release),
> +        open: Some(display_open::<Self> as _),

Why the cast? That shouldn't be needed.

> +        // SAFETY: `file_operations` supports zeroes in all fields.
> +        ..unsafe { core::mem::zeroed() }
> +    };
> +}