rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Ballance <andrewjballance@gmail.com>
To: remo@buenzli.dev
Cc: a.hindborg@kernel.org, alex.gaynor@gmail.com,
	aliceryhl@google.com, andriy.shevchenko@linux.intel.com,
	benno.lossin@proton.me, bjorn3_gh@protonmail.com,
	boqun.feng@gmail.com, dakr@kernel.org,
	devicetree@vger.kernel.org, dirk.behme@de.bosch.com,
	djrscally@gmail.com, gary@garyguo.net,
	gregkh@linuxfoundation.org, heikki.krogerus@linux.intel.com,
	linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org,
	ojeda@kernel.org, rafael@kernel.org, robh@kernel.org,
	rust-for-linux@vger.kernel.org, sakari.ailus@linux.intel.com,
	saravanak@google.com, tmgross@umich.edu,
	andrewjballance@gmail.com
Subject: Re: [PATCH 07/10] rust: Add arrayvec
Date: Wed, 26 Mar 2025 16:06:15 -0500	[thread overview]
Message-ID: <20250326210615.696266-1-andrewjballance@gmail.com> (raw)
In-Reply-To: <20250326171411.590681-8-remo@buenzli.dev>

On Wed, Mar 26, 2025 at 12:13 PM Remo Senekowitsch Wrote: 
> +pub struct ArrayVec<const N: usize, T> {
> +    array: [core::mem::MaybeUninit<T>; N],
> +    len: usize,
> +}

I would reverse the order of the generic arguments so T is first so
that it matches the declaration of an array: [T; N] 

> +    /// Adds a new element to the end of the vector.
> +    ///
> +    /// # Panics
> +    ///
> +    /// Panics if the vector is already full.
> +    pub fn push(&mut self, elem: T) {
> +        if self.len == N {
> +            panic!("OOM")
> +        }
> +        self.array[self.len] = MaybeUninit::new(elem);
> +        self.len += 1;
> +    }

This function should not panic. I would rewrite it so that its signature is
pub fn push(&mut self, elem: T) -> Result<(), T>

> +impl<const N: usize, T> AsRef<[T]> for ArrayVec<N, T> {
> +    fn as_ref(&self) -> &[T] {
> +        // SAFETY: As per the type invariant, all elements at index < self.len
> +        // are initialized.
> +        unsafe { core::mem::transmute(&self.array[..self.len]) }
> +    }
> +}
> +
> +impl<const N: usize, T> AsMut<[T]> for ArrayVec<N, T> {
> +    fn as_mut(&mut self) -> &mut [T] {
> +        // SAFETY: As per the type invariant, all elements at index < self.len
> +        // are initialized.
> +        unsafe { core::mem::transmute(&mut self.array[..self.len]) }
> +    }
> +}

I would replace both uses of transmute here with slice::from_raw_parts
like you did in drop. 

also it's not needed but you could also impl Deref and DerefMut for ArrayVec
because you have already impl'ed AsRef and AsMut so it should be simple

> +impl<const N: usize, T> Drop for ArrayVec<N, T> {
> +    fn drop(&mut self) {
> +        unsafe {

this is missing a safety comment

> +            let slice: &mut [T] =
> +                core::slice::from_raw_parts_mut(self.array.as_mut_ptr().cast(), self.len);
> +            core::ptr::drop_in_place(slice);
> +        }
> +    }
> +}

Andrew


  reply	other threads:[~2025-03-26 21:06 UTC|newest]

