"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?
>
>> +impl<'a> NullBorrowFormatter<'a> {
>> + /// Create a new [`Self`] instance.
>> + pub(crate) fn new(buffer: &'a mut [u8]) -> Result<NullBorrowFormatter<'a>> {
>> + *(buffer.first_mut().ok_or(EINVAL)?) = 0;
>> +
>> + // INVARIANT: We null terminated the buffer above.
>> + Ok(Self { buffer, pos: 0 })
>> + }
>
> I would probably just use an Option for this constructor.
OK.
>
>> + #[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!
>
>> + /// Return the position of the write pointer in the underlying buffer.
>> + #[expect(dead_code)]
>> + pub(crate) fn pos(&self) -> usize {
>> + self.pos
>> + }
>
> You delete this function in one of the later patches, so it makes more
> sense not to add it.
Oops.
>
>> +}
>> +
>> +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`?
Best regards,
Andreas Hindborg
