Re: [PATCH v5 5/6] rust: rbtree: add `RBTreeCursor`
From: Boqun Feng
Date: Tue Jun 25 2024 - 17:13:05 EST
On Thu, Jun 06, 2024 at 02:50:08PM +0000, Matt Gilbride wrote:
[...]
> +impl<'a, K, V> RBTreeCursor<'a, K, V> {
[...]
> + fn get_neighbor_raw(&self, direction: Direction) -> Option<*mut bindings::rb_node> {
I'd suggest we avoid Option<*mut T> as hard as possible, because it
prevents niche optimization (i.e. the size of Option<*mut ..> above is
16 on a 64 bit system). Could you make it return a
Option<NonNull<bindings::rb_node> instead?
(I think we can also make RBTreeCursor::current as a
NonNull<bindings::rb_node>, but that might be too much, althought that
won't hurt)
> + // SAFETY: `self.current` is valid by the type invariants.
> + let neighbor = unsafe {
> + match direction {
> + Direction::Prev => bindings::rb_prev(self.current),
> + Direction::Next => bindings::rb_next(self.current),
> + }
> + };
> +
> + if neighbor.is_null() {
> + return None;
> + }
> +
> + Some(neighbor)
with Option<NonNull<bindings::rb_node>>, you can implement this as:
NonNull::new(neighbor)
Regards,
Boqun
> + }
> +
[...]