Re: [PATCH v5 5/6] rust: rbtree: add `RBTreeCursor`

[Boqun Feng]

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

> +    }
> +
[...]