"Alice Ryhl" <aliceryhl@xxxxxxxxxx> 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@xxxxxxxxxx>
>> ---
>> 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
