Re: [PATCH 3/9] rust: block,core: rename `RawWriter` to `BufferWriter`

[Andreas Hindborg]

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

> On Mon, Jun 16, 2025 at 3:26 PM Andreas Hindborg <a.hindborg@xxxxxxxxxx> 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