linux-kernel - Re: [PATCH 2/2] rust: alloc: add `Vec::dec

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAJ-ks9kEpC=7jK9CLNKBWYpoOyAo_Wr8eyvaYf0ZSiC8m-VREQ@mail.gmail.com>
Date: Mon, 17 Mar 2025 10:44:25 -0400
From: Tamir Duberstein <tamird@...il.com>
To: Benno Lossin <benno.lossin@...ton.me>
Cc: Alice Ryhl <aliceryhl@...gle.com>, Danilo Krummrich <dakr@...nel.org>, 
	Andrew Ballance <andrewjballance@...il.com>, Miguel Ojeda <ojeda@...nel.org>, 
	Alex Gaynor <alex.gaynor@...il.com>, Boqun Feng <boqun.feng@...il.com>, 
	Gary Guo <gary@...yguo.net>, Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
	Andreas Hindborg <a.hindborg@...nel.org>, Trevor Gross <tmgross@...ch.edu>, 
	rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] rust: alloc: add `Vec::dec_len`

On Mon, Mar 17, 2025 at 10:42 AM Benno Lossin <benno.lossin@...ton.me> wrote:
>
> On Mon Mar 17, 2025 at 1:59 PM CET, Alice Ryhl wrote:
> > On Mon, Mar 17, 2025 at 11:47:50AM +0000, Alice Ryhl wrote:
> >> On Mon, Mar 17, 2025 at 07:34:44AM -0400, Tamir Duberstein wrote:
> >> > On Mon, Mar 17, 2025 at 6:04 AM Benno Lossin <benno.lossin@...ton.me> wrote:
> >> > >
> >> > > On Sun Mar 16, 2025 at 11:32 PM CET, Tamir Duberstein wrote:
> >> > > > Add `Vec::dec_len` that reduces the length of the receiver. This method
> >> > > > is intended to be used from methods that remove elements from `Vec` such
> >> > > > as `truncate`, `pop`, `remove`, and others. This method is intentionally
> >> > > > not `pub`.
> >> > >
> >> > > I think it should be `pub`. Otherwise we're loosing functionality
> >> > > compared to now. If one decides to give the raw pointer to some C API
> >> > > that takes ownership of the pointer, then I want them to be able to call
> >> > > `dec_len` manually.
> >> >
> >> > This is premature. It is trivial to make this function pub when the need arises.
> >>
> >> Normally I'd agree with Benno, but in this case I think having it
> >> private is preferable. The function is safe, so it's too easy for
> >> end-users to confuse it with truncate.
> >
> > Thinking more about this ... I think we should have `set_len` and
> > `inc_len` instead. That way, both methods are unsafe so people will not
> > accidentally use `set_len` when they meant to use `truncate`.
>
> I agree for this on the public API. The way I usually saw `set_len`
> being used for decrementing was truncation without dropping the old
> values. And that is going to be `vec.dec_len(vec.len())` with the
> current design. `vec.set_len(0);` is much clearer in that respect.
>
> But for the internals, I'd say that `dec_len` is nicer, so for `pop` one
> would then use `self.dec_len(1)`.
>
> How about we keep `set_len` and make `dec_len` a private, safe helper?

This discussion is _way_ too speculative for my taste. If you'd like
to do this kind of thing, I'm happy to drop this patch or the series.
I'm not comfortable adding API whose usage I haven't seen and don't
understand.