Re: [PATCH v10 5/5] rust: rbtree: add `RBTree::entry`

[Benno Lossin]

On 19.08.24 17:07, Matt Gilbride wrote:
> From: Alice Ryhl <aliceryhl@xxxxxxxxxx>
> 
> This mirrors the entry API [1] from the Rust standard library on
> `RBTree`. This API can be used to access the entry at a specific key and
> make modifications depending on whether the key is vacant or occupied.
> This API is useful because it can often be used to avoid traversing the
> tree multiple times.
> 
> This is used by binder to look up and conditionally access or insert a
> value, depending on whether it is there or not [2].
> 
> Link: https://doc.rust-lang.org/stable/std/collections/btree_map/enum.Entry.html [1]
> Link: https://android-review.googlesource.com/c/kernel/common/+/2849906 [2]
> Signed-off-by: Alice Ryhl <aliceryhl@xxxxxxxxxx>
> Tested-by: Alice Ryhl <aliceryhl@xxxxxxxxxx>
> Reviewed-by: Boqun Feng <boqun.feng@xxxxxxxxx>
> Signed-off-by: Matt Gilbride <mattgilbride@xxxxxxxxxx>

One nit below, but feel free to add if it doesn't work:

Reviewed-by: Benno Lossin <benno.lossin@xxxxxxxxx>

> ---
>  rust/kernel/rbtree.rs | 310 ++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 235 insertions(+), 75 deletions(-)

[...]

> +    /// Gets a mutable reference to the value in the entry.
> +    pub fn get_mut(&mut self) -> &mut V {
> +        // SAFETY:
> +        // - `self.node_links` is a valid pointer to a node in the tree.
> +        // - We have exclusive access to the underlying tree, and can thus give out a mutable reference.
> +        unsafe {
> +            &mut (*(container_of!(self.node_links, Node<K, V>, links) as *mut Node<K, V>)).value

Does `.cast_mut()` work here instead of `as *mut Node<K, V>`? (also
below)

---
Cheers,
Benno

> +        }
> +    }
> +
> +    /// Converts the entry into a mutable reference to its value.
> +    ///
> +    /// If you need multiple references to the `OccupiedEntry`, see [`self#get_mut`].
> +    pub fn into_mut(self) -> &'a mut V {
> +        // SAFETY:
> +        // - `self.node_links` is a valid pointer to a node in the tree.
> +        // - This consumes the `&'a mut RBTree<K, V>`, therefore it can give out a mutable reference that lives for `'a`.
> +        unsafe {
> +            &mut (*(container_of!(self.node_links, Node<K, V>, links) as *mut Node<K, V>)).value
> +        }
> +    }