"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
