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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87o6twoxoc.fsf@kernel.org>
Date: Mon, 07 Jul 2025 16:58:11 +0200
From: Andreas Hindborg <a.hindborg@...nel.org>
To: "Miguel Ojeda" <miguel.ojeda.sandonis@...il.com>
Cc: "Boqun Feng" <boqun.feng@...il.com>,  "Miguel Ojeda" <ojeda@...nel.org>,
  "Alex Gaynor" <alex.gaynor@...il.com>,  "Gary Guo" <gary@...yguo.net>,
  Björn Roy Baron <bjorn3_gh@...tonmail.com>,  "Benno
 Lossin"
 <lossin@...nel.org>,  "Alice Ryhl" <aliceryhl@...gle.com>,  "Trevor Gross"
 <tmgross@...ch.edu>,  "Danilo Krummrich" <dakr@...nel.org>,  "Jens Axboe"
 <axboe@...nel.dk>,  <linux-block@...r.kernel.org>,
  <rust-for-linux@...r.kernel.org>,  <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/9] rust: block,core: rename `RawWriter` to `BufferWriter`

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

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