linux-kernel - Re: [PATCH v2 05/14] rust: block: use `NullBorrowFormatter`

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: <aG5ttHBYW3SQlSv7@google.com>
Date: Wed, 9 Jul 2025 13:25:08 +0000
From: Alice Ryhl <aliceryhl@...gle.com>
To: Andreas Hindborg <a.hindborg@...nel.org>
Cc: Boqun Feng <boqun.feng@...il.com>, Miguel Ojeda <ojeda@...nel.org>, 
	Alex Gaynor <alex.gaynor@...il.com>, Gary Guo <gary@...yguo.net>, 
	"Björn Roy Baron" <bjorn3_gh@...tonmail.com>, Benno Lossin <lossin@...nel.org>, 
	Trevor Gross <tmgross@...ch.edu>, Danilo Krummrich <dakr@...nel.org>, Jens Axboe <axboe@...nel.dk>, 
	linux-block@...r.kernel.org, rust-for-linux@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 05/14] rust: block: use `NullBorrowFormatter`

On Tue, Jul 08, 2025 at 09:45:00PM +0200, Andreas Hindborg wrote:
> Use the new `NullBorrowFormatter` to write the name of a `GenDisk` to the
> name buffer. This new formatter automatically adds a trailing null marker
> after the written characters, so we don't need to append that at the call
> site any longer.
> 
> Signed-off-by: Andreas Hindborg <a.hindborg@...nel.org>
> ---
>  rust/kernel/block/mq/gen_disk.rs   | 8 ++++----
>  rust/kernel/block/mq/raw_writer.rs | 1 +
>  rust/kernel/str.rs                 | 7 -------
>  3 files changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs
> index 679ee1bb21950..e0e42f7028276 100644
> --- a/rust/kernel/block/mq/gen_disk.rs
> +++ b/rust/kernel/block/mq/gen_disk.rs
> @@ -7,9 +7,10 @@
>  
>  use crate::{
>      bindings,
> -    block::mq::{raw_writer::RawWriter, Operations, TagSet},
> +    block::mq::{Operations, TagSet},
>      error::{self, from_err_ptr, Result},
>      static_lock_class,
> +    str::NullBorrowFormatter,
>      sync::Arc,
>  };
>  use core::fmt::{self, Write};
> @@ -143,14 +144,13 @@ pub fn build<T: Operations>(
>          // SAFETY: `gendisk` is a valid pointer as we initialized it above
>          unsafe { (*gendisk).fops = &TABLE };
>  
> -        let mut raw_writer = RawWriter::from_array(
> +        let mut writer = NullBorrowFormatter::from_array(
>              // SAFETY: `gendisk` points to a valid and initialized instance. We
>              // have exclusive access, since the disk is not added to the VFS
>              // yet.
>              unsafe { &mut (*gendisk).disk_name },
>          )?;
> -        raw_writer.write_fmt(name)?;
> -        raw_writer.write_char('\0')?;
> +        writer.write_fmt(name)?;

Although this is nicer than the existing code, I wonder if it should
just be a function rather than a whole NullBorrowFormatter struct? Take
a slice and a fmt::Arguments and write it with a nul-terminator. Do you
need anything more complex than what you have here?

Alice