public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] rust: rbtree: fix cursor method lifetimes to match tree lifetime
@ 2025-11-07  5:06 Hang Shu
  2025-11-07  5:37 ` Onur Özkan
  2025-11-07  9:27 ` Alice Ryhl
  0 siblings, 2 replies; 3+ messages in thread
From: Hang Shu @ 2025-11-07  5:06 UTC (permalink / raw)
  To: ojeda
  Cc: Hang Shu, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Hang Shu, Charalampos Mitrodimas, Borys Tyran,
	Onur Özkan, Daniel Sedlak, Tamir Duberstein, Matt Gilbride,
	rust-for-linux, linux-kernel

From: Hang Shu <hangshu847@gmail.com>

The returned keys and values of cursor methods should be bound by
the lifetime of the rbtree itself ('a), not the lifetime of the cursor.

Without this adjustment, examples like the following fail to compile:

fn test_rbtree_cursor(rbtree: &mut RBTree<i32, i32>) -> &i32 {
    rbtree.try_create_and_insert(1, 1, GFP_KERNEL).unwrap();
    let mut cursor = rbtree.cursor_front().unwrap();
    // compile error
    // cannot return value referencing local variable `cursor`
    cursor.peek_next().unwrap().1
}

This modification ensures that references to tree elements remain valid
independently of the cursor's scope,
aligning with the actual lifetime dependencies in the data structure.

The changes will be applied to multiple similar methods
throughout the Cursor implementation to maintain consistency.

Fixes: 98c14e40e07a ("rust: rbtree: add cursor")
Signed-off-by: Hang Shu <hangshu847@gmail.com>
---
 rust/kernel/rbtree.rs | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/rust/kernel/rbtree.rs b/rust/kernel/rbtree.rs
index 9e178dacddf1..702a1b6ef7a9 100644
--- a/rust/kernel/rbtree.rs
+++ b/rust/kernel/rbtree.rs
@@ -742,7 +742,7 @@ unsafe impl<'a, K: Sync, V: Sync> Sync for Cursor<'a, K, V> {}
 
 impl<'a, K, V> Cursor<'a, K, V> {
     /// The current node
-    pub fn current(&self) -> (&K, &V) {
+    pub fn current(&self) -> (&'a K, &'a V) {
         // SAFETY:
         // - `self.current` is a valid node by the type invariants.
         // - We have an immutable reference by the function signature.
@@ -750,7 +750,7 @@ pub fn current(&self) -> (&K, &V) {
     }
 
     /// The current node, with a mutable value
-    pub fn current_mut(&mut self) -> (&K, &mut V) {
+    pub fn current_mut(&mut self) -> (&'a K, &'a mut V) {
         // SAFETY:
         // - `self.current` is a valid node by the type invariants.
         // - We have an mutable reference by the function signature.
@@ -831,16 +831,16 @@ fn mv(self, direction: Direction) -> Option<Self> {
     }
 
     /// Access the previous node without moving the cursor.
-    pub fn peek_prev(&self) -> Option<(&K, &V)> {
+    pub fn peek_prev(&self) -> Option<(&'a K, &'a V)> {
         self.peek(Direction::Prev)
     }
 
     /// Access the next node without moving the cursor.
-    pub fn peek_next(&self) -> Option<(&K, &V)> {
+    pub fn peek_next(&self) -> Option<(&'a K, &'a V)> {
         self.peek(Direction::Next)
     }
 
-    fn peek(&self, direction: Direction) -> Option<(&K, &V)> {
+    fn peek(&self, direction: Direction) -> Option<(&'a K, &'a V)> {
         self.get_neighbor_raw(direction).map(|neighbor| {
             // SAFETY:
             // - `neighbor` is a valid tree node.
@@ -850,16 +850,16 @@ fn peek(&self, direction: Direction) -> Option<(&K, &V)> {
     }
 
     /// Access the previous node mutably without moving the cursor.
-    pub fn peek_prev_mut(&mut self) -> Option<(&K, &mut V)> {
+    pub fn peek_prev_mut(&mut self) -> Option<(&'a K, &'a mut V)> {
         self.peek_mut(Direction::Prev)
     }
 
     /// Access the next node mutably without moving the cursor.
-    pub fn peek_next_mut(&mut self) -> Option<(&K, &mut V)> {
+    pub fn peek_next_mut(&mut self) -> Option<(&'a K, &'a mut V)> {
         self.peek_mut(Direction::Next)
     }
 
-    fn peek_mut(&mut self, direction: Direction) -> Option<(&K, &mut V)> {
+    fn peek_mut(&mut self, direction: Direction) -> Option<(&'a K, &'a mut V)> {
         self.get_neighbor_raw(direction).map(|neighbor| {
             // SAFETY:
             // - `neighbor` is a valid tree node.
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] rust: rbtree: fix cursor method lifetimes to match tree lifetime
  2025-11-07  5:06 [PATCH] rust: rbtree: fix cursor method lifetimes to match tree lifetime Hang Shu
@ 2025-11-07  5:37 ` Onur Özkan
  2025-11-07  9:27 ` Alice Ryhl
  1 sibling, 0 replies; 3+ messages in thread
From: Onur Özkan @ 2025-11-07  5:37 UTC (permalink / raw)
  To: Hang Shu
  Cc: ojeda, Hang Shu, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Charalampos Mitrodimas,
	Borys Tyran, Daniel Sedlak, Tamir Duberstein, Matt Gilbride,
	rust-for-linux, linux-kernel

On Fri,  7 Nov 2025 05:06:56 +0000
Hang Shu <m18080292938@163.com> wrote:

> From: Hang Shu <hangshu847@gmail.com>
> 

Jfyi, there is no need for adding this.

> The returned keys and values of cursor methods should be bound by
> the lifetime of the rbtree itself ('a), not the lifetime of the
> cursor.
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] rust: rbtree: fix cursor method lifetimes to match tree lifetime
  2025-11-07  5:06 [PATCH] rust: rbtree: fix cursor method lifetimes to match tree lifetime Hang Shu
  2025-11-07  5:37 ` Onur Özkan
@ 2025-11-07  9:27 ` Alice Ryhl
  1 sibling, 0 replies; 3+ messages in thread
From: Alice Ryhl @ 2025-11-07  9:27 UTC (permalink / raw)
  To: Hang Shu
  Cc: ojeda, Hang Shu, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Trevor Gross, Danilo Krummrich, Charalampos Mitrodimas,
	Borys Tyran, Onur Özkan, Daniel Sedlak, Tamir Duberstein,
	Matt Gilbride, rust-for-linux, linux-kernel

On Fri, Nov 7, 2025 at 6:07 AM Hang Shu <m18080292938@163.com> wrote:
>
> From: Hang Shu <hangshu847@gmail.com>
>
> The returned keys and values of cursor methods should be bound by
> the lifetime of the rbtree itself ('a), not the lifetime of the cursor.
>
> Without this adjustment, examples like the following fail to compile:
>
> fn test_rbtree_cursor(rbtree: &mut RBTree<i32, i32>) -> &i32 {
>     rbtree.try_create_and_insert(1, 1, GFP_KERNEL).unwrap();
>     let mut cursor = rbtree.cursor_front().unwrap();
>     // compile error
>     // cannot return value referencing local variable `cursor`
>     cursor.peek_next().unwrap().1
> }
>
> This modification ensures that references to tree elements remain valid
> independently of the cursor's scope,
> aligning with the actual lifetime dependencies in the data structure.
>
> The changes will be applied to multiple similar methods
> throughout the Cursor implementation to maintain consistency.
>
> Fixes: 98c14e40e07a ("rust: rbtree: add cursor")
> Signed-off-by: Hang Shu <hangshu847@gmail.com>
> ---
>  rust/kernel/rbtree.rs | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/rust/kernel/rbtree.rs b/rust/kernel/rbtree.rs
> index 9e178dacddf1..702a1b6ef7a9 100644
> --- a/rust/kernel/rbtree.rs
> +++ b/rust/kernel/rbtree.rs
> @@ -742,7 +742,7 @@ unsafe impl<'a, K: Sync, V: Sync> Sync for Cursor<'a, K, V> {}
>
>  impl<'a, K, V> Cursor<'a, K, V> {
>      /// The current node
> -    pub fn current(&self) -> (&K, &V) {
> +    pub fn current(&self) -> (&'a K, &'a V) {
>          // SAFETY:
>          // - `self.current` is a valid node by the type invariants.
>          // - We have an immutable reference by the function signature.
> @@ -750,7 +750,7 @@ pub fn current(&self) -> (&K, &V) {
>      }
>
>      /// The current node, with a mutable value
> -    pub fn current_mut(&mut self) -> (&K, &mut V) {
> +    pub fn current_mut(&mut self) -> (&'a K, &'a mut V) {

This would allow me to call current_mut() twice on the same cursor to
get two mutable references to the same value. That is not okay.

If you want to have methods that return a reference with the tree's
lifetime instead of the cursor's, then you need to add new methods
(probably called into_*) rather than modify the existing ones.

Alice

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-11-07  9:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-07  5:06 [PATCH] rust: rbtree: fix cursor method lifetimes to match tree lifetime Hang Shu
2025-11-07  5:37 ` Onur Özkan
2025-11-07  9:27 ` Alice Ryhl

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox