Rename the `RawWriter` to `BufferWriter`, wihich is a more suitable name.
Also move the module from `block` to `str`.

The ability to format a string to a byte buffer is something that is not
specific to `block`, so there is no reason this code should live in
`block`.

Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>

---

`BufferWriter` is used in `rnull` for interacting with `configfs`.
---
 rust/kernel/block/mq.rs                                 |  1 -
 rust/kernel/block/mq/gen_disk.rs                        |  9 +++++----
 rust/kernel/str.rs                                      |  3 +++
 .../{block/mq/raw_writer.rs => str/buffer_writer.rs}    | 17 +++++++++++------
 4 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs
index fb0f393c1cea..faa3ccb5a49a 100644
--- a/rust/kernel/block/mq.rs
+++ b/rust/kernel/block/mq.rs
@@ -89,7 +89,6 @@
 
 pub mod gen_disk;
 mod operations;
-mod raw_writer;
 mod request;
 mod tag_set;
 
diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs
index cd54cd64ea88..a04b709514ac 100644
--- a/rust/kernel/block/mq/gen_disk.rs
+++ b/rust/kernel/block/mq/gen_disk.rs
@@ -5,10 +5,11 @@
 //! C header: [`include/linux/blkdev.h`](srctree/include/linux/blkdev.h)
 //! C header: [`include/linux/blk_mq.h`](srctree/include/linux/blk_mq.h)
 
-use crate::block::mq::{raw_writer::RawWriter, Operations, TagSet};
+use crate::block::mq::{Operations, TagSet};
 use crate::{bindings, error::from_err_ptr, error::Result, sync::Arc};
 use crate::{error, static_lock_class};
 use core::fmt::{self, Write};
+use kernel::str::BufferWriter;
 
 /// A builder for [`GenDisk`].
 ///
@@ -139,14 +140,14 @@ 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 = BufferWriter::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)?;
+        writer.write_char('\0')?;
 
         // 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/str.rs b/rust/kernel/str.rs
index a927db8e079c..050793fb7d3a 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -936,3 +936,6 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
 macro_rules! fmt {
     ($($f:tt)*) => ( ::core::format_args!($($f)*) )
 }
+
+mod buffer_writer;
+pub use buffer_writer::BufferWriter;
diff --git a/rust/kernel/block/mq/raw_writer.rs b/rust/kernel/str/buffer_writer.rs
similarity index 77%
rename from rust/kernel/block/mq/raw_writer.rs
rename to rust/kernel/str/buffer_writer.rs
index 7e2159e4f6a6..364842a6cff8 100644
--- a/rust/kernel/block/mq/raw_writer.rs
+++ b/rust/kernel/str/buffer_writer.rs
@@ -10,14 +10,14 @@
 /// # Invariants
 ///
 /// `buffer` is always null terminated.
-pub(crate) struct RawWriter<'a> {
+pub struct BufferWriter<'a> {
     buffer: &'a mut [u8],
     pos: usize,
 }
 
