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@kernel.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)?;
// SAFETY: `gendisk` points to a valid and initialized instance of
// `struct gendisk`. `set_capacity` takes a lock to synchronize this
diff --git a/rust/kernel/block/mq/raw_writer.rs b/rust/kernel/block/mq/raw_writer.rs
index 7e2159e4f6a6f..0aef55703e71d 100644
--- a/rust/kernel/block/mq/raw_writer.rs
+++ b/rust/kernel/block/mq/raw_writer.rs
@@ -24,6 +24,7 @@ fn new(buffer: &'a mut [u8]) -> Result<RawWriter<'a>> {
Ok(Self { buffer, pos: 0 })
}
+ #[expect(dead_code)]
pub(crate) fn from_array<const N: usize>(
a: &'a mut [crate::ffi::c_char; N],
) -> Result<RawWriter<'a>> {
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 05d79cf40c201..4140b4af64e50 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -881,7 +881,6 @@ pub(crate) fn new(buffer: &'a mut [u8]) -> Result<NullBorrowFormatter<'a>> {
Ok(Self { buffer, pos: 0 })
}
- #[expect(dead_code)]
pub(crate) fn from_array<const N: usize>(
a: &'a mut [crate::ffi::c_char; N],
) -> Result<NullBorrowFormatter<'a>> {
@@ -891,12 +890,6 @@ pub(crate) fn from_array<const N: usize>(
unsafe { core::slice::from_raw_parts_mut(a.as_mut_ptr().cast::<u8>(), N) },
)
}
-
- /// Return the position of the write pointer in the underlying buffer.
- #[expect(dead_code)]
- pub(crate) fn pos(&self) -> usize {
- self.pos
- }
}
impl Write for NullBorrowFormatter<'_> {
--
2.47.2On 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@kernel.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
"Alice Ryhl" <aliceryhl@google.com> writes:
> 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@kernel.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?
I don't need anything more complex right now. But I think the
`NullTerminatedFormatter` could be useful anyway:
+/// A mutable reference to a byte buffer where a string can be written into.
+///
+/// The buffer will be automatically null terminated after the last written character.
+///
+/// # Invariants
+///
+/// `buffer` is always null terminated.
+pub(crate) struct NullTerminatedFormatter<'a> {
+ buffer: &'a mut [u8],
+}
+
+impl<'a> NullTerminatedFormatter<'a> {
+ /// Create a new [`Self`] instance.
+ pub(crate) fn new(buffer: &'a mut [u8]) -> Option<NullTerminatedFormatter<'a>> {
+ *(buffer.first_mut()?) = 0;
+
+ // INVARIANT: We null terminated the buffer above.
+ Some(Self { buffer })
+ }
+
+ pub(crate) fn from_array<const N: usize>(
+ buffer: &'a mut [crate::ffi::c_char; N],
+ ) -> Option<NullTerminatedFormatter<'a>> {
+ Self::new(buffer)
+ }
+}
+
+impl Write for NullTerminatedFormatter<'_> {
+ fn write_str(&mut self, s: &str) -> fmt::Result {
+ let bytes = s.as_bytes();
+ let len = bytes.len();
+
+ // We want space for a null terminator. Buffer length is always at least 1, so no overflow.
+ if len > self.buffer.len() - 1 {
+ return Err(fmt::Error);
+ }
+
+ let buffer = core::mem::take(&mut self.buffer);
+ // We break the null termination invariant for a short while.
+ buffer[..len].copy_from_slice(bytes);
+ self.buffer = &mut buffer[len..];
+
+ // INVARIANT: We null terminate the buffer.
+ self.buffer[0] = 0;
+
+ Ok(())
+ }
+}
+
If you insist, I can write something like
fn format_to_buffer(buffer: &mut [u8], args: fmt::Arguments) -> fmt::Result
although I am not sure I see the point of this change.
Best regards,
Andreas Hindborg
On Fri, Jul 11, 2025 at 11:29 AM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> "Alice Ryhl" <aliceryhl@google.com> writes:
>
> > 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@kernel.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?
>
> I don't need anything more complex right now. But I think the
> `NullTerminatedFormatter` could be useful anyway:
>
> +/// A mutable reference to a byte buffer where a string can be written into.
> +///
> +/// The buffer will be automatically null terminated after the last written character.
> +///
> +/// # Invariants
> +///
> +/// `buffer` is always null terminated.
> +pub(crate) struct NullTerminatedFormatter<'a> {
> + buffer: &'a mut [u8],
> +}
> +
> +impl<'a> NullTerminatedFormatter<'a> {
> + /// Create a new [`Self`] instance.
> + pub(crate) fn new(buffer: &'a mut [u8]) -> Option<NullTerminatedFormatter<'a>> {
> + *(buffer.first_mut()?) = 0;
> +
> + // INVARIANT: We null terminated the buffer above.
> + Some(Self { buffer })
> + }
> +
> + pub(crate) fn from_array<const N: usize>(
> + buffer: &'a mut [crate::ffi::c_char; N],
> + ) -> Option<NullTerminatedFormatter<'a>> {
> + Self::new(buffer)
> + }
> +}
> +
> +impl Write for NullTerminatedFormatter<'_> {
> + fn write_str(&mut self, s: &str) -> fmt::Result {
> + let bytes = s.as_bytes();
> + let len = bytes.len();
> +
> + // We want space for a null terminator. Buffer length is always at least 1, so no overflow.
> + if len > self.buffer.len() - 1 {
> + return Err(fmt::Error);
> + }
> +
> + let buffer = core::mem::take(&mut self.buffer);
> + // We break the null termination invariant for a short while.
> + buffer[..len].copy_from_slice(bytes);
> + self.buffer = &mut buffer[len..];
> +
> + // INVARIANT: We null terminate the buffer.
> + self.buffer[0] = 0;
> +
> + Ok(())
> + }
> +}
> +
>
> If you insist, I can write something like
>
> fn format_to_buffer(buffer: &mut [u8], args: fmt::Arguments) -> fmt::Result
>
> although I am not sure I see the point of this change.
I don't mind. I just thought it was simpler since you only need to
support a single write instead of having to support multiple writes.
Alice
© 2016 - 2025 Red Hat, Inc.