From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-4322.protonmail.ch (mail-4322.protonmail.ch [185.70.43.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C2CCD24337D; Mon, 17 Mar 2025 14:42:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.70.43.22 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1742222552; cv=none; b=PWaLs0rLa96c1ur8WIzsH7whWJGvk1md02/xbnatMsY05RGLUdt3fL4hfzZMKUKbteL+QL0m8YlogDPZrHSlx2IcB/7Oz7L1eqamaRB6nT5DmrOCOkqA+TkZMS9kqy7gGYop+/PLnsmGo1axz2Iri69bMw5/YJl67g4yO7aXAiQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1742222552; c=relaxed/simple; bh=B7w4fF/AQBq5dEVwxdmK1HtwIC8KWcaPAsFWCNa0C2U=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=i9hwresmiacpdrsGZw24x7HxHd7na31j3M86LCxubmQLZsJLrJc0VNOWUt2fwlD0O/XuwL4v2mX7pSk0KM2eAqcOX8QphKkxi/xlGbBkabjKsRVrLapLXkb7IIQ28SGbo1/bsql8MbZ0YWcn9aFJZZUYPOYVOg5Wpt9fUbrZpGU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=proton.me; spf=pass smtp.mailfrom=proton.me; dkim=pass (2048-bit key) header.d=proton.me header.i=@proton.me header.b=ToKl/xxx; arc=none smtp.client-ip=185.70.43.22 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=proton.me Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=proton.me Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=proton.me header.i=@proton.me header.b="ToKl/xxx" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=proton.me; s=protonmail; t=1742222548; x=1742481748; bh=B7w4fF/AQBq5dEVwxdmK1HtwIC8KWcaPAsFWCNa0C2U=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post; b=ToKl/xxxpx+5YIjI0l2d3U66G5GtgDC1CcUY33hlP+2YCs1G3xywS8/rb+xCyJfk8 7QwgfrHvxjAzYc88oY8J8iYMGsIwF53WyT2x6Mpqoa9zdVSNej1lR9c7kF6dtOrE84 htRcjMafXeKfVORHk6Gf1ThpPMmEcWw918B0KhvPoU69MitX8kmOte3ZtfKuV29G1k PZzca7AZmj+njtUZhx4Gcw3jsOdPkSyljrTG1VMdRPgEV7Raq5jXX1zTGqcq5fSzvk 30n28sXXnG0PnfbTL/gEQbBKg6XsDYanWwDJuX3DjbyWZSjp+D0gtDBVDNFq1W3mgo y2dl9pz/eSsgg== Date: Mon, 17 Mar 2025 14:42:23 +0000 To: Alice Ryhl , Tamir Duberstein From: Benno Lossin Cc: Danilo Krummrich , Andrew Ballance , Miguel Ojeda , Alex Gaynor , Boqun Feng , Gary Guo , =?utf-8?Q?Bj=C3=B6rn_Roy_Baron?= , Andreas Hindborg , Trevor Gross , rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/2] rust: alloc: add `Vec::dec_len` Message-ID: In-Reply-To: References: <20250316-vec-set-len-v1-0-60f98a28723f@gmail.com> <20250316-vec-set-len-v1-2-60f98a28723f@gmail.com> Feedback-ID: 71780778:user:proton X-Pm-Message-ID: d709fa82dd85d03fde09c7bd5a390a1cececf112 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable 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=E2=80=AFAM Benno Lossin 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 m= ethod >> > > > is intended to be used from methods that remove elements from `Vec= ` such >> > > > as `truncate`, `pop`, `remove`, and others. This method is intenti= onally >> > > > 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 AP= I >> > > that takes ownership of the pointer, then I want them to be able to = call >> > > `dec_len` manually. >> >=20 >> > This is premature. It is trivial to make this function pub when the ne= ed arises. >>=20 >> 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? --- Cheers, Benno