-impl<'a> RawWriter<'a> {
-    /// Create a new `RawWriter` instance.
-    fn new(buffer: &'a mut [u8]) -> Result<RawWriter<'a>> {
+impl<'a> BufferWriter<'a> {
+    /// Create a new [`Self`] instance.
+    pub fn new(buffer: &'a mut [u8]) -> Result<BufferWriter<'a>> {
         *(buffer.last_mut().ok_or(EINVAL)?) = 0;
 
         // INVARIANT: We null terminated the buffer above.
@@ -26,16 +26,21 @@ fn new(buffer: &'a mut [u8]) -> Result<RawWriter<'a>> {
 
     pub(crate) fn from_array<const N: usize>(
         a: &'a mut [crate::ffi::c_char; N],
-    ) -> Result<RawWriter<'a>> {
+    ) -> Result<BufferWriter<'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) },
         )
     }
+
+    /// Return the position of the write pointer in the underlying buffer.
+    pub fn pos(&self) -> usize {
+        self.pos
+    }
 }
 
-impl Write for RawWriter<'_> {
+impl Write for BufferWriter<'_> {
     fn write_str(&mut self, s: &str) -> fmt::Result {
         let bytes = s.as_bytes();
         let len = bytes.len();

-- 
2.47.2

On Mon, Jun 16, 2025 at 3:26 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> Rename the `RawWriter` to `BufferWriter`, wihich is a more suitable name.
> Also move the module from `block` to `str`.

The prefix should probably be "rust: block,str", or just "rust:".

(This patch would be ideally first in the series rather than in the
middle, by the way)

> -pub(crate) struct RawWriter<'a> {
> +pub struct BufferWriter<'a> {

Since you are re-exporting, can this be kept for the crate?

> +    /// Create a new [`Self`] instance.

It is not a big deal here, but when you have a "move" commit, please
try to keep changes to the minimum, e.g. type renaming could be done
before or after.

> +    /// Return the position of the write pointer in the underlying buffer.
> +    pub fn pos(&self) -> usize {
> +        self.pos
> +    }

This is not mentioned in the commit message (and should have been a
different patch too).

By the way, cannot you use `{Raw,}Formatter`? You could add a
formatter that null-terminates automatically and/or that tracks the
lifetime, but we add the null manually elsewhere.

Speaking of which, aren't you null-terminating manually anyway in `gen_disk.rs`?

Ah, no, you are adding two nulls -- one at the end of the buffer, and
one in the middle. So the current code will fail if it needs one final
byte (i.e. when the last null would have been enough).

Given all that, I would say just drop this one, and use the one we
have. Then we can add a fancier formatter later on independently if
needed.

Cheers,
Miguel

"Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com> writes:

> On Mon, Jun 16, 2025 at 3:26 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>> Rename the `RawWriter` to `BufferWriter`, wihich is a more suitable name.
>> Also move the module from `block` to `str`.
>
> The prefix should probably be "rust: block,str", or just "rust:".

OK 👍

>
> (This patch would be ideally first in the series rather than in the
> middle, by the way)

I'll move it.

>
>> -pub(crate) struct RawWriter<'a> {
>> +pub struct BufferWriter<'a> {
>
> Since you are re-exporting, can this be kept for the crate?

Yep.

>
>> +    /// Create a new [`Self`] instance.
>
> It is not a big deal here, but when you have a "move" commit, please
> try to keep changes to the minimum, e.g. type renaming could be done
> before or after.

Will do.

>
>> +    /// Return the position of the write pointer in the underlying buffer.
>> +    pub fn pos(&self) -> usize {
>> +        self.pos
>> +    }
>
> This is not mentioned in the commit message (and should have been a
> different patch too).

Right.

> By the way, cannot you use `{Raw,}Formatter`? You could add a
> formatter that null-terminates automatically and/or that tracks the
> lifetime, but we add the null manually elsewhere.

I'll look into that. It looks like I could just use `RawFormatter`. I
don't recall the reason for `RawWriter`, it's been years and it was
Wedson who introduced it originally.

>
> Speaking of which, aren't you null-terminating manually anyway in `gen_disk.rs`?
>
> Ah, no, you are adding two nulls -- one at the end of the buffer, and
> one in the middle. So the current code will fail if it needs one final
> byte (i.e. when the last null would have been enough).

The null insertion at the call site should be removed, it's a leftover
from before `BufferWriter` handled that.

>
> Given all that, I would say just drop this one, and use the one we
> have. Then we can add a fancier formatter later on independently if
> needed.

Will try that. Thanks for taking a look.


Best regards,
Andreas Hindborg

On Mon, Jul 7, 2025 at 4:58 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> The null insertion at the call site should be removed, it's a leftover
> from before `BufferWriter` handled that.

Just in case -- I think that if you do that then you would only get
the null terminator at the end of the buffer.

i.e. the one your formatter (`RawWriter`) places is really at the end
of the buffer, rather than immediately after your formatted text, so
if you don't have the one at the call site, you would end with garbage
in the middle unless the disk name fits perfectly into the buffer.

Cheers,
Miguel

"Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com> writes:

> On Mon, Jul 7, 2025 at 4:58 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>> The null insertion at the call site should be removed, it's a leftover
>> from before `BufferWriter` handled that.
>
> Just in case -- I think that if you do that then you would only get
> the null terminator at the end of the buffer.
>
> i.e. the one your formatter (`RawWriter`) places is really at the end
> of the buffer, rather than immediately after your formatted text, so
> if you don't have the one at the call site, you would end with garbage
> in the middle unless the disk name fits perfectly into the buffer.

I'll be sure to thoroughly check everything. Thanks for taking a look.


Best regards,
Andreas Hindborg