Thread overview: 83+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-26 17:13 [PATCH 0/10] More Rust bindings for device property reads Remo Senekowitsch
2025-03-26 17:13 ` [PATCH 01/10] rust: Move property_present to property.rs Remo Senekowitsch
2025-03-26 20:51   ` Rob Herring
2025-03-26 22:41     ` Rob Herring
2025-04-04 12:48     ` Remo Senekowitsch
2025-03-26 20:58   ` Andrew Ballance
2025-03-27  8:37   ` Andy Shevchenko
2025-03-27 13:55     ` Rob Herring
2025-03-27 17:49       ` Andy Shevchenko
2025-03-26 17:13 ` [PATCH 02/10] rust: Add an Integer trait Remo Senekowitsch
2025-03-26 20:00   ` Rob Herring
2025-03-26 17:13 ` [PATCH 03/10] device property: Add fwnode_property_read_int_array() Remo Senekowitsch
2025-03-27  8:41   ` Andy Shevchenko
2025-04-02 16:04     ` Remo Senekowitsch
2025-04-03 13:28       ` Andy Shevchenko
2025-04-03 16:08         ` Rob Herring
2025-04-03 16:15           ` Remo Senekowitsch
2025-04-03 17:04           ` Remo Senekowitsch
2025-04-03 17:22             ` Rob Herring
2025-04-04 12:29           ` Remo Senekowitsch
2025-04-03 16:04     ` Rob Herring
2025-04-03 16:15       ` Andy Shevchenko
2025-04-03 16:36         ` Rob Herring
2025-04-03 17:54           ` Andy Shevchenko
2025-04-03 18:48             ` Rob Herring
2025-04-03 20:36               ` Miguel Ojeda
2025-04-04 11:00                 ` Andy Shevchenko
2025-04-04 14:12                   ` Rob Herring
2025-04-04 16:35                     ` Andy Shevchenko
2025-03-26 17:13 ` [PATCH 04/10] rust: Add bindings for reading device properties Remo Senekowitsch
2025-03-26 21:27   ` Rob Herring
2025-04-02 16:28     ` Remo Senekowitsch
2025-03-26 17:13 ` [PATCH 05/10] rust: Read properties via single generic method Remo Senekowitsch
2025-03-26 21:33   ` Rob Herring
2025-03-26 17:13 ` [PATCH 06/10] rust: property: Add child accessor and iterator Remo Senekowitsch
2025-03-26 21:04   ` Andrew Ballance
2025-03-26 21:40   ` Rob Herring
2025-03-26 17:13 ` [PATCH 07/10] rust: Add arrayvec Remo Senekowitsch
2025-03-26 21:06   ` Andrew Ballance [this message]
2025-03-27 14:40   ` Danilo Krummrich
2025-03-26 17:13 ` [PATCH 08/10] rust: property: Add property_get_reference_args Remo Senekowitsch
2025-03-26 21:07   ` Andrew Ballance
2025-03-26 21:25     ` Miguel Ojeda
2025-03-26 21:45       ` Remo Senekowitsch
2025-03-27 14:32   ` Danilo Krummrich
2025-03-26 17:13 ` [PATCH 09/10] rust: property: Add PropertyGuard Remo Senekowitsch
2025-03-26 21:10   ` Andrew Ballance
2025-03-26 22:25   ` Rob Herring
2025-03-26 17:13 ` [PATCH 10/10] samples: rust: platform: Add property read examples Remo Senekowitsch
2025-03-26 22:01   ` Rob Herring
2025-03-26 22:23     ` Remo Senekowitsch
2025-03-27  0:02       ` Rob Herring
2025-03-27 10:28   ` Danilo Krummrich
2025-03-26 20:54 ` [PATCH 0/10] More Rust bindings for device property reads Andrew Ballance
2025-03-27  8:42   ` Andy Shevchenko
2025-04-14 15:26 ` [PATCH v2 0/5] " Remo Senekowitsch
2025-04-14 15:26   ` [PATCH v2 1/5] rust: Move property_present to separate file Remo Senekowitsch
2025-04-14 16:00     ` Danilo Krummrich
2025-04-14 16:40       ` Remo Senekowitsch
2025-04-14 18:00         ` Danilo Krummrich
2025-04-15 11:17           ` Remo Senekowitsch
2025-04-14 15:26   ` [PATCH v2 2/5] rust: Add bindings for reading device properties Remo Senekowitsch
2025-04-14 17:44     ` Danilo Krummrich
2025-04-14 18:05       ` Danilo Krummrich
2025-04-14 23:55       ` Remo Senekowitsch
2025-04-15  9:48         ` Danilo Krummrich
2025-04-15 11:11           ` Remo Senekowitsch
2025-04-15 12:46             ` Rob Herring
2025-04-15 13:11               ` Danilo Krummrich
2025-04-15 14:47               ` Remo Senekowitsch
2025-04-16 18:28           ` Gary Guo
2025-04-23 12:29     ` Dirk Behme
2025-04-24 11:25       ` Remo Senekowitsch
2025-04-23 12:34     ` Dirk Behme
2025-04-14 15:26   ` [PATCH v2 3/5] rust: property: Add child accessor and iterator Remo Senekowitsch
2025-04-14 16:09     ` Danilo Krummrich
2025-04-14 15:26   ` [PATCH v2 4/5] rust: property: Add property_get_reference_args Remo Senekowitsch
2025-04-14 15:26   ` [PATCH v2 5/5] samples: rust: platform: Add property read examples Remo Senekowitsch
2025-04-23 12:39     ` Dirk Behme
2025-04-24  5:47       ` Dirk Behme
2025-04-14 15:38   ` [PATCH v2 0/5] More Rust bindings for device property reads Miguel Ojeda
2025-04-14 16:07     ` Remo Senekowitsch
2025-04-14 16:44       ` Miguel Ojeda

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250326210615.696266-1-andrewjballance@gmail.com \
    --to=andrewjballance@gmail.com \
    --cc=a.hindborg@kernel.org \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=benno.lossin@proton.me \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dakr@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dirk.behme@de.bosch.com \
    --cc=djrscally@gmail.com \
    --cc=gary@garyguo.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=heikki.krogerus@linux.intel.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rafael@kernel.org \
    --cc=remo@buenzli.dev \
    --cc=robh@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=sakari.ailus@linux.intel.com \
    --cc=saravanak@google.com \
    --cc=tmgross@umich.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).