On Wed, Jul 09, 2025 at 05:49:57PM +0200, Andreas Hindborg wrote: > "Alice Ryhl" <aliceryhl@xxxxxxxxxx> writes: > > > On Tue, Jul 08, 2025 at 09:44:58PM +0200, Andreas Hindborg wrote: > >> Add `NullBorrowFormatter`, a formatter that writes a null terminated string > >> to an array or slice buffer. Because this type needs to manage the trailing > >> null marker, the existing formatters cannot be used to implement this type. > >> > >> Signed-off-by: Andreas Hindborg <a.hindborg@xxxxxxxxxx> > >> --- > >> rust/kernel/str.rs | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > >> 1 file changed, 59 insertions(+) > >> > >> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs > >> index 78b2f95eb3171..05d79cf40c201 100644 > >> --- a/rust/kernel/str.rs > >> +++ b/rust/kernel/str.rs > >> @@ -860,6 +860,65 @@ fn deref_mut(&mut self) -> &mut Self::Target { > >> } > >> } > >> > >> +/// 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 NullBorrowFormatter<'a> { > >> + buffer: &'a mut [u8], > >> + pos: usize, > >> +} > > > > Do you need `pos`? Often I see this kind of code subslice `buffer` > > instead. > > How would that work? Can I move the start index of `buffer` in some way > without an unsafe block? Yes. I think this will work: let buffer = mem::take(&mut self.buffer); self.buffer = &mut buffer[pos..]; Temporarily storing an empty slice avoids lifetime issues. > >> + #[expect(dead_code)] > >> + pub(crate) fn from_array<const N: usize>( > >> + a: &'a mut [crate::ffi::c_char; N], > >> + ) -> Result<NullBorrowFormatter<'a>> { > >> + Self::new( > >> + // SAFETY: the buffer of `a` is valid for read and write as `u8` for > >> + // at least `N` bytes. > >> + unsafe { core::slice::from_raw_parts_mut(a.as_mut_ptr().cast::<u8>(), N) }, > >> + ) > >> + } > > > > Arrays automatically coerce to slices, so I don't think this is > > necessary. You can just call `new`. > > Nice! I'm guessing it used to be necessary back when we used core::ffi::c_char since &[i8;N] doesn't coerce to &[u8]. But now that we use the right c_char definition, that isn't the case anymore. > >> +impl Write for NullBorrowFormatter<'_> { > >> + 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 > >> + if self.pos + len > self.buffer.len() - 1 { > > > > Integer overflow? > > In the subtraction? `buffer.len()` is at least 1, because of the type invariant. > > Or do you mean I should do a checked add for `self.pos + len`? Ah, I guess self.pos and len are both <= the length of a slice, which is at most isize::MAX, so the addition can't overflow an usize. Would be good to comment this, though. Alice